關於代碼審查，那些你不曾關注的細節

作者 | Marcus Eisele

譯者 | 彎月

出品 | CSDN（ID：CSDNnews）

在工作中，我們都要進行代碼審查。每個人都知道代碼審查，每個人都會做代碼審查（至少我希望你會做）。但如果花點時間討論一下，你就會發現在“良好的代碼審查應該做些什麽”這個問題上，可謂仁者見仁智者見智。每個參與者應盡的義務是什麽？代碼審查的最佳方式又是什麽？

什麽是代碼審查？

首先讓我們來看一看什麽是代碼審查？維基百科上的代碼審查定義如下：

代碼審查（有時稱為同行審查）是一項確保軟體品質的活動，是由一個或多個人通過查看和閱讀部分源代碼來檢查程式的一種方式，代碼審查通常發生在實現完成後或實現中間階段。參與審查的人員中必須至少有一人不是代碼的作者。執行審查的人員稱之為“審查人”，但不包含作者本人。

——維基百科

這段引文後面還寫了代碼審查的目標，其中，除了找出被審查代碼中的品質這一主要目標外，通過執行這些審查還可以實現：

提高代碼品質

保持項目的一致性

發現 bug

學習（通過代碼被審查）和教學（通過審查其他人的代碼）

營造共同的責任感

通過他方查找代碼中的小錯誤，防止這些小錯誤日積月累腐蝕代碼

尋找更好的解決方案的常見方法

從上述定義中我們能看出什麽？代碼審查是一種好方法，它可以保持軟體的可維護性，並在軟體投入生產之前發現 bug。除此之外，該方法還有助於教導團隊的新成員，即使沒有正式的培訓或訓練，我們也可以通過代碼審查將一些技巧傳授給他人。

但如果僅參照該定義展開代碼審查的工作，我們仍然不清楚應該做些什麽？下文將闡述每個參與者應盡的義務，並從中總結出代碼審查的最佳方式。

被審查者視角：不給審查者添麻煩

首先申明一點：代碼審查不是為了評判或審查某個人的編程能力。“代碼審查隻關注代碼”，無論這段代碼是團隊中的高級開發人員寫的，還是新人實習生寫的——被審查者的職責在審查之前就開始了。

在我看來，被審查的人應該盡量減輕審查者的工作，試著換位思考。我認為加大審查者工作難度的主要因素如下：

將重構的代碼和新代碼的實現混合在一起

在提交功能變更時，重新格式化整個代碼庫

提交代碼時不寫提交注釋（我會在後面討論該話題）

在一次代碼審核（或一次代碼提交）中提交多個功能

需要考慮的因素

在寫完上面這段話後，我想起在代碼審查中最煩人的因素是“干擾”和“大小”。所謂干擾，我指的是與提交注釋無關的一些變更，它們往往會造成思想負擔。因為在審查過程中，你無法確定眼前的這些代碼只是外觀上的變化還是非常重要的功能。

其次是大小——一次隻應該提交一個變更。如果你遇到地問題是提交過大，那麽對應地解決方法就是拆分之後再分別審查，然後再將這些分離的提交合並到臨時的功能分支上，最後再合並到主分支上。由此，我總結出了以下幾條代碼審查的最佳方式：

1. 單獨提交代碼清理命令（重新格式化或修複拚寫錯誤等）和重構

我甚至不建議將重新格式化與重構混合到一起。如果你想重構代碼，那麽請注意用正確的格式。如果代碼中只有與重構相關的變更，那麽代碼審查會容易得多。當代碼中出現大量基本上只是清理命令的變更時，我們有時很容易忽略小的變化（還記得我說過的干擾嗎）。

2. 編寫相關的提交說明

務必確保你的提交注釋可以很好地向審查者說明提交的內容，還有尤其要說明代碼變更的原因。如果你的設計受到了很大限制，也一定要寫好說明。

如何書寫好提交注釋？下面是我在工作中提交代碼時寫的提交注釋，這也是我們的團隊慣用的格式：

XXX：添加一些新功能（簡短的說明）

對於代碼實現更為詳細的描述

你可以寫很多行，但還可以包括：

* 項目符號

* 不同的細節

* 如果你有很多點，一行寫不下，那麽也可以在各個點之間插入空行

引用與之相關的問題

其中 XXX 是問題的鏈接（例如 JIRA-007：標題），如果代碼變更沒有相應的追蹤問題，那麽可以只是用關鍵詞替代，如 FIX、BUG 或 MAINT（維護）。

上述有關提交說明的建議不僅針對代碼審核，更是普遍適用的最佳方式。提交說明中如果漏掉了什麽重要的資訊總是令人惱火，相反，清晰明了的提交注釋也會令人心情愉悅。通常在審核代碼遇到問題時，就可以試著看能否從提交注釋中找到答案。

3. 隻提交準備好審查的代碼

這也是對他人的尊重。如果我希望得到另一個人的建議，就不會給他製造不必要的麻煩。因此，請確保你的代碼通過了所有測試。另外，在讓別人審查你的代碼前，先進行自我審查，仔細看看你提交的代碼差異。

4. 審查期間不要更改代碼

這種做法會給審查者帶來更多壓力，以致中斷審查進度。如果你想修改審查過程中發現的問題，那麽請確保在接受審查的代碼基礎上再另建一份提交。如此一來，審查者就可以在現階段審查完成後，再來看你新修改的代碼。最終，在所有審批都確認後，你可以將所有的提交壓縮成一個。

回顧被審查者的工作，我們可以得出一個結論，即不要給審查者製造不必要的麻煩當屬代碼審查過程中被審查者的最佳行為準則：

確保你的代碼通過了自己的審查，並且你沒有發現任何明顯的問題，可以放心地合並代碼（如果你發現了問題，並想討論某些內容，那麽提前跟你的審核者打招呼）；

代碼中沒有混合不相乾的變更，不會太長也不會增加閱讀難度；

針對代碼變更寫好提交注釋，明確交待變更的目的

「對碼不對人」的審查者

首先再次申明：代碼審查不是為了評判或審查某個人的編程能力。

我覺得這點對審查代碼變更的人而言更重要，我建議你在評論時更加謹慎，通常可能只是因為你沒有看到寫代碼的人在實現過程中遇到的問題。那麽，在代碼審查時究竟應該做些什麽？通常我會檢查幾個重點項，下面我將據此總結出代碼審查的最佳方式。

目的

我的第一個檢查點是代碼是否與提交注釋相符，如果兩者存在差異，那麽就很難驗證代碼的正確性。

實現

在驗證了實現的目的後，我會驗證實現本身，在讀完提交說明後，我問自己的第一個問題就是：“我應該如何實現？”

接下來，我會將審查的代碼與我想象的解決方案做比較。如果我的想法與審查的代碼之間存在很大差異，那麽通常我會再回頭查看提交注釋，或前一次變更，並快速整理我設想的版本。

通常在幾分鐘後，我就會知道自己的解決方案是否可行，此時我需要考慮的因素包括：

性能（如果是在同一個代碼庫的話，通常不那麽重要）

可讀性（恕我直言，這幾乎是最重要的）

嘗試覆蓋比被審查的解決方案更多的極端情況

是否可以用更好的代碼樣式（比如更多可重用的代碼）來實現相同的功能

然而，實際情況可能往往有所不同——你想到了一個解決方案，同時驚訝於你所審核的代碼作者的睿智。如果真是如此，那簡直太棒了，因為你不僅學到了新東西，而且你需要做的就只是看看眼前的代碼在格式和編程風格上是否與開發指南相符。

可維護性

對我來說，可維護性是最重要的因素之一（這可能有偏見，因為目前我正在開發一個長期項目），但若能不影響將來的實現速度也依舊非常了不起。

出於可維護性的原因，我會直接查看代碼測試。如果沒有，那麽對我而言無疑是個壞消息，因為這通常意味著我“需要動手寫一些測試用例”。

如果代碼中有測試用例，那麽我會驗證它們是否在測試正確的東西，還有變更後的 API 的使用方法以及測試用例是否合理。通過單元測試我們就能看出實現的使用方法。如果某個實現難以閱讀或難以使用，那麽大多數時候只能說明該實現不完美。

接下來我會注意 API 級別的重大變化。如果 REST 控制器被修改了，我就會很擔心，因為我不想我們的變更牽扯到其他服務的部署。

理想狀態下，這不是問題，因為我們兩邊會同時部署，但大多數情況下，我會避免這種做法，同時採用一個不會破壞客戶端的版本。負責服務間通信的 DTO（數據傳輸對象，Data Transfer Objects）的變更亦是如此。如果DTO中引入了一個新值，我們就需要格外謹慎。

下一步我會縱觀實現整體。如果其很難理解，我會想辦法（就像前面想象代替實現方案一樣）讓它更加容易閱讀和理解；如果我想到更好的方法，也會征求代碼作者的意見，大多數時候，我們會迅速達成共識或商量出一個折中方案。

最後我需要檢查的是外部文檔。如果有外部文檔（例如外部 wiki 或 README.md），我會快速檢查文檔是否反映了代碼中的變更。

安全

一般來講，代碼都需要擁有某種安全機制（例如 OAUTH 層，或 REST 控制器的 BASIC AUTH）。在添加新代碼時，我會驗證這些代碼是否得到了正確的保護。

寫評語的方法

在寫審查評語時，一定要很友好，還要讓你的評語盡量簡潔準確。請確保評語針對的是代碼，而不是寫代碼的人。盡可能避免使用所有格（比如你的、我的等等），這些詞語會讓人誤解，儘管你想說的是代碼本身。

你需要寫明你的評語是代碼變更要求，還是可能需要討論的意見。如果你在一段非常好的代碼中寫了很多挑剔的評語，那麽一定要給寫代碼的人一些讚美的話。

最後，總結代碼審查中，審查者的最佳操作方式：

保持友好

審查代碼，而不是寫代碼的人

評語要簡潔準確

在最後不要忘了為好的代碼點讚

表明你是否需要等代碼修改完畢後再看一次（如果你們的審查工具不支持這一功能的話）

希望這篇文章可以幫助你了解“代碼審查的最佳方式”。每天在工作中，我都會遵照這些準則，這對我和我的團隊都有很大的幫助。如果你對該主題有其他看法，也請告訴我。

原文：https://programmerfriend.com/index.php/2018/12/10/code-review-best-practices/

本文為 CSDN 翻譯，如需轉載，請注明來源出處。