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元/人/天。