垃圾代码评析——关于《C程序设计伴侣》6.2(一)

【题目】

  西安交大的计算机专业2011级有6个班,每个班的人数不等,但最多不超过100个。现在期末考试结束后,老师要统计每个班的平均成绩并将各个班级的成绩按照班级平均成绩的次序打印出来(也就是按照从低到高的顺序,先打印平均成绩较低的班级的成绩)。老师希望你帮他写一个程序来完成这一工作。
    ——陈良乔 ,《C程序设计伴侣》,人民邮电出版社,2012年10月,p104

【评析】

  作为例题或练习来说,这个题目很垃圾。最垃圾的地方在于那个“100”。这非但与实际不合,而且使得代码基本无法测试,因为通常做练习时不可能真的输入600个数据进行测试。既然不能较完整地进行测试,代码的正确性就难免大打折扣。


【样本】

  •  向二维数组输入数据

  定义二维数组之后,接下来的任务,自然是利用scanf()函数将数据输入到这个数组中。因为这是一个二维数组,所以我们需要一个嵌套的for语句,第一层for语句在数组的第一个维度循环(从0到5),第二层for语句则在数组的第二个维度循环(从0到99)。这样才有可能访问到数组中的所有数据。因为每个班级的人数并不一定是100人,所以我们还需要在输入的时候用条件结构对输入数据进行判断,如果输入数据是0,则表示这个班级的输入结束,可以开始下一个班级的循环。经过这样的分析,我们可以将程序的输入数据部分实现如下:
    ——陈良乔 ,《C程序设计伴侣》,人民邮电出版社,2012年10月,p104

【评析】

  “如果输入数据是0,则表示这个班级的输入结束”,这其实是一个很拙劣的假设,因为根据常识分数是有可能为0的。除此之外,这还会带来其他问题,导致代码错误。

【样本】 

1. #include <stdio.h>
2. #include <stdLib.h>
3. #include <memory.h>
4. 
5. int main() 
6. {
7.    // 定义保存批量数据的二维数组,
8.    // 并用memset()函数完成数组的初始化
9.    const int classnum = 6;
10.    const int stnum = 100;
11.    int scores[classnum][stnum];
12.    memset(scores,0,classnum*stnum*sizeof(int));
13.    
14.    // 利用for语句完成数据的输入
15.    // 逐个班级循环
16.    for(int i = 0; i < classnum;++i)
17.    {
18.       printf("please input the  scores of class %d:\n",i+1);
19.       // 逐个学生循环
20.       for(int j = 0; j < stnum; ++j)
21.       {
22.          // 将输入的数据保存到scores[i][j]
23.          scanf("%d",&scores[i][j]);
24.          
25.          // 判断刚刚输入的数据是否为0,
26.          // 如果为0,则利用break结束本层循环
27.          if(0 == scores[i][j])
28.             break;
29.       }
30.    }
31.    
32.    //
33.    
34.    return 0;
35. }

  在这里,我们首先定义了保存批量数据用的scores这个二维数组,然后利用一个嵌套的for语句将输入的数据保存到数组中。for语句会逐个遍历数组中的数据,让我们可以利用scanf()函数将输入数据保存到数组元素(scores[i][j])中。因为一些特殊的设定(以0表示输入结束),我们还需要利用条件结构对输入数据进行判断(if(0 == scores[i][j])),如果发现输入数据是0,则结束本层循环,开始下一个班级的成绩输入。
    ——陈良乔 ,《C程序设计伴侣》,人民邮电出版社,2012年10月,p104~105

【评析】

  这段代码的第一个问题是

2. #include <stdLib.h>

   不难注意到其中的<stdlib.h>被写成了<stdLib.h>,这在某些环境下会造成错误。

9.    const int classnum = 6;
10.    const int stnum = 100;
11.    int scores[classnum][stnum];

  这是一种半吊子写法。为什么这么说呢?
  首先,完全没必要!对这个问题来说没有任何必要用变量给出数组的尺寸,因为6和100都是问题给出的常数。定义数组只要

int scores[6][100];

  对于初学者来说就是很好的写法了。不需要把这个数组定义成复杂的VLA(variable length array)。
  VLA只是C99的特性,很多编译器都不支持,例如微软的VC++6.0。而且在C语言的最新标准C11中,VLA并不是编译器必须支持的,它只是一种Conditional feature。况且使用VLA,使得代码丧失了方便地进行初始化的可能性(VLA不可以初始化)。
  如果是为了避免在定义数组时使用6和100这样的MagicNumber,其实只要使用宏定义就完全可达到目的:

 #define CLASSNUM 6
 #define STNUM 100
 int scores[CLASSNUM][STNUM];

  这里使用了预处理命令。
  有一些人对预处理极其反感,反感的原因之一是这里的两个符号常量在预处理之后依然被替换为字面常量,在调试时可能多有不便。这个问题同样容易解决,只要

enum { 
         CLASSNUM  = 6 , 
         STNUM = 100 
     };
int scores[CLASSNUM][STNUM];

  这种写法克服了使用宏定义的缺点和不足。

12.    memset(scores,0,classnum*stnum*sizeof(int));

  这条语句驴唇不对马嘴。它的目的是想实现“完成数组的初始化”,但是由于scores中有classnum*stnum个int类型数据,所以所谓的初始化的含义是向scores写classnum*stnum个int类型的初始数据。而12行的含义则是用classnum * stnum * sizeof(int)个(unsigned char)0填充scores这块空间,和数组的初始化完全不是一回事。
  12行本来是不必要的,但是代码作者有一种“初始化强迫症”——不管有没有必要都初始化。然而使用VLA又将使用常规方法初始化的路作茧自缚地给堵上了,于是自作聪明地使用不伦不类的memset()。和这种吃力不讨好且概念错误的方法相比,前面任何一种非VLA方案,都可以简单轻松地用

int scores[CLASSNUM][STNUM] = {0};

的方法实现将scores所有int类型元素初始化为0值的功能。

14.    // 利用for语句完成数据的输入
15.    // 逐个班级循环
16.    for(int i = 0; i < classnum;++i)
17.    {
18.       printf("please input the  scores of class %d:\n",i+1);
19.       // 逐个学生循环
20.       for(int j = 0; j < stnum; ++j)
21.       {
22.          // 将输入的数据保存到scores[i][j]
23.          scanf("%d",&scores[i][j]);
24.          
25.          // 判断刚刚输入的数据是否为0,
26.          // 如果为0,则利用break结束本层循环
27.          if(0 == scores[i][j])
28.             break;
29.       }
30.    }

   这个循环嵌套结构用于输入。它的如意算盘是把scores这个二维数组填充成类似字符串那种结构,即对于每个一位数组scores[i]中都有一个0值元素表示它本身及其后面的元素无效。但是作者忘记了,在有些情况下这根本不可能。比如,一口气输入classnum* stnum个非0数据,scores中就不可能存在值为0的元素。这一点对于这个题目后面的代码来说是一个巨大的漏洞。

(待续)

posted @ 2012-11-04 22:36  garbageMan  阅读(2846)  评论(107编辑  收藏  举报