追求代碼質量（4）: 用代碼度量進行重構

jopen 10年前發布 | 37K 次閱讀 代碼質量 代碼分析/審查/優化

原文出處： IBM中國

在我上中學的時候，有一位英語教師說：“寫作就是重寫別人已經 重寫過的東西。” 直到大學，我才真正理解了他這句話的意思。而且，當我自覺地采用這個實踐的時候，就開始喜歡上了寫作。我開始為我寫的東西自豪。我開始真正在意我的表達方式和要傳達的內容。

當我開始開發人員生涯時，我喜歡閱讀有經驗的專家編寫的技術書籍，而且想知道為什么他們花這么多時間編寫代碼。那時，編寫代碼看起來是件容易的工作 —— 有些人（總是比我級別高的人）會給我一個問題，而我會用任何可行的方法解決它。

直到我開始與其他開發人員合作大型項目，才開始理解我的技能的真正意義所在。我也就在這個時候起，開始有意識地關心我編寫的代碼，甚至關心起其他人 編寫的代碼。現在我知道了，如果不注意代碼質量，那么遲早它們會給我造成一團亂麻。

我恍然大悟 的一刻出現在 1999 年底，那時我正在閱讀 Martin Fowler 那本影響重大的書Refactoring: Improving the Design of Existing Code（重構：改進現有代碼的設計，這本書對一系列重構模式進行分類，并由此建立了重構的公共詞匯。在此之前，我一直都在重構我的代碼（或者其他人的代碼），但是卻不知道自己做的就是重構。現在，我開始為我編寫和重構的代碼感到更加自豪，因為我做的工作正是在促進代碼的編寫方式并讓它們日后更易維護。

什么是重構？

按照我的觀點，重構就是改進已經改進的 代碼的行為。實際上，重構是個永不停止的代碼編寫過程，它的目的是通過結構的改進而提高代碼體的可維護性，但卻不 改變代碼的整體行為。重要的是要記住重構與重寫 代碼明顯不同。

重寫代碼會修改代碼的行為甚至合約，而重構保持對外接口不變。對于重構方法的客戶機來說，看不到區別。事情像以前一樣工作，但是工作得更好，主要是因為增強的可測試性或者明顯的性能提升。

主動和被動重構

那么問題就變成了“我怎么才能知道什么時候該進行重構呢？” 一段代碼的可維護性是個主觀的問題。但是，我們中的多數人都會發現，維護自己編寫的代碼要比維護其他人編寫的代碼容易得多。但在這點上也有爭議 —— 在整個職業生涯中維護自己的代碼是最大挑戰。沒有幾個真正的 “代碼牛仔” 足夠幸運地能夠不斷地變換工作，而不必修改其他人的代碼。對于我們中的多數人來說，必須維護其他人的代碼恰恰是程序員生活的一部分。決定代碼是否需要重構的方法，通常是主觀的。

但是，也有可能客觀地判斷代碼是否應當重構，不論是自己的代碼還是別人的代碼。在 這個系列前面的文章中，我介紹了如何用代碼度量客觀地測試代碼質量。實際上，可以用代碼度量很容易地找出可能難以維護的代碼。一旦客觀地判斷出代碼中有問題，那么就可以用方便的重構模式改進它。

提取方法模式

Martin Fowler 的書出版之后的幾年中，增加了許多新的重構模式分類；但是，迄今為止最容易學習的模式，也可能是最有效的模式，仍然是提取方法（Extract Method） 模式。在這個模式中，方法的一個邏輯部分被移除，并被賦予自己的方法定義。現在被移走的方法體被新方法的調用代替，如圖 1 的 UML 圖所示：

提取方法模式提供了兩個關鍵好處：

  • 原來的方法現在更短了，因此也更容易理解。
  • 移走并放在自己方法中的邏輯體現在更容易測試。

降低圈復雜度

在使用的時候，對于被高度圈復雜度值感染的方法來說，提取方法是一劑良藥。您可能會記得，圈復雜度通過度量方法的路徑數量；所以，可以認為如果提取 出其中一些路徑，重構方法的整體復雜性會降低。

例如，假設在運行了像 PMD 這樣的代碼分析工具之后，結果報告顯示其中一個類包含的一個方法有較高的圈復雜度值，如圖 2 所示：

圖 2. 圈復雜度值高達 23！

在仔細查看了這個方法之后，發現這個方法過長的原因是使用了太多的條件邏輯。正如我以前在這個系列中指出的（請參閱 參考資料），這會增加方法中產生缺陷的風險。謝天謝地，updateContent()方法還有個測試用例。即使已經認為這個方法有風險，測試也會減輕一些 風險。

另一方面，測試已經精心地編寫成可以測試updateContent()方法中的 23 個路徑。實際上，好的規則應當是：應當編寫至少 23 個測試。而且，要想編寫一個測試用例，恰好能隔離出方法中的第 18 個條件，那將是極大的挑戰！

小就是美

是否真的要測試長方法中的第 18 個條件，是個判斷問題。但是，如果邏輯中包含真實的業務值，就會想到測試它，這個時候就可以看到提取方法模式的作用了。要把風險降到最小很簡單，只需把條件邏輯分解成更小的片段，然后創建容易測試的新方法。

例如，updateContent()方法中下面的這小段條件邏輯創建一個狀態String。如清單 1 所示，邏輯的隔離看起來足夠簡單：

清單 1. 條件邏輯成熟到可以進行提取 
//...other code above

String retstatus = null;
if ( lastChangedStatus != null && lastChangedStatus.size() > 0 ){
 if ( status.getId() == ((IStatus)lastChangedStatus.get(0)).getId() ){
  retstatus = "Change in Current status";
 }else{
  retstatus = "Account Previously Changed in: " + 
    ((IStatus)lastChangedStatus.get(0)).getStatusIdentification();
 }
}else{
  retstatus = "No Changes Since Creation";
}

//...more code below

通過把這一小段條件邏輯提取到簡潔的新方法中（如清單 2 所示），就做到了兩件事：一，把updateContent()方法的整體復雜性降低了 5；二，邏輯的隔離很完整，可以容易地對它進行測試。

清單 2. 提取方法產生 getStatus 
private String getStatus(IStatus status, List lastChangedStatus) {
  String retstatus = null;
  if ( lastChangedStatus != null && lastChangedStatus.size() > 0 ){
    if ( status.getId() == ((IStatus)lastChangedStatus.get(0)).getId() ){
      retstatus = "Change in Current status";
    }else{
      retstatus = "Account Previously Changed in: " + 
        ((IStatus)lastChangedStatus.get(0)).getStatusIdentification();
    }
  }else{
    retstatus = "No Changes Since Creation";
  }
  return retstatus;
}

現在可以把updateContent()方法體中的一部分替換成對新創建的getStatus()方法的調用，如清單 3 所示：

清單 3. 調用 getStatus 
//...other code above

String iStatus = getStatus(status, lastChangedStatus);

//...more code below

請記住運行現有的測試，以驗證什么都沒被破壞！

測試私有方法

您將注意到在 清單 2 中定義的新getStatus()方法被聲明為private。這在想驗證隔離的 方法的行為的時候就形成了一個有趣的挑戰。有許多方法可以解決這個問題：

  • 把方法聲明成public。
  • 把方法聲明成protected，并把測試用例放在同一個包中。
  • 在父類中建立一個內部類，這個內部類是個測試用例。

還有另一個選擇：保留方法現有的聲明不變（即private），并采用優秀的 JUnit 插件項目來測試它。

PrivateAccessor 類

JUnit 插件項目有一些方便的工具，可以幫助 JUnit 進行測試。其中最有用的一個就是PrivateAccessor類，它把對private方法的測試變成小菜一碟，無論選擇的測試框架是什么。PrivateAccessor類對 JUnit 沒有顯式的依賴，所以可以把它用于任何測試框架，例如 TestNG。

PrivateAccessor的 API 很簡單 —— 向invoke()方法提供方法的名稱（作為String）和方法對應的參數類型和相關的值（分別在Class和Object數組中），就會返回被調用方法的值。在幕后，PrivateAccessor類實際上利用 Java 的反射 API 關閉了對象的可訪問性。但是請記住，如果虛擬機有定制的安全性設置，那么這個工具可能無法正確工作。

在清單 4 中，調用getStatus()方法時兩個參數值都設置為null。invoke()方法返回一個Object，所以要轉換成String。還請注意invoke()方法聲明它要throws Throwable，必須捕獲異常或者讓測試框架處理它，就像我做的那樣。

清單 4. 測試私有方法 
public void testGetStatus() throws Throwable{
  AccountAction action = new AccountAction();

  String value = (String)PrivateAccessor.invoke(action,
      "getStatus", new Class[]{IStatus.class, List.class},
       new Object[]{null, null});

  assertEquals("should be No Changes Since Creation", 
    "No Changes Since Creation", value);
}

請注意invoke()方法被覆蓋成可以接受一個Object實例（如清單 4 所示）或一個Class（這時期望的private方法也是static的）。

還請記住，使用反射調用private方法會對生成的結果帶來一定程度的脆弱性。如果有人改變了getStatus()方法的名字，以上測試就會失敗；但是，如果經常測試，就可以迅速地進行適當的修正。

結束語

在抗擊圈復雜度時，請記住大部分編寫到應用程序中的路徑是應用程序的整體行為所固有的。也就是說，很難顯著地減少路徑的整體數量。重構只是把這些路徑放在更小的代碼段中，從而更容易測試。這些小的代碼段也更容易維護。

參考資料

學習

獲得產品和技術

  • JUnit-addons：對于使用 JUnit 的開發人員有用的庫。
  • PMD：掃描 Java 代碼的問題。

討論

 本文由用戶 jopen 自行上傳分享，僅供網友學習交流。所有權歸原作者，若您的權利被侵害，請聯系管理員。
 轉載本站原創文章，請注明出處，并保留原始鏈接、圖片水印。
 本站是一個以用戶分享為主的開源技術平臺，歡迎各類分享！