如何進行CodeView

關于Code Review的重要性，我相信好的工程師都能認識到。 參考 "讓Code Review稱為一種習慣" 和 "從Code Review談如何做技術"。

同時引用一下有人對Google Code Review的描述：

The biggest thing that makes Google’s code so good is simple: code review. At Google, no code, for any product, for any project, gets checked in until it gets a positive review.

Code Review 主要Revivew什么

Architecture/Design

單一職責原則.

這是經常被違背的原則。一個類只能干一個事情, 一個方法最好也只干一件事情。 比較常見的違背是處理促銷的時候，同時處理驗券邏輯。

行為是否統一

比如緩存是否統一，錯誤處理是否統一， 日志打印是否統一， 方法實現是否統一等等。

同一邏輯/同一行為 有沒有走同一Code Path？低質量程序的另一個特征是，同一行為/同一邏輯，因為出現在不同的地方或者被不同的方式觸發，沒有走同一Code Path 或者各處有一份copy的實現， 導致非常難以維護。

代碼污染

使用魔數，例如直接比對數字。編寫不容易閱讀的代碼，應用了很多復雜的設計。

重復代碼

主要看有沒有把公用組件，可復用的代碼，函數抽取出來。

Open/Closed 原則

就是好不好擴展。 Open for extension, closed for modification.（做好接口設計會很好解決這些問題）。

面向接口編程 和 不是 面向實現編程

主要就是看有沒有進行合適的抽象， 把一些行為抽象為接口。

健壯性

對Corner case有沒有考慮完整，邏輯是否健壯？有沒有潛在的bug？

有沒有內存泄漏？有沒有循環依賴?（針對特定語言，比如Objective-C) ？有沒有野指針？

有沒有考慮線程安全性， 數據訪問的一致性

錯誤處理

有沒有很好的Error Handling？比如Price接口出錯。

有沒有增加必要的監控，及時報警。

改動是不是對代碼的提升

新的改動是打補丁，讓代碼質量繼續惡化，還是對代碼質量做了修復？

效率/性能

兩層循環，數據請求量的限制，較大數據等耗時操作是否處理得當。

關鍵算法的時間復雜度多少？有沒有可能有潛在的性能瓶頸。

復雜需求，是否有必要的設計， 可預見的效率問題， 開發模式一致性的問題 應該盡早在Design Review階段解決。如果Design階段沒有解決，那至少在Code Review階段也要把它找出來。

Style

可讀性

衡量可讀性的可以有很好實踐的標準，就是Reviewer能否非常容易的理解這個代碼。 如果不是，那意味著代碼的可讀性要進行改進。

命名對可讀性非常重要，我傾向于函數名/方法名長一點都沒關系，必須是能自我闡述的。

英語用詞盡量準確一點（哪怕有時候需要借助Google Translate，是值得的）

函數長度/類長度

函數太長的不好閱讀。 類太長了，比如超過了1000行，那你要看一下是否違反的“單一職責” 原則。

恰到好處的注釋。 但更多我看到比較差質量的工程的一個特點是缺少注釋。

參數個數（不要超過5個參數）

Review Your Own Code First

跟著名的橡皮鴨調試法（Rubber Duck Debugging）一樣，每次提交前整體把自己的代碼過一遍非常有幫助，尤其是看看有沒有犯低級錯誤。

如何進行Code Review

多問問題。多問 “這塊兒是怎么工作的？” “如果有XXX case，你這個怎么處理？”

每次提交的代碼不要太多，最好不要超過1000行，否則review起來效率會非常低。

當面討論代替Comments。 大部分情況下小組內的同事是坐在一起的，face to face的 code review是非常有效的。

區分重點，不要舍本逐末。 優先抓住 設計，可讀性，健壯性等重點問題。

Code Review的意識

作為一個Developer , 不僅要Deliver working code, 還要Deliver maintainable code.

必要時進行重構，隨著項目的迭代，在計劃新增功能的同時，開發要主動計劃重構的工作項。

開放的心態，虛心接受大家的Review Comments。

審核人及職責分配
所謂職責歸屬：即指誰對這個Code負責

提交人

審核人

職責歸屬

P2.1及以下

P2.3

審核人

P2.2及以上

P2.3

提交人&審核人

p2.3及以上

p3.1/p3.2

提交人&審核人

對于comments的檢查。周會上抽查。

審核流程
開始
RD完成開發自測
自查不過
自查通過
Review your
own code
RD按照comment意見修改
重新自測并重新提交代碼
是
否
是否可以Merge
Merge至主分支
1.只有主分支權限的人才能Merge

結束
是
否
是否需要
代碼走讀
RD發起Code Review
跟著名的橡皮鴨調試法（Rubber Duck Debugging）一樣，每次提交前整體把自己的代碼過一遍非常有幫助，尤其是看看有沒有犯低級錯誤。
是否走讀標準
1.修改了核心鏈路代碼；
2.修改代碼行數> 600 行；
3.修改公用業務組件或邏輯；
4.審核人提出走讀意見；
1.按照要求選擇審核人；
2.設置PR完成時間；
3.補充code主要變動范圍
4.跟進PR完成狀態
發起人組織
Code Review會
stash上同步記錄 commits
否
Review
是否通過
merge條件：
1.approve >= 2人；
2.comment 都完成修改；

備注標簽
流程節點
分支判斷
圖示例

常見代碼開發規范

thrift服務提供一般情況都需要在finally里輸出info級的入參和返回參數的日志

實現Object的toString方法而不是使用json序列化工具輸出日志

Integer的等值使用equals而不是==

使用apache commons 包而不是自己造輪子

組織參數抽取方法來完成而不要在service里面寫，方法命名規范成buildXXXXX

線程池的使用需要統一到一個Utils類中，避免線程池定義泛濫。

所有遠程服務調用都需要封裝delegate類，禁止直接使用octo客戶端進行調用。在真正調用一般都要finally里輸出info日志 入參和返回值。

靜態變量需要統一在一個類中定義。

OCTO生成的類不要傳遞到service層，構造一個本地類進行使用。

不要把配置寫在代碼里，充分利用MCC和*.properties

MCC里配置開關和頻繁修改的屬性，長期不修改的放到*.properties

給方法起名字是為了自己和其他人能看懂

變量命名以及方法名不要使用拼音縮寫

Json序列化統一使用travel-insurance-common表里的JsonUtils

不要在ThriftService里寫業務邏輯。下沉service

不要拷貝代碼

執行update語句一定要注意 where 條件里 一定要走索引，這句話的意思是，把where 條件單獨放一個select里 然后explain一下，如果是全表掃描 那這個SQL會鎖全表。

不要在返回boolean的方法里面寫if (xxxxx&xxxx){return true}else{ return false}

永遠不要在事務里rpc

拋出異常最好子類化，這樣從cat看到異常就知道是啥原因。

邏輯嵌套不要超過3層

@Transactional(rollbackFor =Exception.class) 使用事務的時候 一般情況這么用。

查詢不要使用mybatis生成的condition來拼，mybatis生成工具只負責生成表的po和最基本的insert 和update ，如果有分表，請所有sql全部手寫，必須帶分表鍵。

不要在static{}代碼塊里做初始化動作，禁止在static{}代碼塊里rpc