垃圾“程序是怎樣練成的”——關於《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;
      }
   }
}  

 　　當然，代碼中還有其他毛病，但這些毛病與程序的總體思路的錯誤或其他函數相關，在這里沒辦法進一步糾正。