#### 評審的藝術——談談現實中的代碼評審
作者: 四火 發布時間: 2018-05-15 13:30
* * * * *
曾經寫過一點關于代碼評審（code review）的文章，比如這篇和這篇，現在覺得關于它的認識又有了不少更新。軟件工程的技術和實踐分成兩部分，一部分是和書本知識一致的，大約占一半，這部分基本上在大學里就可以學，自學只要方法得當、刻苦努力也可是途徑；但是第二部分來自于實際團隊、經驗，內容通常無法從書本當中獲得，而且難說對錯，不同的人和不同的經歷造就了不同的認識。代碼評審就是第二部分頗具槽點，可以大加討論的典型。
代碼評審是展現個性和性格的途徑
我本人特別反對一種頗為常見的觀點，就是“一個良好運作的項目，不同的人，應該寫出一樣的代碼”。我非常理解這種觀點的初衷，一個良好規范約束的團隊中，不同的人寫出來的代碼應當一致。但實際上，能真正這樣做的團隊，我還根本沒有見到過。所謂的一致代碼，仔細品味，發現不同的工程師寫出來就是不一致。因此“一致”這個詞一定是在一定程度內的大體描述而已，并非越一致約好。其實，代碼的創造本就是具備個性的。毫無疑問我們應當遵從規約，應當符合習慣，但是一旦試圖過度使用它們去約束代碼，不但難以落實，而且容易產生無趣、低效和矛盾的氛圍。
再回到代碼評審上。代碼評審本身，以及基于評審意見的來回溝通，和代碼本身比起來，要更難以，也更不應該要求一致。我見過許多代碼評審的風格，有人喜歡挑小毛病，有人喜歡展開觀點夸夸其談，有人喜歡給實際例子來證明自己的看法……無論哪一種風格，我都不覺得有什么大的問題。但是，就我個人而言，我可以談談我自己的代碼評審風格：
我會關注三種問題，**需求和業務上的問題、代碼結構的問題、代碼風格格式的問題**。
前兩種可能存在阻礙我ship代碼的“嚴重”問題（說“ship”就意味著認可代碼具備了push到主線分支的條件了）。我已經記不清多少次和代碼作者因為這樣的問題爭論了。爭論是個藝術，有時候并不一定能夠達成一致，有的人比較容易被說服，有的人則更堅持己見。我不想說哪一種更好，但是確實這是代碼評審中展現風格的事實——都是就事論事，但有人害怕或者不喜歡得罪人，就會顯得push over一點；有人則不在乎那么多，堅信自己的想法更合適，就會顯得defensive一點。我可能屬于后者，似乎在職業生涯的各種階段都會有和我出現代碼評審沖突的事例，但是在某些情況下我也會選擇disagree and commit（不同意但是執行團隊達成的意見）。我相信有些團隊會喜歡我的backbone，也會有團隊討厭我的這種風格。下面的內容，也多為基于自己風格的表述。
堅定自己的意見，但是委婉地表達
關于這一點，我也在努力地改進。可以提及的點很多，技巧也很多，但是老實說，沖突還是不可避免。在我經歷的幾乎所有的團隊中，有時候會有老好人，但是基本上所有的老好人都缺少對于原則的堅持。溝通是門藝術，在代碼評審中也一樣體現。
最重要的一條，只針對代碼，不針對人。這條很簡單，都知道對事不對人的重要性，但是要非常小心不能違背。
對于大多數代碼風格格式上的問題，我都會標注這個問題是一個picky或者nit的問題（“挑剔的問題”，這是我在Amazon的時候學到的方式）。這樣的好處在于，明確告知對方，我雖然提出了這個問題，但是它沒有什么大不了的，如果你堅持不改，我也不打算說服你。或者說，我對這個問題持有不同的看法，但是我也并不堅信我的提議更好。
使用也許、或許、可能、似乎這樣表示不確定的語氣詞（包括使用虛擬語氣）。這樣的好處是，緩和自己觀點表達的語氣。比如：“這個地方重構一下，去掉這個循環，**也許會更好**”。
間接地表達質疑。比如說，看到對方用了一個參數3，但是你覺得不對，但又不很確定，可以這樣說：“**我有一個疑問**，為什么這里要使用3而不是其他值？”對方可能會反應過來這個值選取得不夠恰當。
放上例子、討論的鏈接，以及其它一些輔助材料證明自己的觀點，但是不要直接表述觀點，讓對方來確認這個觀點。比如：“下面的討論是關于這個邏輯的另一種實現方式，不知你覺得是否會稍簡潔一些？”
先肯定，再否定。這個我想很多人一直都在用，先擺事實誠懇地說一些同意和正面的話，然后用however一轉，說出相反的情況，這樣也就在言論中比較了pros和cons，意味著這是經過trade-off得出的結論。
……
一些很常見的可以在代碼評審中提及的問題
這樣的問題其實有不少，多數和實現的技術無關，但是又很容易不小心略過。它們多數時候是問題，當然也有時候不是。
比如說，我最痛恨的之一，**職責過于寬泛或者職責不清的類**或模塊。我無數次見過這樣的類：Helper，或者Utils（注意，它們都沒有模型或者模塊前綴）。考慮項目的規模，大多數情況下，這樣的類很容易變成一個越吹越大的氣球，似乎什么東西都可以往里擱，是個十足的違反單一職責原則的糟糕設計。
比如說，在線程使用，容器使用，連接管理……中，缺乏上限控制的設計，在一些情況下導致資源使用過度膨脹。生產者和消費者的隊列設計，一旦消費者掛掉或者跟不上，隊列里越堆越多，沒有拒絕機制。
再比如說，缺少分包、分層，所有的東西都疊在一起，十幾個，甚至幾十個類文件并列在同一個文件夾下面。
再再比如說，代碼缺乏設計，流水賬結構，面條型代碼，或者簡單鋪陳幾個過程式的方法，這種修改往往代價還不小，自然修改的落實率低，因而令提出問題的人也頗為頭疼。
……
**避免一次評審提及過多的問題**
少數情況下，手里拿到一份實在是問題多到令人發指的代碼，這時候需要扼制自己想罵人的沖動。先抓問題的主干，不要去挑那些細枝末節的毛病，因為很有可能，這些代碼會被改得面目全非，或者重寫。寫一大堆評審意見，反而容易淹沒最重要的問題。
說說容易，但是這個度有時候并不好掌握。我的習慣是，在明確代碼要解決的問題以后，先快速走讀一遍代碼，判斷我是應該粗略地抓主要矛盾呢，還是應該嚴格地挑刺呢。我也見過一些別的方式。
不一樣的評審要求
我始終覺得，代碼評審的要求不應該完全一致。不要過度追求公平。對于新項目代碼，以及新員工寫的代碼，要適當嚴格一些。
新項目的代碼是形成后續風格和質量的模板。我們也許都聽說過“破窗戶原理”，在一塊干干凈凈的代碼田地里，新耕種的代碼才容易保持一致，也可以避免一些“為了和現有代碼風格保持一致”而導致質量妥協的情況出現。
新員工代碼的高質量要求則更多地在于對他們良好習慣養成的負責。軟件工程師良好職業習慣的養成，代碼可以說是最重要一環，而前三個月的影響舉足輕重。
還不如不做的評審
有些情況下，代碼評審是非常耗時費力的工作。特別是對業務背景不熟悉，對實現技術不熟悉，或者干脆是對方提交上來一大堆修改，閱讀非常費力。我不知道哪一種是最難的，但是如果因為這些原因很難做到良好的評審，我會在評審中說明，或者放棄一些評審的工作，保證評審的質量。
代碼評審是建立在團隊中影響的好方式。一方面是業務邏輯，一方面是代碼結構，我反對在兩方面都難以給出足夠清晰的評審意見的時候，提一堆風格方面的次要問題。否則很容易達成負面影響。
先談到這里，我想還有不少可以涉及的內容，而它們都難以在課堂和書本上學到。爭論才有意思，實踐出真知大抵如此吧。你有什么一致的看法，或者反駁，歡迎和我討論。
