• 劣质代码评析——刻舟求剑的故事


     【题目】

      将一个5*5的矩阵中最大的元素中最大的元素放在中心,4个角分别放4个最小的元素(顺序为从左到右,从上到下依次从小到大存放),写一函数实现之,用main函数调用。

        ——谭浩强 ,《C程序设计(第四版)学习辅导》,清华大学出版社,2010年7月,p108
    【评析】
      这其实是一个相当复杂的问题,题目作者自己大概根本没意识到,更没有真正想清楚这个问题的解决,拍拍脑袋就出题了。
      题目中明显的问题是语义不清,比如什么是“最大”?什么是“最小”?什么是倒数第二小?这些含义都不明确。所以如果不进一步对问题进行补充说明,题目的解并不能保证唯一。客气点说题目不严格,如果事实求是说,题目本身就是错误的。
      例如,1、1、2、2、3、3这6个数中,究竟1是这组整数中第二小的数,还是2是第二小的整数?这个问题的答案很可能有歧义,更有歧义的是究竟哪个位置上的数是第2小的数?
      “写一函数实现之”完全是一个过分的要求,也是一个外行的要求,就跟要求别人用脚指头敲键盘一样。
    【原代码】

    View Code
    0. #include <stdio.h>
    1. int main()
    2. {void change(int *p);
    3.  int a[5][5],*p,i,j;
    4.  printf("input matrix:\n");
    5.  for(i=0;i<5;i++)
    6.    for(j=0;j<5;j++)
    7.      scanf("%d",&a[i][j]);
    8.  p=&a[0][0];
    9.  change(p);
    10.  printf("Now matrix:\n");
    11.  for(i=0;i<5;i++)
    12.    {for(j=0;j<5;j++)
    13.       printf("%d ",a[i][j]);
    14.     printf("\n");
    15.    }
    16.  return 0;
    17. }
    18. void change(int *p)
    19.  {int i,j,temp;
    20.   int *pmax,*pmin;
    21.   pmax=p;
    22.   pmin=p;
    23.   for(i=0;i<5;i++)
    24.     for(j=i;j<5;j++)
    25.      {if(*pmax<*(p+5*i+j))pmax=p+5*i+j;
    26.       if(*pmin>*(p+5*i+j))pmin=p+5*i+j;
    27.      }
    28.   temp=*(p+12);
    29.   *(p+12)=*pmax;
    30.   *pmax=temp;
    31.   temp=*p;
    32.   *p=*pmin;
    33.   *pmin=temp;
    34.   pmin=p+1;
    35.   for(i=0;i<5;i++)
    36.     for(j=0;j<5;j++)
    37.       if(((p+5*i+j)!=p)&&(*pmin>*(p+5*i+j)))pmin=p+5*i+j;
    38.   temp=*pmin;
    39.   *pmin=*(p+4);
    40.   *(p+4)=temp;
    41.   pmin=p+1;
    42.   for(i=0;i<5;i++)
    43.     for(j=0;j<5;j++)
    44.       if(((p+5*i+j)!=(p+4))&&((p+5*i+j)!=p)&&(*pmin>*(p+5*i+j)))
    45.            pmin=p+5*i+j;
    46.   temp=*pmin;
    47.   *pmin=*(p+20);
    48.   *(p+20)=temp;
    49.   pmin=p+1;
    50.   for(i=0;i<5;i++)
    51.      for(j=0;j<5;j++)
    52.        if(((p+5*i+j)!=p)&&((p+5*i+j)!=(p+4))&&((p+5*i+j)!=(p+20))&&
    53.             (*pmin>*(p+5*i+j))) pmin=p+5*i+j;
    54.   temp=*pmin;
    55.   *pmin=*(p+24);
    56.   *(p+24)=temp;
    57. }

      ——谭浩强 ,《C程序设计(第四版)学习辅导》,清华大学出版社,2010年7月,p109~110
    【评析】

    View Code
    4.  printf("input matrix:\n");
    5.  for(i=0;i<5;i++)
    6.    for(j=0;j<5;j++)
    7.      scanf("%d",&a[i][j]);
    8.  p=&a[0][0];
    9.  change(p);
    10.  printf("Now matrix:\n");
    11.  for(i=0;i<5;i++)
    12.    {for(j=0;j<5;j++)
    13.       printf("%d ",a[i][j]);
    14.     printf("\n");
    15.    }

      不会写代码的人毛病,就是急着先把能写的内容一下子写完。在这段代码中可以看到change(p)像是掉进了一堆乱麻里的一只牙签一样,整个代码的结构乱作一团。这是一只很拙劣的风格,对比一下下面的写法就知道这段代码有多糟糕了。

    #include <stdio.h>
    
    int main( void )
    {
       int a[5][5] ;
    
       input(a,5);
       change(*a);
       output(a,5);
    
       return 0;
    }
    

      哪个写法清晰明了是一目了然的吧?
      结构化程序设计中有一条著名的原则,这就是自顶向下(Top Down),这个原则说的是先从大处着眼,把问题分解为若干小问题,然后再把这些小问题进一步分解为更小的问题,直到无法再分解为止。这叫逐步细化。
    每一层分解都是一种抽象,抽象的目的是为了概括,概括的目的是为了有一个简单的表现形式,从而把不必要的细节隐藏在这种简单的表现形式的后面。函数是实现这种思想的利器。在每次的分解过程中,问题总是被分解为同一层次上的问题。这样的代码才可能具有优雅的均衡感和良好的可读性。

    8.  p=&a[0][0];
    9.  change(p);

      是典型的多此一举。因为p这个变量多余。这两句话无非就是

    change(*a);   //或change(a[0]);  //或change(&a[0][0]);
    

    而已。需要注意的是,change()函数要操作的是整个数组,而这里的实参仅仅是一个指向a[0][0]的指针。很显然,change()得到的信息并不充分,它不可能知道它要操作的是一个int [5][5]类型的数组。换言之,chang()函数不可能完成任务,除非它再通过别的歪门邪道获得其他必要的信息。
      下面考察chang()函数定义。
      从没见过这么丑的函数!居然能把函数写得如此之丑,不禁叹为奇迹。

    void change(int *p)

    前面提到过,参数不完整。这注定后面要走歪门邪道(比如许多无厘头的Magic Number)。

    23.   for(i=0;i<5;i++)
    24.     for(j=i;j<5;j++)
    25.      {if(*pmax<*(p+5*i+j))pmax=p+5*i+j;
    26.       if(*pmin>*(p+5*i+j))pmin=p+5*i+j;
    27.      }

      这段代码试图寻找最大值与最小值的位置。但是正如前面讨论过的那样,最大、最小值的位置可能不是唯一的。那么究竟要找的是哪一个呢?其实代码作者自己也不知道,找到哪个算哪个,无头苍蝇撞大运。
      这其中有一个明显的错误,就是内层for语句中的j=i。这表明它并不是在整个数组中寻找最大最小值的位置,而只是在数组中的一个局部寻找最大最小值的位置。这样的代码显然是错误的。
      即使改正了这个错误,从C语言的语法角度讲,这段代码也还是错误的。因为p是指向main()中a[0][0]的指针,而a[0][0]是一维数组a[0]的一个元素。根据C语言的规则,这个p做加减法得到的结果必须依然指向a[0]中的元素或者a[0]中最后一个元素的下一个位置,否则代码行为是未定义的。而在p+5*i+j这个表达式中,由于5*i+j的值可能大于5,所以是未定义行为。所谓未定义行为通俗地来说就是,C语言并没有承诺这样写会得到什么,这样的代码可能得到任何结果。就像疯子的胡言乱语可以随意解释一样。
      即使抛开前面一系列错误不谈,后面的代码依然是错误的:

    28.   temp=*(p+12);
    29.   *(p+12)=*pmax;
    30.   *pmax=temp;
    31.   temp=*p;
    32.   *p=*pmin;
    33.   *pmin=temp;

      这里,代码把pmax所指向的数据对象与p+12(这个表达式本身就不靠谱)所指向的数据对象交换,然后又把pmin所指向的数据对象与p所指向的数据对象交换。但是代码作者忘记了,pmin所指向的位置可能恰恰是p+12所指向的位置,在这种情况下,第二次交换就是最大元素与p指向的元素的交换,而非所期待的最小元素与*p的交换,这样得到的结果显然是错误的。在这里代码作者犯了中国古代寓言“刻舟求剑”中一模一样的错误。古希腊哲学家说人不能两次走入同一条河,说的也是同样的道理。
      这个错误从另一个方面提示我们,写代码应该一个问题一个问题地解决,不要眉毛胡子一把抓,把几个问题搅和在一块。
      好了。从内容到形式,从算法到语法,代码已经是错得一塌糊涂了,差不多要亮瞎我的双眼了。没有任何理由再继续看下去了。就此打住。

  • 相关阅读:
    poj 3264 Balanced Lineup
    poj 2762 Going from u to v or from v to u?
    hdu 3671 Boonie and Clyde
    zoj 3195 Design the city
    poj 1523 SPF
    Codeforces Polo the Penguin and Matrix
    MVC原理的简述(转)
    C#访问权限修饰符
    XML Schema介绍
    Sql批量删除/插入
  • 原文地址:https://www.cnblogs.com/pmer/p/2604229.html
Copyright © 2020-2023  润新知