Code review應該怎么做

代碼評審有兩種不同的方法，一種是代碼走查，一種是代碼審查，我們這里討論的僅指代碼走查。通常自己寫的代碼都難以發現問題，需要以第二雙眼睛再次檢查代碼，幫助我們及時地發現潛在的問題。

 做代碼審查之前，團隊成員間需要達成一個共識，制定一份合理的代碼規范標准。以此為前提，后續再補充，否則會事倍功半,可以以下面為例：

1. Code review 不應該承擔發現代碼錯誤的職責。Code Review主要是審核代碼的質量以及團隊內部知識共享而不是以缺陷和錯誤來批判他人，也不需要評論，表揚或是批評；如可讀性，可維護性，以及程序的邏輯和對需求和設計的實現。代碼中的bug和錯誤應該由單元測試，功能測試，性能測 試，回歸測試來保證的（其中主要是單元測試，因為那是最接近Bug，也是Bug沒有擴散的地方）
2. Code review 不應該成為保證代碼風格和編碼標准的手段，代碼規范與代碼優化一定要區分開，不可相提並論。首先每個程序員的提交review的代碼就應該是符合規范的，屬於每個人自己的事情，不應該交由團隊來完成。其次，作為一個審查者，你的任務不是確保被審查的代碼都采用你的編碼風格，因為它不可能跟你寫的一模一樣，而是要確保被審查的代碼的正確性。
3. Code review不應該僅僅只是走查，評審就完事了，還需要進行質量分析，CODING企業版產品集成了代碼評審和質量分析功能。

1. 體系結構和代碼設計的注意事項以及常見問題

• 代碼復用: 如果代碼被復制一次，雖然不喜歡這種方式，但通常沒什么問題。但如果再一次被復制，就應該通過提取公共的部分來重構它。
• 用更好的代碼: 如果在一塊混亂的代碼做修改，添加幾行代碼也許更容易，但建議更進一步，用比原來更好的代碼。
• 檢查new 操作的結果是否為null，Java編程新手有時候會檢查new操作的結果是否為null。可能的檢查代碼為：

1. Integer i = new Integer (400);  
2. if (i == null)  
3. throw new NullPointerException(); 

檢查當然沒什么錯誤，但卻不必要，if和throw這兩行代碼完全是浪費，他們的唯一功用是讓整個程序更臃腫，運行更慢。

• 用== 替代.equals，在Java中，有兩種方式檢查兩個數據是否相等：通過使用==操作符，或者使用所有對象都實現的.equals方法。原子類型(int, flosat, char 等)不是對象，因此他們只能使用==操作符
• 在catch 塊中作清除工作
• 沒有正確實現equals，hashCode，或者clone 等方法。方法equals，hashCode，和clone 由java.lang.Object提供的缺省實現是正確的。不幸地是，這些缺省實現在大部分時候毫無用處，因此許多類覆蓋其中的若干個方法以提供更有用的功能。但是，問題又來了，當繼承一個覆蓋了若干個這些方法的父類的時候，子類通常也需要覆蓋這些方法。在進行代碼審查時，應該確保如果父類實現了equals，hashCode，或者clone等方法，那么子類也必須正確。
• 潛在的bugs: 是否會引起的其他錯誤？循環是否以我們期望的方式終止？
• 錯誤處理: 錯誤確定被優雅的修改？會導致其他錯誤？如果這樣，修改是否有用？
• 效率: 如果代碼中包含算法，這種算法是否是高效？ 例如，在字典中使用迭代，遍歷一個期望的值，這是一種低效的方式。
• 新代碼與全局的架構是否保持一致？
• 基礎代碼是否有結合使用了一些標准或設計樣式，新的代碼是否遵循當前的規范？代碼是否正確遷移，或參照了因不規范而淘汰的舊代碼？
• 代碼的位置是否正確？比如涉及訂單的新代碼是否在訂單服務相關的位置？
• 新代碼是否重用了現存的代碼？新代碼是否可以被現有代碼重用？新代碼是否有重復代碼？如果是的話，是否應該重構成一個更可被重用的模式，還是當前還可以接受？
• 新代碼是否被過度設計了？是否引入現在還不需要的重用設計？

2. 可讀性和可維護性

• 很多人圖方便不寫注釋，自己方便了，可以別人看人看起來就費勁了。恰到好處的注釋是很有必要的，第一，不能太多或太少，注釋的形式根據代碼具體的情況有不同； 其次避免用注釋包裹代碼； 最后盡量留下簡明扼要的注釋
• 代碼清晰表達意圖，寫別人看得懂的單詞，如果選用英語，那么避免日語、法語和漢語拼音等，盡量使用語義化的命名組合。
• 避免寫一些現在不需要、將來也不太可能需要的功能
• 字段、變量、參數、方法、類的命名是否真實反映它們所代表的事物, 是否能夠望文生義?
• 避免大段代碼，要寫高內聚、低耦合的代碼，這是我們一直要追求的目標。
• 測試是否很好地覆蓋了用例的各種情況？它們是否覆蓋了正常和異常用例？是否有忽略的情況？
• 錯誤信息是否可被理解？ log打的是否正確和足夠?
• 不清晰的代碼是否被文檔、注釋或容易理解的測試用例所覆蓋？具體可以根據團隊自身的喜好決定使用哪種方式。
• 簡單就是美，避免簡單的功能寫出復雜的代碼
• 不要把代碼寫死，預測可能發生的變化
• 通過提高代碼的復用性提高代碼的可維護性

3. 功能

• 代碼是否真的達到了預期的目標？如果有自動化測試來確保代碼的正確性，測試的代碼是否真的可以驗證代碼達到了協定的需求？
• 代碼看上去是否包含不明顯的bug，比如使用錯誤的變量進行檢查，或誤把and寫成or？
• 是否有會在生產環境中導致應用停止運行的明顯錯誤？代碼是否會錯誤地指向測試數據庫，是否存在應在真實服務中移除的硬編碼的stub代碼？
• 你對性能的需求是什么，你是否考慮了安全問題？
• 這個新增或修復的功能是否會反向影響到現存的性能測試結果
• 外部調用很昂貴(a. 數據庫調用. b. 不必要的遠程調用. c. 移動或穿戴設備過頻繁地調用后端程序)

4. 安全

• 代碼的常規審查不可少，安全審查也不可少，對安全性要求較高的程序尤其要注意。如果缺少了這道流程，萬一遭受攻擊，帶來的損失將遠超過我們的想象，包括識別威脅，檢查是否存在常見安全漏洞，以及遭受安全威脅時如何應對和補救等，這是一個很打的話題，這里就不做展開。
• 識別可能受到的威脅，STRIDE 可用來識別對上述元素的威脅。STRIDE，是“假冒身份、篡改數據、否認、信息泄露、拒絕服務和權限提升”英文單詞的縮寫。
• 檢查是否新的路徑和服務需要認證
• 數據是否需要加密
• 密碼是否被很好地控制？
• 這里的密碼包含密碼（如用戶密碼、數據庫密碼或其他系統的密碼）、秘鑰、令牌等等。這些永遠不應該存放在會提交到源碼控制系統的代碼或配置文件 中，有其他方式管理這些密碼，例如通過密碼服務器（secret server）。當審查代碼時，要確保這些密碼不會悄悄進入你的版本控制系統中
• 代碼的運行是否應該被日志記錄或監控？是否正確地使用？

日志和監控需求因各個項目而不同，一些需要合規，一些擁有比別人嚴格的行為、事件日志規范。如果你有規章規定哪些需要記錄日志，何時、如何記錄，那么作為代碼審查者你應該檢查提交的代碼是否滿足要求。如果你沒有固定的規章，那么就考慮：

• 代碼是否改變了數據（如增刪改操作）？是否應該記錄由誰何時改變了什么？
• 代碼是否涉及關鍵性能的部分？是否應該在性能監控系統中記錄開始時間和結束時間？
• 每條日志的日志等級是否恰當？一個好的經驗法則是“ERROR”觸發一個提示發送到某處，如果你不想這些消息在凌晨3點叫醒誰，那么就將之降 級為“INFO”或“DEBUG”。當在循環中或一條數據可能產生多條輸出的情況下，一般不需要將它們記錄到生產日志文件中，它們更應該被放在 “DEBUG”級別。

5. 其他方面

1. 是否存在內存無限增長? 例如, 如果審查者看到不斷有變量被追加到list或map中, 那么就要考慮下這個list或map什么時候失效, 或清除無用數據
2. 代碼是否及時關閉了連接或數據流?
3. 資源池配置是否是否正確? 有沒有過大或者過小?
4. 調用接口出錯的時候, 是否有出錯處理邏輯, 並且處理正確?
5. 進程意外重啟后, 是否能夠恢復到崩潰前的環境?
6. 正確性(主要與多線程環境關系密切)
7. 代碼是否使用了正確的適合多線程的數據結構
8. 代碼是否在不需要的地方使用同步或鎖操作？如果代碼始終運行在單線程中，鎖往往是不必要的
9. 條件判斷語句或其他邏輯是否可以將最高效的求值語句放在前面來使其他語句短路？
10. 代碼是否存在許多字符串格式化？是否有方法可以使之更高效？
11. 日志語句是否使用了字符串格式化？是否先使用條件判斷語句校驗了日志等級，或使用延遲求值？

6. Code Review 的實際操作建議

1. 代碼審查是應該在互相溝通中進行討論的，而不是相互對抗。預先確定哪些是要點哪些不是，可以減少沖突並擬定預期。
2. 經常進行Code Review, 不要攢了1w行才讓同事幫你review, 這是坑隊友.

• 要Review的代碼越多，那么要重構，重寫的代碼就會越多。而越不被程序作者接受的建議也會越多，唾沫口水戰也會越多。
• 程序員代碼寫得時候越長，程序員就會在代碼中加入越來越多的個人的東西。 程序員最大的問題就是“自負”，無論什么時候，什么情況下，有太多的機會會讓這種“自負蔓延開來，並開始影響團隊影響整個項目，以至於聽不見別人的建議，從而讓Code Review變成了口水戰。
• 越接近軟件發布的最終期限，代碼也就不能改得太多。

1. Code Review要簡短、輕量，不要太正式

• 平時工作量也比較忙，審查太長時間會影響工作進度，造成延期交付，得不償失。
• 就算review時間沒有造成什么影響，增加review時間也不會明顯增加發現問題數量。
• 只用讓代碼審查簡短、輕量，才能有效的發現問題,這樣更適合迭代、增量開發，為開發者提供更快的反饋。

4. 減少代碼審查人數量，且讓不同人review，建議三人組。

• 並不是代碼審查人員越多就能發現越多Bug，第一個人審查完后，第二個人發現的bug僅為剩下問題的一半，再多個人發現問題數量也沒有明顯變化，所以一般不超過三人。
• 更多代碼審查人員意味着多人在尋找同樣的問題，造成重復工作量，另外由於指望其他人員，使得審查人員積極性、主動性不高，更加不利於代碼審查工作的有效進行。
• 三個人我認為是最合適最高效的數量，可以從不同的方向評審。保持積極的正面的態度

代碼審查對於代碼作者，審查人以及團隊都是有益的，經常閱讀自己代碼並修改重構自己代碼的習慣，因為編寫者都會覺得自己代碼寫的很完美，這是正常的現象。不過如果你過段時間再次看自己當初以為寫的很牛的代碼可以發現好多吐槽點，好多可以優化重構的地方，保持這種經常閱讀自己代碼並重構的習慣可以提高自己的代碼能力。也可以經常閱讀別人的代碼看別人的代碼風格有何借鑒之處，正所謂三人行必有吾師。

 

7. 代碼審查工具：

通常，代碼審查工具大致分為兩類，一類是按照預先定義號的規則來審查代碼，自動執行並生成報告，另一種是合作共同檢查和討論變更的場景，而且需要將這過程的歷史也存儲下來以備將來參考。需要說明的是，沒有最好的工具，只有適合自己的工具，適合就是最好，請大家根據自己的項目情況來做出選擇。

這里說一下第二種方式，借助的是CODING企業版工具。

CODING 企業版提供整套企業級的軟件研發管理系統，讓企業的管理人員可以更好地進⾏研發團隊的項目管理，便捷而深入地把握開發進度，讓開發流程高效可控。
CODING 企業版從項目的管理，到代碼管理，直到代碼的推送和部署，CODING 企業版提供了一整套的完整解決方案。

企業管理：企業管理提供企業個性化定制、企業概覽和統計、項目及成員批量管理、項目權限一鍵開關、安全管理等能滿足企業基本管理的需求。

項目管理：提供當前企業最前沿的敏捷開發，全生命周期項目和任務管理，幫助企業更好的管理項目和任務，以應對快速變化的 IT 項目管理需求。

• 簡潔高效的任務指派

• 雲端共享的項目文件

• 系統化的 Wiki 知識庫

• 多維度的項目統計

代碼管理：代碼管理提供以 Git 為基礎的日常開發源代碼版本管理，包括代碼瀏覽、分支管理、發布管理、版本對比、合並請求、項目網絡等等，提供強於 Git 的代碼管理使用體驗。

• 代碼倉庫

• 代碼評審

• 發布管理

另外價格也比較合適，免費試用15天，免費期過后是1元/人/天。