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

【样本】

1. #include <stdio.h>
2. #include <stdlib.h>
3. #include <stdbool.h>
4. #include <string.h>
5. 
6. //定义表示学生信息结点的结构体 
7. typedef struct _student
8. {
9.    char name[20];
10.    float score;
11.    //定义指向链表中下一个结点的指针  
12.    struct _student* next;
13. }student;
14. 
15. void printlist(student*);
16. 
17. int main( )
18. {
19.   //第一步:定义指向链表头和尾的指针 
20.   student* head = NULL ;
21.   student* tail = NULL ;
22.   
23.   char name[20] = "" ;
24.   float score = 0.0f ;
25.   
26.   bool ishead = true;
27.   while(scanf("%s %f",name,&score)!=EOF)
28.   {
29.      //第二步“:根据结点数据创立结点 
30.      //首先,用malloc()函数动态申请内存,内存的大小就是一个结点的大小 
31.      student* node = malloc(sizeof(student));  
32.      
33.      //然后将用户输入的数据保存到这个结点 
34.      strcpy(node->name,name);
35.      node->score = score;
36.      
37.      //第三步:调整节点之间的指向关系 
38.      //如果这是链表中的第一个结点。 
39.      if(ishead)
40.      {
41.          //将指向链表首结点的head指向这个结点 
42.          head = node ;
43.          //首结点尚无下一个结点 
44.          head->next = NULL ;
45.          //当前结点就是链表的尾结点 
46.          tail = node;
47.          
48.          //首结点已经处理完毕,下一个结点就是普通结点了 
49.          ishead = false;
50.      }
51.      else
52.      {
53.          //将新结点添加到已有链表的末尾 
54.          tail->next = node;
55.          //将新的结点作为新的尾结点 
56.          tail = node;
57.      }
58.   }//第四步:重复第二步和第三步,逐个添加结点
59.   //这里利用一个while循环重复第二步和第三步。
60.   //直到用户用"Ctrl+z"结束数据输入为止 
61.   
62.   //将尾结点的next设置为NULL,表示这是链表的结束 
63.   if(NULL!=tail)
64.       tail->next = NULL;
65.   else
66.       return -1;
67.   
68.   //对链表进行处理… 
69.   
70.   printlist(head);
71.   return 0;
72. }
73. 
74. void printlist(student* head)
75. {
76.    //将首结点作为当前结点 
77.    student* node = head;
78.    //判断当前结点是否为NULL,如果不是NULL,则输出其指向的结构体数据 
79.    while(NULL!=node)
80.    {
81.       //利用当前结点结构体数据的指针访问其数据成员 
82.       printf("name: %s,score: %.2f\n",node->name,node->score);
83.       //将结点所指向的下一个结点作为当前结点 
84.       node = node->next;
85.    }
86. }

  ——陈良乔 ,《C程序设计伴侣》,人民邮电出版社,2012年10月,p234~237

【评析】

  这两段代码的功能分别建立链表和输出链表。
  首先考察一下第一段代码。

6. //定义表示学生信息结点的结构体 
7. typedef struct _student
8. {
9.    char name[20];
10.    float score;
11.    //定义指向链表中下一个结点的指针  
12.    struct _student* next;
13. }student;

   这段代码定义了链表结点的数据类型。这个类型定义得十分业余,具体表现在结点类型中有三个数据项,通常链表结点中只有两项,一项是结点存储的数据,另一项是指向下一结点的指针。下面写法要好得多

typedef struct 
{
   char name[20];
   float score;
}
student_t;

typedef struct node
{
   student_t data;
   //定义指向链表中下一个结点的指针  
   struct node * next;
}node_t;

   两者的差别何在呢?打个比方,前者就如同小学生写的算术式,而后者则相当于中学生写的代数式。代数式无疑比算术式更抽象,更难于把握,但也更有概括性,更具有一般性。小学生可能知道求一个正整数18的个位数可以用18除以10得到的余数8就是个位数,但中学生会说任何一个正整数n的个位数都是n%10。
将代码写得更抽象有什么好处呢?至少代码具有更好的可复用性和可维护性。这一点在后面将会进一步加以说明。

19.	  //第一步:定义指向链表头和尾的指针 
20.	  student* head = NULL ;
21.	  student* tail = NULL ;

  前面正常,后面变态。定义head,天经地义。然而为链表又定义一个tail,绝对是空前绝后之举。这程序设计大概是讲类人猿的生物老师学的,完全没有进化到人类阶段。我看到这个tail的第一反应就是“尾大不掉”和“拖泥带水”。

  须知,链表和字符串一样,都属于自包含结尾标志的,因此处理这样的数据,只需要一个“头”就足矣。
  据作者自己说,这样做的理由是“链表总是在末尾添加新的结点,所以我们要保存指向链表尾结点的指针”。但实际上,从来没有“链表总是在末尾添加新的结点”这种说法,这种说法完全是作者自己臆测的。而且,即使需要在链表尾部添加结点,也绝对不需要这个tail,因为从链表头出发总是能找到链表尾,这是链表自身的一个特性。

23.      char name[20] = "" ;
24.      float score = 0.0f ;

   这段代码有两个毛病,第一个是无缘无故地初始化,非常无聊。这是很多JAVAer和C++er给C语言带来的污染,因为JAVA和C++的类中经常需要初始化,但在C语言中这却是根本不必要的。
  这段代码的另一个毛病就是,在这里定义这两个变量本身说明作者完全是趴在地上写代码,根本没有站起来,因此缺乏全局观念。从全局看,程序主要对链表进行操作,因此main()中需要定义head。与之相比这里的name和score完全是无关紧要的临时变量,是鸡毛蒜皮。把鸡毛蒜皮和程序处理的主要数据对象head摆在同样的位置,只能说明这是胡子眉毛一把抓。 

26.	  bool ishead = true;

  什么叫“废话”,这就是。画蛇添足地加个tail不算,还要在头上贴个标签。没有人清楚到底这里的“head”是头还是“ishead”是头。

27.   while(scanf("%s %f",name,&score)!=EOF)

  这行代码有两个毛病。
  首先scanf()赤身裸体地就冲出来了,完全无视用户的心理体验。
  其次用scanf("%s %f",name,&score)!=EOF作为while语句的循环条件有很严重的安全漏洞。当用户输入不符合"%s %f"格式要求时(例如输入abc abc)由于scanf("%s %f",name,&score)!=EOF的值为1,循环体部分得到执行,会建立没有意义的结点,造成整个链表错乱。

29.	     //第二步“:根据结点数据创立结点 
30.	     //首先,用malloc()函数动态申请内存,内存的大小就是一个结点的大小 
31.	     student* node = malloc(sizeof(student));  

  这里有一个明显的漏洞,就是作者没有考虑malloc()返回值为NULL的情况。当申请内存失败时,后面的语句会产生一些无法预料的可怕后果。

33.         //然后将用户输入的数据保存到这个结点 
34.         strcpy(node->name,name);
35.         node->score = score;

  如果感觉不到这两条语句的笨重,说明对数据封装还缺乏起码的理解。这两句话其实本可以用一句简单的赋值表达式语句搞定,假如前面结点的数据结构设计为

typedef struct 
{
   char name[20];
   float score;
}
student_t;

typedef struct node
{
   student_t data;
   //定义指向链表中下一个结点的指针  
   struct node * next;
}node_t;

//……
  student_t data_temp ;
  
  while(scanf("%s %f",data_temp.name,&data_temp.score)!=EOF)
  {
     student* node = malloc(sizeof(student));  
     
     node->data = data_temp ;
     //……
     
  }

  不难发现,以后即使student_t类型有什么改动时,这里的node->data = data_temp ;根本无须修改。这就是“代数式”与“小学算术式”的根本区别。换句话说由于node->data = data_temp ;的普适性,它的可复用性更好,因而代码的可维护性也就更好。而样本中的写法,则是稍有变化就需要到处修改。 

37.         //第三步:调整节点之间的指向关系 
38.         //如果这是链表中的第一个结点。 
39.         if(ishead)
40.         {
41.             //将指向链表首结点的head指向这个结点 
42.             head = node ;
43.             //首结点尚无下一个结点 
44.             head->next = NULL ;
45.             //当前结点就是链表的尾结点 
46.             tail = node;
47.             
48.             //首结点已经处理完毕,下一个结点就是普通结点了 
49.             ishead = false;
50.         }
51.         else
52.         {
53.             //将新结点添加到已有链表的末尾 
54.             tail->next = node;
55.             //将新的结点作为新的尾结点 
56.             tail = node;
57.         }

  前面说过ishead多余,因为这里的if(ishead)完全可以用if(head==NULL)替代。
  这里不难注意到if和else分支都有tail=node;所以if语句可以简化为

     if(head==NULL)
     {
         head = node ;
         head->next = NULL ;
     }
     else
     {
         tail->next = node;
     }
     tail = node ;

  相信大家现在不难看出head->next = NULL ;这句非常无厘头了吧?链表的尾部结点的next应该是NULL,把头结点的next赋值为NULL纯粹属于思维混乱。
  所以这句应该删除:

    if(head==NULL)
     {
         head = node ;
     }
     else
     {
        tail->next = node;
     }
     tail = node ;

  当然,更合理的写法应该是

     if(head==NULL)
     {
         head = node ;
     }
     else
     {
         tail->next = node;
     }
     tail = node ;
     tail->next = NULL;

  这样每次添加结点,都会形成一个全须全尾的完整的链表。

58.      }//第四步:重复第二步和第三步,逐个添加结点
59.      //这里利用一个while循环重复第二步和第三步。
60.      //直到用户用"Ctrl+z"结束数据输入为止 

  这个说法是错误的,只是在某些操作系统下,键入Ctrl+Z才会使得scanf()返回值为EOF,在有些操作系统下则不是。这从另一个方面坐实了最初scanf("%s %f",name,&score)!=EOF这个条件的设计不当。

62.      //将尾结点的next设置为NULL,表示这是链表的结束 
63.      if(NULL!=tail)
64.          tail->next = NULL;
65.      else
66.          return -1;

  这段代码更滑稽。滑稽之处在于作者不在前面while语句内部生成完整的链表,却跑到while语句之后去“擦屁股”。但是,且慢,作者突然发现如果第一次输入结点数据时若直接输入Ctrl+Z,那么while语句直接结束,可此时tail居然还是NULL。在这种情况下tail->next显然没有意义,作者无计可施,于是仓皇出逃:return -1;。直接宣布他无法建立空链表。这里的“-1”是不知所云的写法,因为通常main()的返回值应该为0或1。
  实际上呢,作者也实在是有些杯弓蛇影草木皆兵过于露怯了。这里根本不用慌忙逃窜,只需要把if语句写成

 if(NULL!=tail)
      tail->next = NULL;

  就可以了。翁同龢老师早就说过“每临大事有静气”,这句话实在是应该刻在匾上,挂在每一所IT学校的大门口。这样至少可以让某些不学无术的人少露些怯。

  逐字逐句地评析完了这段代码,但其实前面指出的这些毛病还不是最严重的。这段代码最严重的问题是在main()中建立链表。这绝对是刚学编程的童鞋们的一种风范——一main()到底,坦荡荡地宣布他不懂结构化程序设计,不会用函数创建链表。这种毛病,在拙著《品悟C》P253页第7章问题4<将main()函数进行到底>有详细剖析和阐述,这里就不再重复了。

  讲完了第一段代码,再讲一下第二段输出链表的代码。
  这段代码居然是用一个函数实现的,这点值得表扬。
  这段代码的其实很容易写,按说不应该出什么毛病。但居然,还是有毛病。毛病出在 

74.    void printlist(student* head)
75.    {
76.       //将首结点作为当前结点 
77.       student* node = head;

   在这里,node获得了head的值。但是在后面的代码中

79.       while(NULL!=node)
80.       {
81.          //利用当前结点结构体数据的指针访问其数据成员 
82.          printf("name: %s,score: %.2f\n",node->name,node->score);
83.          //将结点所指向的下一个结点作为当前结点 
84.          node = node->next;
85.       }
86.    }

  一直在使用node却没有使用head。也就是说有两个同值的变量,一个使用了,另一个没使用。绝对是一副“等咱有了钱,买两菜包子,吃一个扔一个”的大款气派。
  实际上这两个变量中有一个是多余的,代码应该写为

void printlist(student* node) 
{

   while(NULL!=node)
   {
      printf("name: %s,score: %.2f\n",node->name,node->score);
      node = node->next;
   }
}

  

  写代码,一定不要忘记带上Occam's Razor——Entities should not be multiplied unnecessarily.


 

posted @ 2012-11-21 23:35  garbageMan  阅读(3193)  评论(41编辑  收藏  举报