垃圾代码评析——关于《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.

 

转载于:https://www.cnblogs.com/pmer/archive/2012/11/21/2781703.html