京東資深架構師程式碼評審歪詩

weixin_33912246發表於2018-05-10
架構

賈言

架構師說, 用20個字描述程式碼評審的內容， 自省也省人。由於是一字一含義， 不連貫， 為了增強趣味性， 每句都增加對應的歪解。只是對常見評審的描述， 不盡之處，歡迎補充!

驗幻空越重 -- 言歡空月蟲

驗： 公共方法都要做引數的校驗，引數校驗不通過明確丟擲異常或對應響應碼。

  • java bean驗證已經是一個很古老的技術了，會避免我們很多問題，可參考：http://beanvalidation.org/ http://www.infoq.com/cn/news/2010/03/javaee6-validation https://www.sitepoint.com/using-java-bean-validation-method-parameters-return-values/
  • 在介面中也明確使用驗證註解修飾引數和返回值， 作為一種協議要求呼叫方按驗證註解約束傳參， 返回值驗證註解約束提供方按註解要求返回引數

幻： 在程式碼中要杜絕幻數，幻數可定義為列舉或常量以增強其可讀性

空： 要時刻警惕空指標異常

  • 常見的 a.equals(b) 要把常量放到左側
  • aInteger == 10 如果 aInteger 為空時會丟擲空指標異常
  • 不確認返回集合是否可為空時要做非空判斷， 再做for迴圈
  • 使用空物件模式， 約定返回空集合， 而非null
  • 使用StringUtils判斷字串非空

越： 如果方法傳入陣列下標作為引數，要在一開始就做下標越界的校驗，避免下標越界異常

重： 不要寫重複程式碼，重複程式碼要使用重構工具提取重構

命循頻異長 - 明勳品宜昌

命： 包 / 類 / 方法 / 欄位 / 變數 / 常量的命名要遵循規範，要名副其實， 這不但可以增加可讀性，還可以在起名的過程中引導我們思考方法 / 變數 / 類的職責是否合適

有意義很重要， 典型無意義命名：

public static final Integer CODE_39120 = 39120; 
public static final String MESSAGE_39120 = "[包裹]與[庫房號]不一致，確定裝箱？"; 
  
public static final Integer CODE_39121 = 39121; 
public static final String MESSAGE_39121 = "[包裹]與[箱號]的承運型別不一致,確定裝箱？"; 
  
Rule rule1 = request.getRuleMap().get("1050");

CODE_39120這個名字和幻數沒多大區別。

循： 不要在迴圈中呼叫服務，不要在迴圈中做資料庫等跨網路操作

頻： 寫每一個方法時都要知道這個方法的呼叫頻率，一天多少，一分多少，一秒多少，峰值可能達到多少，呼叫頻率高的一定要考慮效能指標， 考慮是否會打垮資料庫，是否會擊穿快取

異： 異常處理是程式設計師最基本的素質，不要處處捕獲異常，對於捕獲了只寫日誌，沒有任何處理的 catch 要問一問自己，這樣吃掉異常，是否合理

下面是一個反例， 在匯出檔案的controller方法中做了兩層的try...catch， 在catch塊中記錄日誌後什麼都沒做， 這樣使用者看不到真正想要的內容， 研發也只有看日誌才能發現錯誤， 而“看日誌”， 通常只有業務方反饋問題時才會看， 就會導致研發人員發現錯誤會比現場人員還會晚。

@RequestMapping(value = "/export") 
public void export(CityRelationDomain condition, HttpServletResponse response) { 
   ZipOutputStream zos = null; 
   BufferedWriter bufferedWriter = null; 
   try { 
      condition.setStart(0); 
      condition.setSize(MAX_EXPORT_LINES); 
      List<CityRelationDomain> list = cityRelationService.getOrdersByCondition(condition); 
      response.setCharacterEncoding("GBK"); 
      response.setContentType("multipart/form-data"); 
      response.setHeader("Content-Disposition", "attachment;fileName=export.zip"); 
      zos = new ZipOutputStream(response.getOutputStream()); 
      bufferedWriter = new BufferedWriter(new OutputStreamWriter(zos, "GBK")); 
      bufferedWriter.write("訂單型別編碼,始發城市-省,始發城市-市,目的城市-省,目的城市-市"); 
      ZipEntry zipEntry = new ZipEntry("export.csv"); 
      zos.putNextEntry(zipEntry); 
      for (CityRelationDomain domain : list) { 
         try { 
            bufferedWriter.newLine(); 
            bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getOrderCode())); 
            bufferedWriter.write(','); 
            bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getProvinceNameFrom())); 
            bufferedWriter.write(','); 
            bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getCityNameFrom())); 
            bufferedWriter.write(','); 
            bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getProvinceNameTo())); 
            bufferedWriter.write(','); 
            bufferedWriter.write(CSVExportUtil.trans2CSV(domain.getCityNameTo())); 
         } catch (Exception e) { 
            e.printStackTrace(); 
         } 
      } 
      bufferedWriter.newLine(); 
           bufferedWriter.flush(); 
           zos.closeEntry(); 
           bufferedWriter.close(); 
   } catch (Exception e) { 
      e.printStackTrace(); 
      logger.error("匯出CSV檔案異常"); 
   } finally { 
      try { 
         if (zos != null) { 
            zos.close(); 
         } 
         if (bufferedWriter != null) { 
            bufferedWriter.close(); 
         } 
      } catch (IOException e) { 
         e.printStackTrace(); 
      } 
   } 
}

長： 如果一行程式碼過長，要分解開來;如果一個方法過長，要重構方法;如果一個類過長要考慮拆分類

依輪線日簡 - 依倫先日賤

依： 如果呼叫了外部依賴， 一定要搞清楚這個外部依賴可以提供的效能指標，最好約定 SLA

輪： 不要重複造輪子，如果已經有成熟類庫實現了類似功能，要優先使用成熟類庫的方法，這是因為成熟類庫中的方法都經過很多人的測試驗證，通常情況下我們自己實現的質量最大等同於成熟類庫的質量。

線： 要注意我們的 jsf 服務，web 應用，消費訊息的 worker 都是多執行緒環境，要注意執行緒安全問題，最典型的 HashMap，SimpleDateFormat ，ArrayList 是非執行緒安全的，另外如果使用 Spring 自動掃描服務，那麼這個服務預設是單例，其內部成員是多個執行緒共享的，如果直接用成員變數是有執行緒不安全的。

兩個典型的錯誤程式碼片段：

  無視 SimpleDateFormat 非執行緒安全

@Service public class AService { private static final SimpleDateFormat FORMAT = new SimpleDateFormat("yyyy-MM-dd"); public void doSomething() { //use FORMAT } }

  使用 Service 成員變數

@Service 
public class BService { 
    private Pojo b; 
  
    public void doB() { 
         b = getB(); 
         process(b); 
    } 
}

日： 列印日誌和設定合理的日誌級別，如有必要要新增 if 條件限定是否列印日誌，在日誌中使用 JSON 序列化，生成長字串的 toString() 都要做 if 限定列印，否則配置的日誌級別沒達到，也會做大量字串拼接，佔用很多 gc 年輕代記憶體. 另外一定要通過log4j列印日誌而不是直接把日誌列印到控制檯。

典型錯誤示例：

@Service 
public class FooService { 
    private static final Logger LOGGER = LoggerFactory.getLogger(FooService.class); 
  
    public void doFooThing(Foo foo) { 
        LOGGER.debug("get parameter foo {}", JSONObject.toString(foo)); 
        try {/*do something*/} catch (Exception ex) {ex.printStackTrace();} 
    } 
}

簡： 儘可能保持整體設計的簡潔，方法實現的簡潔，要根據情況使用記憶體快取，redis 快取，jmq 非同步處理。這裡的簡需要把握好分寸。

接偶正分壯 - 潔偶正粉妝

接： 介面是用來隔離變化的，如果一個業務有幾種不同的形態，但都有相同的處理，那麼可以定義介面來隔離業務形態的不同，在服務呼叫處，通過業務型別欄位來獲得不同的服務類。而不要實現一個類，然後在類的各個方法中都根據業務型別做 if else 或更復雜的各種判斷。

典型示例：

做法1：

public interface BarService {    void doBarThing(Bar b); 
      
    void doBarFatherThing(Bar b); 
} 
public class BarServiceImpl  implement BarService{ 
    public void doBarThing(Bar b) { 
        if (b.getType() == BarType.A) { 
            //do some logic 
        } else (b.getType() == BarType.B) { 
            //do some B type logic 
        } 
        //do other doBarThing logic 
    } 
      
    public void doBarFatherThing(Bar b) { 
        if (b.getType() == BarType.A) { 
            //do some logic 
        } else (b.getType() == BarType.B) { 
            //do some B type logic 
        } 
        //do other doBarFatherThing logic 
    } 
}

做法2：

public interface BarService { 
    void doBarThing(Bar b); 
      
    void doBarFatherThing(Bar b); 
} 
public class BarServiceFactory { 
    public BarService getBarService(BarType type) { 
        // get bar service logic 
    } 
} 
//如果有公共邏輯就定義， 沒有就不定義 
public class BaseBarService implement BarService { 
    public void doBarThing(Bar b) { 
        //do other doBarThing logic 
    } 
      
    public void doBarFatherThing(Bar b) { 
        //do other doBarFatherThing logic 
    } 
      
} 
public class TypeABarService extends BaseBarService  implement BarService { 
    public void doBarThing(Bar b) { 
        // doATypeThing 
        super.doBarThing(b); 
    } 
      
    public void doBarFatherThing(Bar b) { 
        // do bar type A service 
super.doBarFatherThing(b); //如果需要就呼叫， 不需要就不呼叫父類 
    } 
      
}

做法 2 的好處是將不同型別的邏輯解耦，各自發展，不會相互影響，如果新增型別也不必影響現有型別邏輯。

偶： 認識系統之間的耦合關係，通過同步資料來做兩個系統之間的互動是一種很強的耦合關係，會使資料接收方依賴於資料傳送方的資料庫定義，如果傳送方想改資料結構，必須要求下游接收方一起修改;通過介面呼叫是一種常見的系統耦合關係，介面的提供方要保證介面的可用性，介面的呼叫方要考慮介面不可用時的應對方案; mq 訊息是一種解耦的方法，兩個系統不存在實時的耦合關係。但是 mq 解耦的方式不能濫用，在同一系統內不宜過多使用 mq 訊息來做非同步，要儘可能保證介面的性

能， 而不是通過 mq 防止出問題後重新消費。

正： 模組之間依賴關係要正向依賴，不能讓底層模組依賴於上層模組;不能讓資料層依賴於服務層也不能讓服務層依賴於 UI 層; 也不能在模組之間形成迴圈依賴關係。

分： 分而治之，複雜的問題要分解成幾個相對簡單的問題來解決，首先要分析出核心問題， 然後分析出核心的入參是什麼，結果是什麼，入參通過幾步變化可以得出結果。

壯： 時刻注意程式的健壯性，從兩個方面實踐提升健壯性：

  • 契約，在設計介面時定義好協議引數，並在實現時第一時間校驗引數，如果引數有問題，直接返回給呼叫方; 如果出現異常情況， 也按異常情況約定應對策略
  • 考慮各種邊界條件的輸出，比如運單號查詢服務， 要考慮使用者輸入錯誤運單時怎麼返回，有邊界的查詢條件，如果使用者查詢條件超過邊界了， 應該返回什麼
  • 為失敗做設計，如果出問題了有降級應對方案。

作者：趙玉開，十年以上網際網路研發經驗，2013年加入京東，在運營研發部任架構師，期間先後主持了物流系統自動化運維平臺、青龍資料監控系統和物流開放平臺的研發工作，具有豐富的物流系統業務和架構經驗。在此之前在和訊網負責股票基金行情繫統的研發工作，具備高併發、高可用網際網路應用研發經驗。

【本文來自51CTO專欄作者張開濤的微信公眾號(開濤的部落格)，公眾號id: kaitao-1234567】

 

相關文章