【51CTO.com快譯】 在今天的文章中，我們將探討如何審查他人編寫的代碼。這意味著我們需要將立場轉換為審查者，并期望代碼提交者能夠遵循理想的開發原則及***實踐，從而減輕我們的工作強度。

大家可以采取多種方式完成代碼審查任務，這里我們將列出一份分布清單，幫助大家有效縮短審查時間并提升審查效率。

免責聲明

每段代碼、每個項目乃至每支團隊都擁有自己的特性，因此本文只作為指導性參考，而非必須嚴格執行的硬性規則。

另外，請注意代碼審查中幾乎必然涵蓋代碼閱讀與審查問題溝通等因素。因此，這里我們主要著眼于提升溝通效果，而非更加具體的代碼內容。大家需要根據實際情況判斷具體代碼內容是否符合要求。

[[178794]]

1.由參與度***的人負責回答問題

假定您本人并沒有參與代碼編寫（而單純負責代碼審查），那么大家首先需要意識到，您并不清楚代碼編寫中的背景或者具體條件。因此，不要輕易將你不太理解部分視為錯誤，而要知道每個人思考問題的思路都不一樣——也就是給他人解釋的機會。

[[178795]]

以此為基礎，為以下問題找到答案：

如果確實存在問題，了解為何會出現這類問題。和代碼編寫者溝通你所發現的問題會帶來哪些嚴重的影響時，如果你有解決方案，請將其作為建議提出。如果您沒有解決辦法，但可以就解決問題給出指導，請確保溝通到位。如果您認為某種特定設計模式能夠解決問題，請詳盡告知。如果您認為代碼中的其它功能或者部分可以用于解決此問題，應明確指出。無論您指出怎樣的解決方案，只要不偏離主題，相信都能為代碼編寫者帶來幫助。

• 如果不屬于問題，不要將其作為問題指出。如果被代碼編寫者誤以為你表述內容是代碼有問題，則其會對其進行修改，而這樣的修改往往并未經過具體討論。頗為諷刺的是，即使對代碼內容的修改幅度不大，但也會引發新的問題。更為糟糕的是，您的建議可能由于不夠明確或者存在歧義而使得開發者對代碼內容進行多次迭代?？傊拐\表達自己的看法能讓真正的問題得到重視與改正，要注意哪些觀點僅僅出于有疑惑而非真正屬于錯誤。

• 讓問題成為問題。不要自行尋找答案，而應由開發者給出直接解釋。具體來講，不要對問題表述進行過度修辭性修飾，也不要引導對方給出您潛意識里想要的答案。

• 讓對方自己做調查。開發者對于代碼更為熟悉，因此其對代碼的編寫及內容也更有發言權。他們能夠比您更快地找到更為確切的答案。然而，您親自進行調查可能同樣會帶來價值，前提是您愿意在審查周期之外拿出時間考慮代碼的內部運作機制。

以下是大家經常會范的錯誤：

修改前：

1. 假定某些代碼一定表現糟糕。表達內容不清，未明確提出問題所在，而假定受審查方未能夠猜到您想傳達的觀點。

2. 為什么執行效果不佳？是不是應該進行修改？

3. 性能表現是否存在問題？其會造成哪些影響？（具體描述。）

修改后：

我認為這條SQL查詢能夠給出正確的結果集，但我覺得其可能執行了表掃描或者并未充分使用索引機制，不過我對此并不確定。您能檢查一下其執行計劃并查看數據庫中的性能是否還有改進空間嗎？也許我們可以調整這條查詢，或者添加更多索引。請注意，目前的狀況意味著用戶每次查看主列表時都會受到影響。

這里承認目前作法中正確的部分（即SQL結果正確）。

1. 其提到合理的懷疑并表示這僅僅是種懷疑。（‘我不確定’。）

2. 其表達了可能的問題來源（‘表掃描’、‘未使用索引’）。

3. 其給出了可行建議（‘檢查執行計劃’）。

4. 建議潛在的修改方向（‘變更查詢’，‘添加索引’）。

5. 其表達了變更在實際應用中的影響（‘用戶查看列表時的性能表現’）。

2. 要明確給出質疑的理由

在提供變更建議或者指出問題時，大家注意要明確給出質疑的理由。這樣做能夠確保其他參與對話的同事根據我們的表述或者自身理解找到正確的解決方案。

這一點非常重要，特別是在大家并沒有明確提出預期解決方法的情況下。單純提出問題并不足以幫助代碼編寫者了解潛在的問題根源。

[[178796]]

以下是負面示例：

這里有拼寫錯誤。

這之所以被歸為負面示例，是因為如果對方都沒有發現拼寫錯誤，那么很可能再次檢查時也發現不了錯誤。當然，除非這里的錯誤非常明顯。無論如何，***直接給出正確的拼寫。

需要在這里提醒下各位：

“這里”是種很模糊的表達，而且除非開發者已經意識到在代碼內添加驗證流程的意義所在，否則其很可能直接向這部分代碼片段中復制一條驗證語句，或者盲目猜測審查者的意圖。

以下是正面示例

這里拼寫成了“typ”，實際應為“type”。

這里糾正了負面示例：指出錯誤拼寫，并給出正確結果。

還有一點是因代碼沒有添加驗證屬性允許任何未授權用戶調用。這可能造成安全問題，包括由未經許可的第三方變更系統的內部狀態。請添加驗證屬性以避免發生此類問題。

解釋問題細節，幫助開發者理解相關影響并在未來的開發工作中避免其再次發生，***給出理想的解決建議。

3.需要審查什么

在審查過程中，我們需要考慮的問題多種多樣。不過總體來講，如果答案讓您感到別扭，那基本代表著我們發現了一個問題，并需要提供反饋意見以協助將其解決。

然而，同樣需要注意的是，大家應該適當放棄爭論，特別是在問題并不會影響產品質量的情況下。很明顯，一味揪住不太正確的縮進習慣不放，只會讓開發者心生怒火。

[[178797]]

一般的情況下，大家都會直接審查全部代碼后，再審查第二遍乃至第三遍; 當然，我們也可以用清單的方式對各個部分進行分別審查。我根據自己的理解，將代碼中的各部分內容進行了重要度排序：

快速檢查清單

1. 變更是否可行？

• 1. 變更能否實現預期效果？也許應當配合項目初始要求進行協同考量。

• 2. 假設及其約束條件是否適用于這套系統？

• 3. 如果會對用戶造成影響，那么這種影響是否在可接受范圍內？（例如向Web應用中添加一套新的大規模JavaScript庫。）

• 4. 變更是否會帶來后續成本？（包括服務器、許可乃至處理器資源利用等。）

• 5. 變更是否合法？（是否避免使用未付費的專利庫、來自其它源的代碼、非兼容許可乃至未受保護的數據等？

2. 其是否擁有正確的技術導向？

1. 現有設計是否適用于以往規劃的解決方案？

2. 變更是否會引入合理的復雜性水平？

3. 是否保證將故障點數量控制在***程度？

4. 是否具備靈活性？其在未來的產品發展中是否仍提供充分的靈活性空間？

5. 使用更為簡單或者適當的解決方案時，是否能夠避免引入第三方庫及代碼？

3. 能否妥善處理錯誤及意外情況？

• 1. 如果可以，其是否會被納入日志記錄？

• 2. 如果可以，其是否會向用戶提供正確的提示信息？

• 3. 如果可以，其是否會拖慢開發團隊的工作進度？

• 4. 錯誤及意外情況是否得到處理？其是否在正確的代碼區段內得到處理

4. 其中是否包含正確的指標數量？

• 1. 其是否能夠指示出有意義的數據？

5. 變更是否安全？

• 1. 其是否避免披露一切應當受到保護的數據？

• 2. 其是否會對一切不應被普通存儲或傳輸的數據進行了加密？

• 3. 其是否對輸入內容或者第三方系統進行檢查或者驗證？

6. 其是否避免引入技術債務？

• 1. 如果引入了技術債務，這些債務條目是否在代碼中注明了引入理由？

• 2. 這些理由是否無法利用現有有效方法加以解決？

• 3. 技術債務是否在可接受范圍內

7. 其是否經過測試？

• 1. 輸入數據是否接受有效性/白名單/黑名單機制處理？

• 2. 其是否為相關數據使用了正確的變量大?。ɡ鐂hort amountOfPeopleInStadium就不太適當。）

• 3. 其是否包含單元測試？這些測試是否有效？

• 4. 其是否包含集成測試？這些測試是否有效？

• 5. 其是否包含自動化測試？這些測試是否有效？

• 6. 如果要修復一項bug，相關測試能夠重現原始bug嗎

8. 代碼是否具備可讀性？

• 1. 其中是否包含拼寫錯誤或者錯別字？

• 2. 其中是否包含奇怪的縮寫？

• 3. 對于寫入內容，其是否可理解且表意明確？（即在說明文檔及注釋中使用合適的語言及表達。）

• 4. 注釋內容是否有意義及有效

9. 代碼是否可維護？

• 1. 如果其向系統中引入了新“概念”，這部分概念的定義是否在說明文檔中有所體現？

• 2. 其中是否避免使用任何奇怪的代碼結構？（即‘炫技型代碼’。）

• 3. 變更與方法的命名與其含義是否相符？（您能否根據其名稱了解其含義？）

10. 是否采用正確的編碼風格？

• 1. 變量/方法/類用法

• 2. 縮進

• 3. 大括號用法

• 4. 注釋密度

4.以正確方式閱讀代碼

根據項目類型以及所引入變更的具體特性，我們往往可以通過多種方式實現這種變更。大家可以這樣思考：當我們在查找特定錯誤代碼片段時，通常會在頭腦中完成調試流程，包括讀取從輸入到處理的對應代碼，并根據調用順序捋順其邏輯。這種代碼閱讀方式能夠幫助大家更加明確各組件間的關聯及交互方式，但其它一些場景則不太適用這種方式。

自上而下閱讀代碼能夠幫助大家了解其中的抽象含義，并弄清是否需要利用靈活代碼以支持不同場景，但這往往不能讓我們很快掌握代碼之間依賴性的冗余或者缺失問題——因此這種作法并不可取。

[[178798]]

在查看不同的模塊或者命名空間時，大家可以關注其子系統交互及組織方式，這能幫助大家找到那些常見的設計與架構問題，但往往會漏掉具體實現細節。

總之，我們應確保在審查代碼時從多個角度考慮問題，同時變換立場及順序反復檢查。

5.以不會導致誤解的方式進行反饋意見討論

作為審查人員，我們的反饋意見往往能夠直接被傳達至初始開發者處。另外，其中某些意見可能相當具體，而另一些則較為開放，意味著需要引入更多開發者進行廣泛討論。如果您的反饋意見單純以大篇幅文本形式體現，那么往往很難在不引發誤解的前提下順利進行討論。

我個人比較喜歡GitHub及Bitbucket等服務中的評論功能。這些意見會分行清晰顯示，用戶能夠清晰掌握其相關上下文，并針對反饋中的特定部分進行具體討論。更重要的是，如果文件內容發生變更（這在反饋過程中非常常見），那么對之前代碼的反饋會被隱藏起來，這意味著過期的反饋意見將不再被納入代碼審查流程。

GitHub目前采用的代碼審查方案是，我們可以將自己的全部信息進行隊列排布，將其作為審查內容的一部分進行發送，并隨后進行批準或者拒絕。微軟提供的TFS在線服務也采取類似的機制。這種方法非常有效，我們能夠隨時發現人們對代碼的錯誤以及改進空間的留言，并在之后進行針對性修改。不過在采取這種方式時，請務必在發送前對全部評論內容進行認真審校。

6.避免“擠牙膏”

要盡可能提供完整的審查意見，而非像擠牙膏般發現一點問題就通知開發者進行修改。無論大家提供的是單一文本還是拆分式注釋，都要盡可能一次性提供全部信息，這樣開發者（也包括我們自己）才能更為充分地利用時間。對于開發者來說，到處亂竄進行代碼修改絕不是什么舒適的體驗，而由此產生的怨氣最終都會被歸結于您。

[[178800]]

有時候人們會通過郵件發送代碼審查反饋。在這種情況下，郵件的結構就變得非常重要，因為良好的格式能讓對方逐行加以回答，我們也可以在得到合理答復后將不必要的部分刪去。

7.要有禮貌

在提交反饋意見時，千萬不要抱有“這就是個錯誤”的心態——即使事實確實如此。請始終堅持“這里還有改進空間/這里應該加以改進”的態度; 另外，除非您***確定，否則請以疑問的口吻提出意見。請記住，接受審查的對象是人而不是機器; 第二，他們已經盡可能做好這份工作了。即使出于技術、知識、經驗或者時間所限而導致開發者拿出了無法接受的代碼成果，也請堅信一點，他們已經為項目的推進貢獻出了大量精力。

[[178801]]

Linus Torvalds式的粗口與責罵雖然讀起來頗具趣味，但對于交流對象而言卻是種很深的侮辱與傷害。另外，為什么非要給自己樹敵呢？畢竟老話說得好，和氣才能生財嘛。

原文標題：How to perform a good code review

【51CTO譯稿，合作站點轉載請注明原文譯者和出處為51CTO.com】