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

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