評審的藝術 — 談談現實中的程式碼評審

2019 年 10 月 7 日
筆記

曾經寫過一點關於程式碼評審（code review）的文章，比如這篇和這篇，現在覺得關於它的認識又有了不少更新。軟體工程的技術和實踐分成兩部分，一部分是和書本知識一致的，大約佔一半，這部分基本上在大學裡就可以學，自學只要方法得當、刻苦努力也可是途徑；但是第二部分來自於實際團隊、經驗，內容通常無法從書本當中獲得，而且難說對錯，不同的人和不同的經歷造就了不同的認識。程式碼評審就是第二部分頗具槽點，可以大加討論的典型。

程式碼評審是展現個性和性格的途徑

我本人特別反對一種頗為常見的觀點，就是「一個良好運作的項目，不同的人，應該寫出一樣的程式碼」。我非常理解這種觀點的初衷，一個良好規範約束的團隊中，不同的人寫出來的程式碼應當一致。

但實際上，能真正這樣做的團隊，我還根本沒有見到過。所謂的一致程式碼，仔細品味，發現不同的工程師寫出來就是不一致。因此「一致」這個詞一定是在一定程度內的大體描述而已，並非越一致約好。其實，程式碼的創造本就是具備個性的。毫無疑問我們應當遵從規約，應當符合習慣，但是一旦試圖過度使用它們去約束程式碼，不但難以落實，而且容易產生無趣、低效和矛盾的氛圍。

再回到程式碼評審上。程式碼評審本身，以及基於評審意見的來回溝通，和程式碼本身比起來，要更難以，也更不應該要求一致。我見過許多程式碼評審的風格，有人喜歡挑小毛病，有人喜歡展開觀點夸夸其談，有人喜歡給實際例子來證明自己的看法……無論哪一種風格，我都不覺得有什麼大的問題。但是，就我個人而言，我可以談談我自己的程式碼評審風格：

我會關注三種問題，需求和業務上的問題、程式碼結構的問題、程式碼風格格式的問題。

前兩種可能存在阻礙我ship程式碼的「嚴重」問題（說「ship」就意味著認可程式碼具備了push到主線分支的條件了）。我已經記不清多少次和程式碼作者因為這樣的問題爭論了。爭論是個藝術，有時候並不一定能夠達成一致，有的人比較容易被說服，有的人則更堅持己見。我不想說哪一種更好，但是確實這是程式碼評審中展現風格的事實——都是就事論事，但有人害怕或者不喜歡得罪人，就會顯得push over一點；有人則不在乎那麼多，堅信自己的想法更合適，就會顯得defensive一點。我可能屬於後者，似乎在職業生涯的各種階段都會有和我出現程式碼評審衝突的事例，但是在某些情況下我也會選擇disagree and commit（不同意但是執行團隊達成的意見）。我相信有些團隊會喜歡我的backbone，也會有團隊討厭我的這種風格。下面的內容，也多為基於自己風格的表述。

堅定自己的意見，但是委婉地表達

關於這一點，我也在努力地改進。可以提及的點很多，技巧也很多，但是老實說，衝突還是不可避免。在我經歷的幾乎所有的團隊中，有時候會有老好人，但是基本上所有的老好人都缺少對於原則的堅持。溝通是門藝術，在程式碼評審中也一樣體現。

最重要的一條，只針對程式碼，不針對人。這條很簡單，都知道對事不對人的重要性，但是要非常小心不能違背。
對於大多數程式碼風格格式上的問題，我都會標註這個問題是一個picky或者nit的問題（「挑剔的問題」，這是我在Amazon的時候學到的方式）。這樣的好處在於，明確告知對方，我雖然提出了這個問題，但是它沒有什麼大不了的，如果你堅持不改，我也不打算說服你。或者說，我對這個問題持有不同的看法，但是我也並不堅信我的提議更好。
使用也許、或許、可能、似乎這樣表示不確定的語氣詞（包括使用虛擬語氣）。這樣的好處是，緩和自己觀點表達的語氣。比如：「這個地方重構一下，去掉這個循環，也許會更好」。
間接地表達質疑。比如說，看到對方用了一個參數3，但是你覺得不對，但又不很確定，可以這樣說：「我有一個疑問，為什麼這裡要使用3而不是其他值？」對方可能會反應過來這個值選取得不夠恰當。
放上例子、討論的鏈接，以及其它一些輔助材料證明自己的觀點，但是不要直接表述觀點，讓對方來確認這個觀點。比如：「下面的討論是關於這個邏輯的另一種實現方式，不知你覺得是否會稍簡潔一些？」
先肯定，再否定。這個我想很多人一直都在用，先擺事實誠懇地說一些同意和正面的話，然後用however一轉，說出相反的情況，這樣也就在言論中比較了pros和cons，意味著這是經過trade-off得出的結論。 ……

一些很常見的可以在程式碼評審中提及的問題

這樣的問題其實有不少，多數和實現的技術無關，但是又很容易不小心略過。它們多數時候是問題，當然也有時候不是。

比如說，我最痛恨的之一，職責過於寬泛或者職責不清的類或模組。我無數次見過這樣的類：Helper，或者Utils（注意，它們都沒有模型或者模組前綴）。考慮項目的規模，大多數情況下，這樣的類很容易變成一個越吹越大的氣球，似乎什麼東西都可以往裡擱，是個十足的違反單一職責原則的糟糕設計。比如說，在執行緒使用，容器使用，連接管理……中，缺乏上限控制的設計，在一些情況下導致資源使用過度膨脹。生產者和消費者的隊列設計，一旦消費者掛掉或者跟不上，隊列里越堆越多，沒有拒絕機制。
再比如說，缺少分包、分層，所有的東西都疊在一起，十幾個，甚至幾十個類文件並列在同一個文件夾下面。
再再比如說，程式碼缺乏設計，流水賬結構，麵條型程式碼，或者簡單鋪陳幾個過程式的方法，這種修改往往代價還不小，自然修改的落實率低，因而令提出問題的人也頗為頭疼。
……

避免一次評審提及過多的問題

少數情況下，手裡拿到一份實在是問題多到令人髮指的程式碼，這時候需要扼制自己想罵人的衝動。先抓問題的主幹，不要去挑那些細枝末節的毛病，因為很有可能，這些程式碼會被改得面目全非，或者重寫。寫一大堆評審意見，反而容易淹沒最重要的問題。

說說容易，但是這個度有時候並不好掌握。我的習慣是，在明確程式碼要解決的問題以後，先快速走讀一遍程式碼，判斷我是應該粗略地抓主要矛盾呢，還是應該嚴格地挑刺呢。我也見過一些別的方式。

不一樣的評審要求

我始終覺得，程式碼評審的要求不應該完全一致。不要過度追求公平。對於新項目程式碼，以及新員工寫的程式碼，要適當嚴格一些。

新項目的程式碼是形成後續風格和品質的模板。我們也許都聽說過「破窗戶原理」，在一塊乾乾淨淨的程式碼田地里，新耕種的程式碼才容易保持一致，也可以避免一些「為了和現有程式碼風格保持一致」而導致品質妥協的情況出現。

新員工程式碼的高品質要求則更多地在於對他們良好習慣養成的負責。軟體工程師良好職業習慣的養成，程式碼可以說是最重要一環，而前三個月的影響舉足輕重。

還不如不做的評審

有些情況下，程式碼評審是非常耗時費力的工作。特別是對業務背景不熟悉，對實現技術不熟悉，或者乾脆是對方提交上來一大堆修改，閱讀非常費力。我不知道哪一種是最難的，但是如果因為這些原因很難做到良好的評審，我會在評審中說明，或者放棄一些評審的工作，保證評審的品質。

程式碼評審是建立在團隊中影響的好方式。一方面是業務邏輯，一方面是程式碼結構，我反對在兩方面都難以給出足夠清晰的評審意見的時候，提一堆風格方面的次要問題。否則很容易達成負面影響。