垃圾代码评析——关于《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”是头。
这行代码有两个毛病。
首先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.