垃圾“程序是怎样练成的”——关于《C程序设计伴侣》第A章（四）

前文链接：http://www.cnblogs.com/pmer/archive/2012/12/13/2817180.html

【样本】

 

 

【评析】

　　边施工边设计还不算，更雷人的是最后才考虑“组装”。这是典型的“自底向上”而非结构化程序设计所提倡的“自顶向下”。一方面作者夸大其词地胡扯什么“在C语言程序设计当中，“自顶向下，逐步求精”就像一句具有魔力的咒语，只要我们一念这个咒语，任何负责困难的问题都会迎刃而解”(P40)，另一方面又在编码时践踏“自顶向下”这条原则。这是典型的打着红旗反红旗。

【样本】

 

 

【评析】

　　自从dijkstra指出goto有害之后，这种耗子窝流程图就难得一见了。各位，赶紧出来瞧鼠窝。
　　怎么看怎么像挖了个大坑，然后跳了下去，把自己给埋了起来而出不来，然后不得已又挖了一个洞才好不容易钻了出来。

【样本】

 

 

【评析】

　　有这样的流程图，代码混乱、复杂也就不足为奇了。
　　首先

   while(true)
   {
      /*……*/
      if(strlen(wd)>0)
      {
         /*……*/
      }
      else
      {
         break;
      }
   }

 

　　这种结构很糟糕，还不如写成

 

  while(true)
   {
      /*……*/
      if(strlen(wd)>0)
      {
         /*……*/
         continue;
      }
      break;
   }

 

　　写出如此糟糕结构最重要的原因在于作者不懂得如何定义变量，把wd这个char [30]定义在了while语句的循环体内部，然而矛盾的是这个while语句是用来处理wd的。这让人想起了一个笑话，一个笨婆娘缝被子，结果把自己给缝进去被子里面去了。这里的wd就是如此。本应该定义在while语句之外，却被“缝”在了循环体内。最后想出来也只好使用和笨婆娘一样的最后招数——“break”了。
　　如果把wd定义在while语句之外，while语句就简洁多了。

   char wd[30] ;
   while( text = cutword(text,wd) , strlen(wd)>0 )
   {
      file->total +=1;
      word* exist = findnode(file->list,wd);
      if( NULL == exist)
      {
         word* node = createnode(wd);
         if(NULL == pre)
         {
            file->list = node;
         }
         else
         {
            pre->next = node;
         }
         pre = node;
      }
      else
      {
         exist->count +=1;
      }
   }

 

　　样本中的strlen(wd)>0也是一种拙劣的写法，因为它等价于 *wd !='\0'。每次循环进行一次函数调用和每次循环只进行一次“!=”运算，效率显然是天壤之别。
　　样本中的NULL == pre在逻辑上是错误的，应该写成NULL == file->list。
　　除此之外

         if(NULL == pre)
         {
            file->list = node;
         }
         else
         {
            pre->next = node;
         }
         pre = node;

 

是一种很笨拙的写法。这种写法把链表的head和结点的next成员区别开来，在这里必然要写个if-else语句。更简洁的写法的基础是把两者视为同一种东西，这样就可以统一对待了。造成这种笨拙的另一个原因是作者僵化固执地把新结点加在链表结尾，实际上这样做毫无必要。
　　综上所述，这个函数可以更好地写为

void parseword(txtfile* file)
{
   char* text = file->text;
   file->list = NULL;
   file->total = 0;
   char wd[30] ;

   cleantext(text);
   while( text = cutword(text,wd) , *wd != '\0' )
   {
      file->total +=1;
      word* exist = findnode(file->list,wd);
      if( NULL == exist)
      {
         word* node = createnode(wd);
         node->next = file->list;
         file->list = node ;
      }
      else
      {
         exist->count +=1;
      }
   }
}  

 　　当然，代码中还有其他毛病，但这些毛病与程序的总体思路的错误或其他函数相关，在这里没办法进一步纠正。