垃圾“程序是怎样练成的”——关于《C程序设计伴侣》第A章(五)
前文链接:http://www.cnblogs.com/pmer/archive/2012/12/15/2819274.html
【样本】
// 表示文件的结构体 typedef struct _txtfile { char name[128]; // 文件名 char text[1024*128]; // 文件的文本内容 word* list; // 保存单词的链表 int total; // 单词总数 float correlation; // 关键词在文件中的词频 } txtfile;
——陈良乔 ,《C程序设计伴侣》,人民邮电出版社,2012年10月,p283
【评析】
“将词频添加到txtfile结构体中”是个愚蠢的想法,因为后面要根据词频进行排序,而txtfile中text成员是个极其庞大、笨重的char [1024*128]类型的数据对象。这就跟让一群人抱着水缸但却让他们按的个头高低排好队一样。
【样本】
——陈良乔 ,《C程序设计伴侣》,人民邮电出版社,2012年10月,p283
【评析】
数据结构反反复复一点谱都没有,居然不以为耻反以为荣。
看到“对于txtfile结构体的不断扩展,也反映了C语言面向结构程序设计中的‘逐步求精’的过程”这句话,任何懂得编程的人恐怕都会产生一种扔臭鸡蛋的冲动。但是这本书的阅读对象是并不懂得编程小朋友。悲夫?!
【样本】
// 计算词频模块的实现 // 参数files和count是保存txtfile结构体的数组指针和元素个数, // keyword是要计算词频的关键词 void countkeyword(txtfile* files,int count,char* keyword) { // 利用for循环,计算关键词在每一个文件中的词频 for(int i = 0; i < count;++i) { // 在当前文件中查找关键词结点 word* keynode = findnode(files[i].list,keyword); // 如果找到结点,则计算词频 if(NULL != keynode) { // 利用单词的个数除以文件的单词总数计算词频 files[i].correlation = keynode->count/(float)files[i].total; } else // 如果没有找到,词频为0 { files[i].correlation = 0.0f; } } }
——陈良乔 ,《C程序设计伴侣》,人民邮电出版社,2012年10月,p284
【评析】
本想这回可以不用说什么了。可一不小心看到了第二行注释中的“参数files和count是保存txtfile结构体的数组指针和元素个数”。“数组指针”不知所云。
【样本】
#include <stdio.h> #include <stdlib.h> #include <string.h> #include <ctype.h> //... int main() { // 文件读取以及数据预处理… // 无限循环,提供用户查询交互 while(true) { puts("please input the keyword:"); char keyword[30] = ""; // 获取用户输入的关键词 scanf("%s",keyword); // 如果用户输入的是“#”,则表示查询结束退出循环 if(0 == strcmp(keyword,"#")) break; // 计算用户输入的关键词在各个文件中的词频 countkeyword(files,filecount,keyword); } return 0; }
——陈良乔 ,《C程序设计伴侣》,人民邮电出版社,2012年10月,p284~285
【评析】
“我们还需要在主函数中构造一个无限循环,让用户输入关键词”是奇葩。因为最初问题的要求是“现在要求输入一个关键词”。
自底向上地写代码最终往往得到的就是这么个效果:本来说是要画美人的,结果画成了张飞。
代码中
char keyword[30]="";
中的初始化多余,30是MagicNumber。
scanf("%s",keyword);
中存在危险的漏洞。这句话应该写为
scanf("30%s",keyword);
文雅一点的写法是
#define MAX_LEN 30 #define S_(X) #X #define S(X) S_(X) scanf(S(MAX_LEN )"%s",keyword);
if(0 == strcmp(keyword,"#")) break;
也是随心所欲的胡写,因为在最初的要求中根本就没这一条。
countkeyword(files,filecount,keyword);
这里的filecount和files没有关系,非常滑稽。
这条语句应该写为
countkeyword(files,sizeof files/sizeof *files,keyword);
【样本】
#include <math.h> // … // 比较规则函数 int cmp(const void* a,const void* b) { // 将void*类型的参数转换为实际的txtfile*类型 const txtfile* file1 = (txtfile*)a; const txtfile* file2 = (txtfile*)b; // 比较txtfile结构体的词频 if(fabs(file1->correlation - file2->correlation) < 0.001) { return 0; } else if(file1->correlation > file2->correlation) { return 1; } else { return -1; } } // 文件排序模块的实现 void sortfiles(txtfile* files,int count) { // 调用qsort()函数对数组进行排序 qsort(files,count,sizeof(txtfile),cmp); } // 数据输出模块的实现 // 参数files和count是保存txtfile结构体数据的数组, // keyword是本次查询的关键词 void printfiles(txtfile* files,int count,char* keyword) { // 输出本次查询的关键词 printf("the keyword is \"%s\"\n",keyword); // 输出这个关键词在各个文件的词频 puts("the correlations are "); for(int i = 0; i < count;++i) { printf("%s %.4f\n",files[i].name,files[i].correlation); } }
——陈良乔 ,《C程序设计伴侣》,人民邮电出版社,2012年10月,p284
【评析】
“根据问题的要求,我们需要根据各个文件的词频大小,对保存在数组中的txtfile文件结构体进行排序”这个想法很愚蠢,因为问题的要求并非如此,而且只需要根据词频对文件名排序就可以了,然而txtfile(不知道啥叫“txtfile文件结构体”)其他部分一同参与排序简直是吃饱了撑的。
代码部分:
cmp()函数形参名称很垃圾。
if(fabs(file1->correlation - file2->correlation) < 0.001)
这样的写法是食古不化,不仅毫无必要,而且弄出了一个MagicNumber 0.001。这里其实可以直接写
if( file1->correlation == file2->correlation )
另外这里的结构也可以写得更简单些
if( file1->correlation == file2->correlation ) { return 0; } if(file1->correlation > file2->correlation) { return 1; } return -1;
sortfiles()函数像没用的裹脚布一样包在了qsort()上,使得qsort()的cmp实参变得不明不白。应该直接调用qsort()函数。
【样本】
while(true) { // 输入关键词… // 计算关键词在各个文件中的词频 countkeyword(files,filecount,keyword); // 按照关键词在各个文件中的词频,对文件进行排序 sortfiles(files,filecount); }
——陈良乔 ,《C程序设计伴侣》,人民邮电出版社,2012年10月,p286
【评析】
非但笨拙,而且脱离了问题的要求。filecount和files毫无关联的错误同前,不再赘述。