劣质代码评析

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

【题目】

　　将一个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的交换，这样得到的结果显然是错误的。在这里代码作者犯了中国古代寓言“刻舟求剑”中一模一样的错误。古希腊哲学家说人不能两次走入同一条河，说的也是同样的道理。
　　这个错误从另一个方面提示我们，写代码应该一个问题一个问题地解决，不要眉毛胡子一把抓，把几个问题搅和在一块。
　　好了。从内容到形式，从算法到语法，代码已经是错得一塌糊涂了，差不多要亮瞎我的双眼了。没有任何理由再继续看下去了。就此打住。

作者： Leo_wl
　　　　
出处： http://www.cnblogs.com/Leo_wl/
　　　　
本文版权归作者和博客园共有，欢迎转载，但未经作者同意必须保留此段声明，且在文章页面明显位置给出原文连接，否则保留追究法律责任的权利。
版权信息

查看更多关于劣质代码评析的详细内容...

声明：本文来自网络，不代表【好得很程序员自学网】立场，转载请注明出处：http://haodehen.cn/did48446

更新时间：2022-09-24 阅读：35次