如何像人一樣做程式碼審查(上篇)
原文由 Michael Lynch 于 發布,訂閱此部落格
最近我一直在閱讀關於程式碼審查最佳實務的文章。我發現這些文章幾乎只專注於找出 bug,排除了審查中其他所有的面向。以有建設性且專業的方式溝通你發現的問題?一點都不重要!只要把所有 bug 找出來,其他的自然會搞定。
所以我靈光一現:如果這套對程式碼有用,為什麼不用在談戀愛上呢?有鑑於此,我要在此宣布推出一本幫助開發者改善感情生活的新電子書:

我這本革命性的電子書將教你經過驗證的技巧,幫你最大化地找出伴侶身上的缺點。這本電子書不會涵蓋以下內容:
- 以同理心與理解和伴侶溝通問題。
- 幫助伴侶改善他們的弱點。
根據我對程式碼審查文獻的研讀,感情中那些部分是顯而易見、不值得討論的。
這聽起來像是一本好電子書嗎?我想你剛剛應該是脫口而出「不不不不不!」吧。
那麼,為什麼我們談論程式碼審查時,卻是這種方式呢?
我只能假設我讀到的那些文章來自未來,那時所有的開發者都是機器人。在那個世界裡,你的隊友會歡迎那些措辭毫不經心的程式碼批評,因為處理這些資訊能溫暖他們冰冷的機器人心。
我要大膽假設,你想改善的是當下的程式碼審查,而你的隊友都是有血有肉的人。我還要更大膽地假設,與同事維持正向關係本身就是目的,而不只是用來降低每個缺陷成本的變數。在這樣的情況下,你的審查方式會有什麼改變?
在這篇文章中,我將討論一些技巧,把程式碼審查不僅視為技術流程,也視為一種社交互動。
什麼是程式碼審查?
「程式碼審查」這個詞可以指涉各種活動,從只是站在隊友肩膀後面一起看程式碼,到二十個人開會逐行剖析程式碼都算。我用這個詞來指的是一種正式且以文字進行,但又不像一連串實體會議那樣厚重的審查流程。

程式碼審查的參與者有作者,也就是撰寫程式碼並送審的人,以及審查者,也就是閱讀程式碼並決定何時可以合併到團隊程式碼庫的人。一次審查可以有多位審查者,但為了簡化,我假設你就是唯一的審查者。
在程式碼審查開始前,作者必須先建立一個變更清單(changelist)。這是一組作者想要合併到團隊程式碼庫的原始碼變更。
審查始於作者將變更清單送交給審查者。程式碼審查以回合為單位進行。每一回合都是作者與審查者之間的一次完整往返:作者送出變更,審查者則針對這些變更給予書面回饋。每一次程式碼審查都包含一或多個回合。
當審查者核准這些變更時,審查便告結束。這通常被稱為給予 LGTM,也就是「looks good to me(我覺得沒問題)」的縮寫。
為什麼這很困難?
如果有個程式設計師傳給你一份他自認很棒的變更清單,而你卻回了一長串理由說明它哪裡不好,那可是一個很難傳達的敏感訊息。
這就是我一點也不懷念 IT 業的原因之一,因為程式設計師是非常不討喜的一群人……以航空業為例,那些大幅高估自己技術水準的人都已經死了。
-Philip Greenspun,ArsDigita 共同創辦人,摘自 Founders at Work
作者很容易把對程式碼的批評,解讀為暗示自己是個不稱職的程式設計師。程式碼審查本是分享知識、做出明智工程決策的機會。但如果作者把討論視為人身攻擊,這一切就無從發生。
彷彿這還不夠困難,你還得透過文字來傳達想法,而文字更容易造成誤解。作者聽不到你的語氣、看不到你的肢體語言,因此更需要謹慎地措辭。對一位已經感到防衛的作者來說,一句無心的提醒,像是「你忘了關閉檔案 handle」,可能會被解讀成「我真不敢相信你居然忘了關檔案 handle!你也太蠢了吧。」
技巧
讓電腦去做無聊的部分
在會議和電子郵件等干擾之間,你能專心看程式碼的時間少之又少。你的專注力更是稀缺資源。閱讀隊友的程式碼非常耗費心力,需要高度集中。別把這些資源浪費在電腦就能做、而且做得更好的任務上。
空白字元錯誤就是一個明顯的例子。比較一下,讓人工審查者找出縮排錯誤並與作者協調修正,與直接使用自動格式化工具相比,需要多少心力:
| 需要人工審查者投入的心力 | 使用格式化工具所需的心力 |
|---|---|
| 什麼都不用做! |
右欄之所以是空的,是因為作者使用的程式碼編輯器會在每次按下「儲存」時自動格式化空白。最差的情況下,作者送出審查,而持續整合系統會回報空白有誤。作者自行修正問題,審查者完全不需要操心。
在你的程式碼審查中找出可以自動化的機械性任務。以下是常見的項目:
| 任務 | 自動化解決方案 |
|---|---|
| 驗證程式碼能否建置 | 持續整合系統,例如 Travis 或 CircleCI。 |
| 驗證自動化測試是否通過 | 持續整合系統,例如 Travis 或 CircleCI。 |
| 驗證程式碼空白是否符合團隊風格 | 程式碼格式化工具,例如 ClangFormat(C/C++ 格式化工具)或 gofmt(Go 格式化工具)。 |
| 找出未使用的 import 或未使用的變數 | 程式碼 lint 工具,例如 pyflakes(Python lint 工具)或 JSLint(JavaScript lint 工具)。 |
自動化能讓你作為審查者做出更有意義的貢獻。當你可以忽略一整類問題,例如 imports 的排序或原始檔名的命名規範,你就能專注在更有趣的事情上,例如功能上的錯誤或可讀性的弱點。
自動化對作者也有好處。它讓他們能在幾秒鐘內而非幾小時後就發現粗心的錯誤。即時的回饋讓他們更容易學習、修正成本也更低,因為相關的脈絡還在他們腦海中。而且,如果他們非得聽到自己犯了蠢錯誤,從電腦那裡聽到總比從你口中聽到好受一些。
與你的團隊合作,把這些自動化檢查直接整合到程式碼審查流程中(例如 Git 的 pre-commit hook 或 GitHub 的 webhook)。如果審查流程要求作者手動執行這些檢查,你就會失去大部分的好處。作者難免會偶爾忘記,這就迫使你仍得繼續審查那些本該由自動化處理的瑣碎問題。
用風格指南終結風格爭論
關於風格的爭論在審查中根本是浪費時間。風格一致當然很重要,但程式碼審查不是爭辯大括號該放哪裡的時候。把風格爭論從審查中剔除的最好方法,就是維護一份風格指南。

一份好的風格指南不僅定義了命名規範或空白規則等表面元素,也定義了該如何使用特定程式語言的功能。舉例來說,JavaScript 和 Perl 充滿了各種功能——它們提供了許多方式來實作相同的邏輯。風格指南定義了做事情的「唯一正解」,這樣你就不會讓團隊一半的人用一套語言功能,另一半的人卻用完全不同的另一套。
一旦你有了風格指南,就不必在審查中浪費時間與作者爭論誰的命名規範比較好。直接以風格指南為準,然後繼續往下就好。如果你的風格指南沒有針對某個特定問題制定規範,通常也不值得為此爭論。如果你遇到指南沒涵蓋、但又重要到需要討論的風格問題,就和整個團隊一起討論。然後,把決議記錄到風格指南中,這樣你就永遠不必再為同樣的問題爭論一次。
選項一:採用現有的風格指南
如果你上網搜尋,可以找到許多現成可供套用的已公開風格指南。Google 的風格指南是最知名的,但如果你不喜歡這種風格,也可以找到其他的。採用現有指南的好處是,你不用從零開始付出巨大成本,就能享有風格指南的好處。
缺點是,各組織會針對自身特定需求來優化風格指南。舉例來說,Google 的風格指南對使用新語言功能相當保守,因為他們擁有龐大的程式碼庫,程式碼必須在從家用路由器到最新 iPhone 的各種裝置上執行。如果你是只有單一產品的四人新創團隊,或許就會選擇更積極地採用前沿的語言功能或擴充。
選項二:逐步建立自己的風格指南
如果你不想採用現有指南,也可以自己建立。每當程式碼審查中出現風格爭論,就把問題拋給整個團隊來決定官方的規範應該是什麼。當達成共識後,就把該決議寫進你的風格指南。
我偏好把團隊的風格指南以 Markdown 形式放在版本控制之下(例如 GitHub Pages)。這樣對風格指南的任何修改都會經過正常的審查流程——必須有人明確核准變更,團隊中的每個人也都有機會提出疑慮。使用 Wiki 和 Google 文件也是可接受的選項。
選項三:混合式做法
結合選項一和選項二,你可以採用一份現有風格指南作為基礎,然後再維護一份本地的風格指南來擴充或覆寫基礎指南。一個很好的例子是 Chromium C++ 風格指南。它以 Google 的 C++ 風格指南為基礎,但在其之上做了自己的修改與補充。
馬上開始審查
把程式碼審查視為高度優先的事項。當你實際在閱讀程式碼並提供回饋時,請慢慢來,但開始審查的動作要立刻進行——理想上,在幾分鐘內就開始。

如果隊友傳給你一份變更清單,很可能代表他在你的審查完成之前,其他工作都被卡住了。理論上,版本控制系統允許作者開分支、繼續工作,然後再把審查中的變更正向合併(forward-merge)到新分支。但實際上,大概只有四個開發者能有效率地做到這件事。對其他所有人來說,光是要理清三方比對(three-way diff)就得花很久,久到足以抵銷等待審查期間所取得的任何進展。
當你馬上開始審查,就會形成一個正向循環。你的審查週轉時間純粹取決於作者變更清單的大小與複雜度。這會激勵作者送出小而範圍明確的變更清單。這些清單對你來說更容易、也更愉快去審查,所以你審得更快,循環就這樣持續下去。
想像你的隊友實作了一項需要 1,000 行程式碼變更的新功能。如果他們知道你能在約 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’ 拼錯了。
作者可以用兩種截然不同的方式來解讀這則意見:
- 解讀一:嗨,好夥伴!你把 ‘successfully’ 拼錯了。不過我還是覺得你很聰明!應該只是筆誤而已。
- 解讀二:你把 ‘successfully’ 拼錯了,白痴。
對比一下省略「你」的意見:
sucessfully -> successfully
後者的意見只是單純的更正,而不是對作者的評價。
幸好,要重寫你的回饋以避免使用「你」並不難。
選項一:把「你」換成「我們」
可以請你把這個變數改成更具描述性的名稱,像是
seconds_remaining嗎?
改成:
可以請我們把這個變數改成更具描述性的名稱,像是
seconds_remaining嗎?
「我們」強調了團隊對程式碼的共同責任。作者可能會離職,你也可能會,但擁有這份程式碼的團隊仍會以某種形式存在。用「我們」來說一件顯然是期望作者自己去做的事,聽起來可能有點傻,但傻總比帶有指責好。

選項二:把句子的主詞拿掉
另一種避免使用「你」的方法,是使用省略主詞的簡寫:
建議改成更具描述性的名稱,像是
seconds_remaining。
你也可以用被動語態達到類似的效果。我在技術寫作中通常極力避免被動語態,但在想要避開「你」時,它可以是個有用的寫法:
這個變數應該被改成更具描述性的名稱,像是
seconds_remaining。
另一個選項是用「不然……如何」或「……怎麼樣」這類問句來表達:
要不要把這個變數改成更具描述性的名稱,像是
seconds_remaining呢?
把回饋包裝成請求,而非命令
程式碼審查比平常的溝通需要更多的技巧與謹慎,因為很容易讓討論偏離成個人爭執。你會以為審查者會在審查中更加注意禮貌,但奇怪的是,我發現情況恰恰相反。大多數人平常不會對同事說「把那個釘書機拿給我,然後去幫我買杯飲料。」但我卻看過許多審查者用同樣強勢的命令口吻來給回饋,例如「把這個類別移到獨立的檔案中。」
寧可讓你的回饋禮貌到有點煩人的程度。把你的意見包裝成請求或建議,而不是命令。
比較一下同一則意見用兩種不同方式呈現:
| 以命令口吻呈現的回饋 | 以請求口吻呈現的回饋 |
|---|---|
把 Foo 類別移到獨立的檔案中。 | 我們可以把 Foo 類別移到獨立的檔案中嗎? |
人們喜歡對自己的工作有掌控感。對作者提出請求,能給他們一種自主感。
請求也讓作者更容易禮貌地反駁。也許他們的選擇有很好的理由。如果你把回饋包裝成命令,作者的任何反駁聽起來都像是抗命。如果你把回饋包裝成請求或問題,作者就可以直接回答你。
比較一下,根據審查者最初如何措辭,對話聽起來會有多大的對抗性差異:
| 以命令口吻呈現的回饋(對抗性) | 以請求口吻呈現的回饋(合作性) |
|---|---|
審查者:把 Foo 類別移到獨立的檔案中。作者:我不想那樣做,因為那樣它就離 Bar 類別太遠了。客戶端幾乎總是會同時用到這兩個。 | 審查者:我們可以把 Foo 類別移到獨立的檔案中嗎?作者:我們可以這麼做,但那樣它就離 Bar 類別太遠了,而且客戶端通常會同時用到這兩個類別。你覺得呢? |
看到當你編造假想對話來證明自己的觀點把意見包裝成請求而非命令時,對話會變得多麼文明了嗎?
讓意見有所本,連結到原則而非個人好惡
當你給作者一則意見時,同時說明你建議的修改以及修改的原因。與其說「我們應該把這個類別拆成兩個」,不如說「目前這個類別同時負責下載檔案和解析檔案。根據單一職責原則,我們應該把它拆成一個下載器類別和一個解析器類別。」
把意見建立在原則之上,能讓討論以更有建設性的方式展開。當你提出具體理由,像是「我們應該把這個函式設為 private,以縮小類別的公開介面」,作者就不能簡單地回一句「不,我比較喜歡我原本的寫法。」或者說,他們可以,但會顯得很可笑,因為你已經說明了修改如何達成某個目標,而他們只表達了個人偏好。
軟體開發既是一門藝術,也是一門科學。你無法總是能用既定的原則來精確闡明一段程式碼到底哪裡出了問題。有時候程式碼就是很醜或很難直觀理解,卻很難說清楚為什麼。在這些情況下,盡你所能客觀地說明,但要保持客觀。如果你說「我覺得這段不太好懂」,那至少是一個客觀的陳述,相較於「這段很混亂」,後者是一種價值判斷,對每個人來說未必成立。
盡可能提供佐證,例如相關連結。你能提供的最好連結就是團隊風格指南中的相關章節。你也可以連結到程式語言或函式庫的文件。獲得高度讚數的 StackOverflow 回答也可以,但你離權威文件越遠,證據的可信度就越低。
下篇預告
如果你喜歡這篇文章,歡迎參考本文的下半部,內容著重於如何在不引發難看衝突的情況下,讓審查順利收尾。其中包含以下技巧:
- 處理過於龐大的程式碼審查、
- 掌握給予稱讚的時機、
- 尊重審查的範圍,以及
- 化解僵局。
如何像人一樣做程式碼審查(下篇)
編輯:Samantha Mason。插圖由 Loraine Yow 繪製。感謝 @global4g 對本文早期草稿提供寶貴的回饋。
隨機一篇部落格
留言
登入後參與討論