天天看點

代碼評審(CR)實踐指南為什麼要做代碼評審高效高質的代碼評審一些可以用來數字化的CR名額

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

  • “政治正确”的代碼評審活動究竟有沒有達到期望的實際效果?
  • 給了我一大堆代碼,到底該從哪裡看起?哪些方面是我該評審的?哪些不是?
  • 别人有沒有認真評審我的代碼?如何讓别人更容易的評審代碼?

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

1, 了解代碼評審的核心目标,建立關于代碼評審的正确預期。

2,了解代碼評審為什麼可能無效,并采取有針對性的實踐來提升代碼評審的效果。

為什麼要做代碼評審

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

CR是代碼規範性的保證

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

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

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

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

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

代碼評審強制提前了這個回報周期,代碼編寫完成之後,就立即有了一位或多位讀者,他們是這個代碼的Reviewer。是以,這段代碼已經在編碼完成之後,立即經曆了可讀性的檢驗。更理想地,如果組織已經有了編碼規範和設計規範,還能確定這段代碼遵循了這些規範。如果這時候發現這段代碼沒有遵循規範,那更是好事,它指向了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個。

快速響應

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

找對人:合适的Reviewer

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

  • 如果你的組織有Owner機制,Owner應該是合适人選
  • 和你工作在相同上下文的同僚
  • 近期修改過相同代碼的同僚
  • 比你更資深的程式員,希望得到他們的專業回報

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

合适的工具

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

考慮結對程式設計

當我們提到“小批量、多批次、快速回報“的時候,如果有過結對程式設計經驗的同學,馬上就會反映過來,這就是一種試圖接近結對程式設計的形式。

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

綜合線上Review和線下Review

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

一些可以用來數字化的CR名額

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

從Author角度:

  • 單次變更的代碼行數 (主要名額)
  • 變更的頻度 (參考)

從Reviewer角度:

  • 響應時間
  • 評論數和拒絕率

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

不要做什麼:

CR的本質是文化建設,強烈建議僅僅把CR的名額用作提升指引,而不要用于和績效有關的評價。無論是前述的幾種名額,還是和Review的品質甚至是缺陷相關的資料。

繼續閱讀