【轉】如何作人性化的代碼審查？從高到低、用例子

（點擊上方公衆號，可快速關注）

 

編譯：精算狗，英文：Michael Lynch

 

最近，我一直在讀有關代碼審查最佳範例的文章。我注意到這些文章的關注點是找到 bug，而忽略了代碼審查其餘的部分。用建設性、專業的問題溝通方式？不相關！只要識別出全部的 bug，剩下的部分會水到渠成。

 

我只能假設我讀過的這些文章都來自將來，那時候全部的開發人員都是機器人。在那個世界，你的隊友歡迎對其代碼未通過推敲措辭的批評，由於處理這樣的信息能溫暖他們冰冷的機器人之心。

我要作一個大膽的假設，你想要在當前世界改進代碼審查，此時你的隊友都是人類。我還要作一個更大膽的假設，你與同事之間積極的關係自己就是一個目的，而不只僅是一個可調整的變量來最小化缺陷的平均成本。在這些狀況下，你的審查實踐會發生怎樣的變化呢？

在這篇文章中，我討論了一些技巧，把代碼審查既看做是技術過程，也看做是社會過程。

什麼是代碼審查?

「代碼審查（code review）」這一術語能夠指一系列活動，從簡單地站在隊友身後讀讀代碼，到 20 人與會的單行代碼分析。我用這一術語指正式的、書面的過程，但也不像一系列現場代碼審查會議那麼重大。

代碼審查的參與者包括做者以及審查者：做者寫代碼並把代碼送去審查，審查者讀代碼並決定代碼何時就緒併入團隊的代碼庫。一次審查能夠由多個審查者完成，可是我作了簡化的假設——你是惟一的審查者。

在代碼審查開始以前，做者必須建立一個變動表。做者想要將源代碼併入團隊代碼庫，變動表包括一系列源代碼的變動。

看成者把變動表發給審查者時，審查就開始了。代碼審查是循環發生的。每一個循環都是做者與審查者之間完整的往返：做者發送變動，審查者給予變動的書面反饋。每次代碼審查都包括一次或者更多的循環。

當審查者批准了這些變動，審查結束。這一般指的是給出 LGTM，「我以爲不錯（looks good to me）」的簡寫。

這爲何很難？

若是程序員給你發了一份變動表，他們以爲這個變動表棒極了。你又給他們寫了一份詳細的清單，解釋爲何這個變動表並很差。這是須要當心處理的信息。

這是我不想念 IT 的一個緣由，由於程序員是很是不可愛的人……好比，在航空業，那些過度高估了本身技術水平的人都死了。

Philip Greenspun，ArsDigita 的聯合創始人，引自《Founders at Work》。

做者容易把對其代碼的批評解讀爲暗示他們不是合格的程序員。代碼審查是一個分享知識和作工程決定的機會。可是若是做者把討論理解爲我的攻擊，這個目標沒法達成。

除此以外，你還面臨着書面傳達想法的挑戰，詞不達意的風險會更高。做者聽不到你的語氣，也看不到你的肢體語言，因此清晰地、當心地傳達你的反饋更爲重要。對一個有戒備心的做者來講，一句無冒犯意味的批註，好比「你忘了關閉文件句柄」，能夠被理解成「真不敢相信你忘了關閉文件句柄！你真是個傻子。」

技巧

1. 讓電腦作無聊的部分

2. 用風格指南平息風格爭論

3. 立刻開始審查

4. 從高級別開始，逐步向下

5. 慷慨地使用代碼示例

6. 永遠別說「你」

7. 把反饋表達成請求，而不是指令

8. 把批註與原則聯繫在一塊兒，而不是觀點

讓電腦作無聊的部分

在會議和郵件的干擾下，可用來專一於代碼的時間不多。你的精神毅力更是短缺。讀隊友的代碼是認知上的負擔，要求高強度的專一。別把這些資源浪費在電腦能作的任務上，尤爲是當電腦能作得更好的時候。

空白錯誤是一個顯著的例子。比較一下人類審查者找到縮進錯誤並與做者一塊兒改正所花費的精力，和僅僅使用一個自動排版工具所花費的精力：

人類審查者須要的精力	排版工具須要的精力
1.審查者尋找空白錯誤，找到錯誤的縮進   2.審查者寫批註，指出錯誤縮進 3.審查者從新讀批註，確保措辭清晰，不含指責意味 4.做者讀批註 5.做者改正代碼縮進 6.審查者覈實做者適當地處理了批註	無！

右邊是空的，由於做者用了一個代碼編輯器，每次他們點擊「保存」時，該代碼編輯器會自動規定空白的格式。在最糟的狀況下，做者把代碼發出去以供審查，持續集成解決方法報告說空格錯誤。做者在不須要審查者顧慮的狀況下，修正這個問題。

在代碼審查中尋找能夠被自動解決的機械性任務。如下是常見的例子：

任務	自動解決方法
驗證代碼的構建	持續集成方法，好比 Travis 或者 CircleCI
證明經過了自動測試	持續集成方法，好比 Travis 或者 CircleCI
驗證代碼空白與團隊風格一致	代碼排版器，好比 ClangFormat (C/C++ 排版器) 或者 gofmt (Go 排版器)
識別未使用的輸入或者變量	代碼 linter，好比 pyflakes (Python linter) 或者 JSLint (JavaScript linter)

自動化使你做爲審查者能作出更多有意義的貢獻。當你能忽略一整個類別的問題，好比輸入的排序或者源文件命名的約定，你可以關注更有趣的事情，好比函數錯誤或者可讀性缺陷。

自動化也能給做者帶來好處。自動化使做者用幾秒鐘發現粗心的錯誤，而不是幾小時。即時反饋使得從錯誤中學習更容易，修正錯誤的代價也更小，由於做者腦海中還有相關的背景。另外，若是他們不得不聽到本身犯下的愚蠢錯誤，對自尊心來講，從電腦那聽到要比從你那聽到更容易被接受。

和你的團隊一塊兒將這些自動檢查加入代碼審查的工做流程中（例如，在 Git 中的 pre-commit hooks 或者 Github 中的 webhooks）。若是審查過程要求做者手動運行這些檢查，你會損失大部分好處。做者老是會忘記一些狀況，迫使你繼續審查簡單的問題，而這些問題原本就能被自動處理。

用風格指南平息風格爭論

關於風格的爭論浪費了審查的時間。一致的風格確實重要，可是代碼審查不是爭論花括號位置的時候。在審查中消除風格爭論的最佳辦法是，遵照一個風格指南。

好的風格指南不只定義了像命名習慣或者空白規則這樣的表面元素，並且定義了怎樣使用給定編程語言的特徵。好比，JavaScript 和 Perl 都包含了一些功能——他們提供了許多實現相同邏輯的方法。風格指南定義了作事的惟一方法，這樣不會以一半隊員用了一組語言特徵而另外一半隊員用了徹底不一樣的一組特徵收尾。

一旦有了一個風格指南，你就不須要浪費審查循環，來跟做者爭論到底誰的命名習慣最好。只要聽從風格指南而後繼續就行。若是你的風格指南沒有指定某個特定問題的約定，那它通常都不值得爭論。若是你遇到一個風格指南未涉及的問題，它又重要到須要討論，和團隊一塊兒推敲。而後把決定加到風格指南，這樣大家永遠不須要再進行一次這個討論。

選擇 1：採納一個現存的風格指南

若是從網上搜索，你能找到已發佈的風格指南可供使用。Google 的編程風格指南是最知名的，可是若是它的風格不適合你，你能夠找到其餘的指南。經過採納一個現存的指南，不須要從頭創造一個風格指南的大量花費就能繼承其好處。

壞處是組織爲他們本身特別的須要優化其風格指南。好比，Google 的風格指南在使用新語言特徵上比較保守，由於他們有一個巨大的代碼庫，其中的代碼要在全部東西上運行，從家用路由器到最新的 iPhone。若是大家是一個只有一個產品的四人小組，你可能選擇在使用前沿語言特徵或者擴展時更大膽。

選擇 2：不斷創造你本身的風格指南

若是你不想採納現存的指南，你能夠本身創造一個。在代碼審查中每產生一次風格爭論，向整個團隊提問來決定官方約定應該是什麼。當大家達成共識，把決定編進風格指南中。

我傾向於將團隊的風格指南做爲源控制下的 Markdown（例如 GitHub 頁面）。這樣，對風格指南的任何改動都須要經過普通的審查過程——某人得明確批准改動，並且團隊中的每一個人都有提出疑慮的機會。Wikis 和 Google 文件都是可接受的選擇。

選擇 3：混合方法

合併選擇 1 和選擇 2，你能夠採納現存的風格指南做爲基礎，而後用本地風格指南來擴展或者覆蓋這個基礎。一個好例子是 Chromium 的 C++ 風格指導。它用 Google 的 C++ 風格指導做爲基礎，可是在其上添上本身的改動和附加。

立刻開始審查

將代碼審查視爲高優先級。當你真正閱讀代碼並反饋時，慢點來，可是要立刻開始審查——最好在幾分鐘內開始。

若是隊員發給你一個變動表，這可能意味着直到你完成審查前，他們會卡在其餘工做上。理論上，源控制系統使做者能建起新的分支，繼續工做，而後從審查中把變更合併進新分支。實際上，一共有大約四個開發者可以高效地作這件事。其餘人要花很長時間來清理三方差別，以至於抵消掉了等待審查完成這段時間裏的進步。

你立刻開始審查，就創造了一個良性循環。你的審查時間徹底變成了一個與做者的變動表大小和複雜度相關的函數。這激勵做者發送短小、範圍狹窄的變動表。對你來講這樣的變動表審查起來更容易，也更愉悅，因此你能更快地審查，循環繼續。

想象一下你的隊員要執行一個新特徵，這個特徵要求 1000 行代碼變動。若是他們知道你能在大概 2 小時內完成一個 200 行的變動表的審查，他們能夠把特徵拆分紅各包含 200 行的變動表，而後在一兩天內檢查完整個特徵。可是，若是不管大小你都要花一天來完成全部的代碼審查，如今就要花一週時間才能檢查完整個特徵。你的隊員不想傻坐一週，因此他們被激勵着去發送更大的代碼審查，好比每一個包含 500 到 600 行。這樣審查起來花銷更大，反饋也更差，由於記 600 行變動表的背景要比 200 行變動表難。

一個審查循環的最大週期應該是一個工做日。若是你正在處理一個更高優先級的問題，不能在一天內完成一個審查循環，讓你的隊員知悉並給予他們把審查交給別人的機會。若是你一個月被強制回絕審查超過一次，可能意味着你的團隊須要放慢腳步，這樣你能保持理智的開發實踐。

從高級別開始，逐步向下

在一個既定的審查循環中，你寫的批註越多，讓做者感受受打壓的風險越大。準確的界限隨開發者的不一樣而不一樣，可是一個審查循環中 20 到 50 個批註通常是危險區的開始。

若是你擔憂把做者淹沒在批註的海洋裏，約束你本身在早期循環中反饋高級別的問題。注意從新設計類接口或者拆分複雜函數這樣的問題。等到這些問題都解決了再去處理低級別的問題，好比變量命名或者代碼評論的清晰度。

一旦做者整合了你高級別的批註，低級別的批註可能會變得無心義。把低級別的批註推遲到後期的循環中，你能夠把本身從當心措辭的工做中解救出來，也省得做者處理沒必要要的批註。這個技巧也細分了審查過程當中你所關注的抽象層，幫助你和做者用清晰、系統的方法完成變動表。

慷慨地使用代碼示例

在一個理想的世界裏，代碼做者會感謝收到的每一次審查。這是他們學習的一個機會，也能防止他們犯錯。事實上，有許多外部因素能致使做者負面地解讀審查，怨恨你給他們批註。可能他們正面臨着截止日期的壓力，因此除了馬上不經審查的批准之外的東西都感受像阻礙。可能大家沒怎麼在一塊兒工做過，因此他們不相信你的反饋是好意的。

一個讓做者對審查過程感受良好的方法是，在審查中找機會送他們禮物。全部開發者都愛收到的禮物是什麼呢？固然是代碼示例啦。

若是經過寫一些建議的改動來減輕做者的負擔，就證實了做爲審查者，你對時間很慷慨。

好比，想象一下你的一個同事不熟悉 Python 的列表推導（list comprehension）特徵。他們給你發送了包含如下代碼的審查：

urls = []

for path in paths:

  url = 'https://'

  url += domain

  url += path

  urls.append(url)

 

回覆「能用列表推導（list comprehension）簡化這個嗎？」會使他們苦惱，由於如今他們得花 20 分鐘搜索他們以前從沒用過的東西。

收到像如下這樣的批註他們會更開心：

考慮用像這樣的列表推導（list comprehension）來進行簡化：

urls = ['https://' + domain + path for path in paths]

 

這個技巧並不侷限於單命令程序。我會常常創建我本身的代碼分支，向做者展現概念的一個大型證實，好比拆分一個大型函數或者增長一個單元測試來覆蓋一個附加邊界狀況。

爲清晰、無爭議的改進保留此技巧。在上面列表推導（list comprehension）示例中，極少有開發者會拒絕減小 83% 的代碼行數。相反，若是你寫了一個冗長的示例來演示某個變更「更好」，而這個變更是基於你本身的我的品味（好比，風格變更），代碼示例讓你看起來執拗己見，而不是慷慨大方。

限制你本身在每一個審查循環中只寫兩到三個代碼示例。若是你開始爲做者寫整個變動表，這標誌着你以爲做者沒能力寫本身的代碼。

永遠別說「你」

這聽起來挺怪異的，可是聽我說：永遠別在代碼審查中使用「你」這個字。

在審查中作的決定應該是基於什麼能讓代碼更好，而不是誰出的主意。你的隊員在他們的變動表中傾注了大量心血，並且極可能爲本身的工做感到驕傲。他們聽到對其工做的批評，天然反應是擺出防護和保護的姿態。

組織反饋所使用的措辭，以最小化激起隊員戒備心的風險。講清楚你是在批評代碼，而不是程序員。看成者在評論中看到「你」這個字，會將他們的注意力從代碼轉移到本身身上。這增長了他們把批評私人化的風險。

考慮一下這個無害的評論：

你拼錯了「successfully」。

做者能夠把這個批註理解成兩種不一樣的意思：

• 理解 1：嗨，好傢伙！你拼錯了「successfully」。可是我仍是以爲你聰明！那可能就是個筆誤。

• 理解 2：你拼錯了「successfully」，笨蛋。

把這個跟省略了「你」的批註比較一下：

sucessfully -> successfully

後者是一個簡單的修正而不是對做者的審判。

幸運地是，在從新寫反饋時避免使用「你」並不難。

選擇 1：用「咱們」替換「你」

你能重命名這個變量，讓它更具備描述性嗎？好比 seconds_remaining。

變成：

咱們能重命名這個變量，讓它更具備描述性嗎？好比 seconds_remaining。

「咱們」增強了團隊對代碼的集體責任。做者可能跳槽到一個不一樣的公司去，你也可能，可是擁有這個代碼的團隊會一直以不一樣的形式存在。當你明顯指望做者本身作某些事的時候，說「咱們」聽起來會比較傻，可是傻要比指責好。

選擇 2：移除句子的主語

另外一個避免使用「你」的方法是用省略句子主語的簡化句子：

建議重命名爲更具備描述性的名稱，好比 seconds_remaining。

你能夠用被動語態實現類似的效果。我在技術寫做中通常會避免像瘟疫同樣使用被動語態，可是它是個有用的方法來避免使用「你」。

變量應該被重命名爲更具備描述性的名稱，好比 seconds_remaining。

另外一個選擇是把它表述爲一個問題，用「……如何」或者「……怎麼樣」開頭：

把變量重命名爲更具備表述性的名稱怎麼樣？好比 seconds_remaining。

把反饋表達成請求，而不是指令

代碼審查相對日常的交流來講，要求更多的機智和謹慎，由於存在高風險把討論轉變成私人爭論。你會指望審查者在審查中表示出禮貌，可是奇怪地是，我發現他們走向了另外一個方向。多數人永遠不會對同事說「給我訂書機，再給我拿瓶汽水。」可是我看到過無數審查者用相似的指令來表達反饋，好比，「把這個類移到一個單獨的文件裏。」

寧肯在反饋中惱人地紳士。把批註表達成請求或者建議那樣，而不是指令。

比較用兩種不一樣方式表達的同一個批註：

表達成指令的反饋	表達成請求的反饋
把 Foo 類移到一個單獨的文件裏。	咱們能把 Foo 類移到一個單獨的文件裏嗎？

人們喜歡掌控本身的工做。向做者提出請求給他們帶來自主意識。

請求也讓做者禮貌地反饋更容易。可能他們的選擇是有合理的。若是把反饋表達成指令，來自做者的任何反饋都像違反指令。若是你把反饋表達成請求或者問題，做者能簡單地回答你。

比較對話的好鬥程度，取決於審查者怎麼表達他們的初始批註：

表達成指令的反饋（好鬥的）	表達成請求的反饋（合做的）
審查者：把 Foo 類移到單獨的文件裏 做者：我不想這麼作，由於那樣就離 Bar 類太遠了。客戶幾乎總會一塊兒調用他們。	審查者：咱們能把 Foo 類移到單獨的文件裏嗎？ 做者：能夠，可是那樣就離 Bar 類太遠了，以及客戶通常會一塊兒使用這兩個類。你以爲呢？

看看當你構建虛擬對話來證實觀點把批註表達成請求而非指令的時候，對話變得多麼有禮貌。

把批註與原則聯繫在一塊兒，而不是觀點

當你給做者寫批註時，既要給出變動建議，也要給出變動的理由。「如今，這個類既負責下載文件，也負責解析文件。咱們應該依照單一責任原則，把它拆分紅一個下載類和一個解析類。」這麼說會更好，而不是說「咱們應該把這個類分紅兩個。」

讓你的批註有原則性的立足點，這樣能讓討論走向更積極的方向更有建設性。但你有一個具體的緣由，好比「咱們應該把這個函數寫成私有函數，來最小化 public 藉口類」，做者就不能簡單地回覆「不，我傾向於個人方法。」更確切地說，他們能夠，可是由於你演示了改動如何知足目標，而他們只陳述了一個偏好，他們會看起來很傻。

軟件開發既是藝術也是科學。你不可能永遠都能用肯定的原則來明確表達代碼到底哪裏出了問題。有時候代碼只是難看或者不符合直覺，不容易肯定爲何。在這些狀況下，解釋你能怎麼作，可是保持客觀性。若是你說「我發現這不容易理解」，這至少是個客觀的陳述；相反，「這莫名其妙」是一個價值判斷，不必定適用於全部人。

儘量以連接的形式提供支持證據。團隊風格指南的相關部分是你能提供的最佳連接。你也能夠連接到語言或者庫的文件。高票 StackOverflow 回答也行，可是離權威文件越遠，你的證據變得越不穩固。

第二部分：即將上線

敬請期待其餘小技巧，包括：

• 處理特別大的代碼審查

• 識別給予表揚的機會

• 尊重審覈的，以及

• 化解僵局

由 Samantha Mason 編輯。插圖來自 Loraine Yow。感謝 @global4g爲這篇文章的早期版本提供寶貴的反饋。

以爲本文有幫助？請分享給更多人

關注「算法愛好者」，修煉編程內功