7 個建議讓 Code Review 高效又高質

Honins發表於2020-12-31

阿里妹導讀:Code Review(CR) 的本質是什麼?是為了查錯?還是為了 KPI?本文分享阿里資深技術專家的看法:CR 是一種關於社會學的長期行為和組織文化,通過 CR,形成一種良性互動的技術氛圍,傳播和分享知識,提升程式碼質量,並給出了 7 個提高 CR 效率和質量的實踐建議。

關於程式碼評審(Code Review)的文章也算是汗牛充棟了,程式碼評審也已經是許多組織的標準化實踐。不過,許多團隊在嘗試程式碼評審實踐時,卻有如下疑問:

“政治正確” 的程式碼評審活動究竟有沒有達到期望的實際效果?

給了我一大堆程式碼,到底該從哪裡看起?哪些方面是我該評審的?哪些不是?

別人有沒有認真評審我的程式碼?如何讓別人更容易的評審程式碼?

這些都不是什麼新問題,但是它們是如此的普遍,而且經年累月地在不同的上下文中被提起,不外乎兩個方面:

理解程式碼評審的核心目標,建立關於程式碼評審的正確預期。

瞭解程式碼評審為什麼可能無效,並採取有針對性的實踐來提升程式碼評審的效果。

為什麼要做程式碼評審

不少同學認為程式碼評審就是用來查錯的,甚至希望用程式碼的缺陷數量來檢驗程式碼評審的效果。這低估了程式碼評審的價值。程式碼評審最本質的作用不是問題發現。除了程式碼評審,我們有更多更好的手段來發現問題。程式碼評審的作用更多是關於社會學的,是一種長期行為和組織文化。

CR 是程式碼規範性的保證

編碼者視角:良性的社交壓力

你正在緊張地編碼,交付時間迫在眉睫。你的組織對程式碼的單元測試有一個要求:凡是新增的程式碼,必須有完整的自動化單元測試。但是,在壓力之下,你想給自己降低一點要求,不寫這部分的單元測試了,以後再編寫吧,或者為了應付工具的覆蓋率要求,先寫一點不那麼有用但是卻能帶來覆蓋率的測試(例如沒有斷言的測試)。

但是,一旦想到你的程式碼將會發出去給你的同事做 Review,有沒有為剛才的這種想法產生一絲絲壓力?這種壓力是良性的,它能給你帶來一種即時的反饋,阻止你去選擇那些短期收益、長期損失的 “投機” 行為。如果沒有程式碼評審這個環節,或許你就會真的 “為所欲為” 了,其實最終還是要為這種取巧行為買單。

維護者視角:程式碼可讀性的保證

有許多方式能實現同一個軟體需求。有興趣的讀者可自行搜尋 “Hello World 的 N 種寫法”。儘管條條大路通羅馬,但是,不同的道路代價是不一樣的。小到變數命名,大到設計結構,如果你採用的是一種不那麼常見的做法,往往就是給後來的程式碼維護者挖坑。這種維護活動可能發生在 1 個月以後,也可能發生在 1 年以後,甚至是更久之後。甚至那時候,作為作者的你,已經不在這個團隊了,已經沒有人能理解當時的軟體為什麼這樣設計。

程式碼評審強制提前了這個反饋週期,程式碼編寫完成之後,就立即有了一位或多位讀者——他們是這個程式碼的 Reviewer。所以,這段程式碼在編碼完成之後,立即經歷了可讀性的檢驗。更理想地,如果組織已經有了編碼規範和設計規範,還能確保這段程式碼遵循了這些規範。如果 CR 過程中發現這段程式碼沒有遵循規範,那更是好事,它指向了 CR 的另外一個關鍵價值:知識傳播。

CR 帶來了知識傳播和設計共識

你可能是一個團隊的 Leader,正在為如何提升團隊成員的程式設計能力發愁。你希望他們去讀書,所以你介紹了諸如《整潔程式碼》之類的入門書籍,你還介紹了經典名著《設計模式》,還推薦了《領域驅動設計》。你也希望團隊成員能理解產品的業務邏輯,所以希望團隊成員週期性地進行業務分享。

所有這些努力都很好。但是,也有可能你會被打擊。一個月過去了,似乎團隊成員對命名規範都建立了概念,但是在怎麼命名這件事上,大家並未形成共識。不少同學已經瞭解了一些設計模式,但是有人過度運用模式,搞得程式碼臃腫不堪,有人則只知道singleton。團隊成員為什麼是實體物件,什麼是值物件爭的不可開交,沒有人說得清楚聚合是什麼,應該在什麼場景下使用。

你真正缺乏的,是一個場景。抽象的概念如果不落到具體的事情上,就很難形成共識。有人或許知道海洋法系的 “判例”,這是在法律層面形成共識的一種非常好的方法。程式碼評審,其實也是在形成判例:哪一類設計是合理的,哪一類設計是不合理的。通過針對具體的問題進行分析,團隊就會逐漸形成設計共識,在過程中,對這些共識不那麼熟悉的新同學,也可以慢慢融入。

檢驗邏輯正確性

當然,CR 也能用來檢驗邏輯正確性。

保證程式碼邏輯正確,是設計者的責任

為了不讓 CR 被濫用並被寄予過高期望,我們在此首先申明一點:保證程式碼的邏輯正確,是設計者的責任。出現了一個空指標錯誤,究竟是編碼做的不好,是 CR 做的不好,還是測試做的不好?首先肯定是程式碼作者製造了這個問題。把這個板子打在 Reviewer 身上公平嗎?或許,Reviewer 確實有責任發現這樣的問題,但是,如果程式碼本來就錯誤多多呢?如果一次性 Review 了 1000 行程式碼,根本看不過來呢?我能找到一大把的理由,來說明為什麼漏掉這麼一個空指標錯誤。

發現邏輯錯誤的其他方法

你還有許多其他的方法來發現錯誤,它們的成本往往並不高,例如:

編寫自動化單元測試

使用程式碼靜態檢查工具

無論是否存在 CR 活動,上述兩點都是一名專業的開發者和開發組織應該大力倡導的行為。

發現錯誤的價值

在上述兩點得到承認的前提下,程式碼評審確實也應該用於發現錯誤——它本質上建立了一種冗餘機制,通過多人工作在同一段程式碼上,發現程式碼中可能發生的認知錯誤(這對於單個開發者往往是很難發現的)以及疏忽。

高效高質的程式碼評審

哪些因素阻礙了程式碼評審的效果

程式碼評審本身並不困難,但是,如果考慮到如下因素,可能就比較複雜了:

你可能對要評審的程式碼的設計上下文一無所知

你可能非常忙碌

你一下子收到了幾千行需要被評審的程式碼

實際操作建議

建議一:小批量——每次 Review 的程式碼量要少

研究發現, 成功的 CR 活動一定是小規模的。例如,《Modern Code Review:A Case at Google》論文介紹說, Google 的 CR 活動中,有 35% 的 CR 僅僅修改了一個檔案,90% 的 CR 修改的檔案數在 10 個檔案以內,甚至有 10% 的 CR 僅僅修改了1行程式碼。

程式碼量少的好處顯而易見:修改了哪裡非常清晰,問題也會一目瞭然。一次推給別人 1000+ 行程式碼,還想得到有價值的 Review,可能性微乎其微。

當然,一次 Review 它代表的功能應該是有意義的,是完整的,如果不是修復缺陷,這一定程度上也對開發者迭代地開發功能的能力提出了要求。

建議二:多批次——Review 要頻繁發生

小批量必然導致了多批次。在微軟 2013 年的一篇論文《iExpectations, Outcomes, and Challenges Of Modern Code Review》和前述的 Google 的論文中都提到了頻繁 Review 的做法。其中,Google 的每週每 Developer 的程式碼變更中位數是 3 個,每週每 Reviewer 的 Review 中位數是 4 個。

建議三:找對人——合適的 Reviewer

誰適合 Review 你的程式碼?選一個和被 Review 的程式碼毫不相干的人肯定是不明智的。下面列出了一些潛在的候選人:

如果你的組織有 Owner 機制,Owner 應該是合適人選

和你工作在相同上下文的同事

近期修改過相同程式碼的同事

比你更資深的程式設計師,希望得到他們的專業反饋

現在已經有一些工具,能夠根據上下文推薦 Reviewer,這也為選擇合適的 Reviewer 提供了便利。

建議四:快速響應

當每次 Review 的粒度不大,Review 又比較頻繁時,快速響應才能成為可能,也是必然的要求。在這個資料上,Google 的中位數是 4 小時。這個指標可以成為一個較好的參照。

建議五:使用現代工具

快速響應、高質量的 Review 離不開現代工具。現代的 Review 工具能自動整合進工作流,高亮變化,甚至能自動彙總變更。這方面已經有許多現代的工具可以使用,還沒有選好工具的讀者,可以自行搜尋。

建議六:考慮結對程式設計

當我們提到 “小批量、多批次、快速反饋” 的時候,如果有過結對程式設計經驗的同學,馬上就會反映過來,結對程式設計和 CR 何其相似。

結對程式設計,共同程式設計的兩位同學擁有完全相同的上下文,不存在上下文切換的煩惱,沒有缺乏時間的煩惱,不需要藉助額外的工具,反饋隨時隨地。事實上,在我的眼中,結對程式設計才是最好的 Code Review。

建議七:綜合線上 Review 和線下 Review

線上 Review 應該是常態化的行為。考慮到 CR 的 “知識傳播” 價值,線下 Review 是有益的補充。有經驗的團隊,會週期或者不定期的組織線下 Review,這樣能獲得比線上 Review 更為廣泛的知識傳播面,也能引起更為熱烈的討論和辯論,有助於形成更高質量的共識。

數字化的指標

研發行為的全面數字化,帶來了一些有價值的資料洞察。如果工具支援,可以通過一些指標的觀測,持續推進 CR 活動。我們把一些建議的指標和資料總結如下:

從 Author 角度:

單次評審的程式碼行數 (主要指標)

評審的頻度 (參考)

從 Reviewer 角度:

響應時間

評論數和拒絕率

從 Reviewer 的團體角度,還可以發現 Author-Reviewer 之間的社群關係,也是一種有價值地瞭解知識傳播的資訊。

不要做什麼:

避免用 CR 的數字化指標進行考核,即使動機良好。CR 的本質是文化建設,強烈建議僅僅把 CR 的指標用作提升指引,而不要用於和績效有關的評價,無論是前述的幾種指標,還是和 Review 的質量甚至是缺陷相關的資料。

實踐

你的組織的程式碼質量和技術氛圍如何?你們正在實施程式碼評審嗎?

如果暫時還沒有,是否願意做一些嘗試?

如果已經在實施,既有的實踐是否達到了程式碼評審的真正目標?

相關文章