關於程式碼評審(CodeReview)那些不得不說的事兒

xindoo 發表於 2022-05-24

  在一個成熟的團隊中,CodeReview是整個研發流程中不可或缺的一步,而那些即將走向成熟的團隊可能對CodeReview有很多的誤解和問題,也不清楚CodeReview該如何去做,本文筆者將結合自己的經驗和知識,談談我對CodeReview流程的一些理解和建議 。

什麼是CodeReview

  CodeReview 國內也稱程式碼評審或者程式碼審查,也簡稱CR,是指在軟體開發過程中,工程師對其他人所寫程式碼做審閱(後文統稱CodeReview),以達到控制程式碼質量的目的。通常的流程都是由程式碼寫作者發起,請團隊內其他人審閱程式碼,其他人對程式碼提出改進建議,再由程式碼寫作者修改重新提交,直至程式碼通過大家的審閱為止。

為什麼要做CodeReview?

在這裡插入圖片描述

  其實很多人都不是很重視CodeReview,因為CodeReview的效果短期很難看到,也很難量化衡量。就如同運動一樣,偶爾過量的運動不僅對身體無益,可能還會起反作用,不過長期的堅持肯定會讓你更健康,但能讓你健康多少很難量化,不過前一段時間github上爆火的專案《程式猿延壽指南》裡給出了可參考的資料,每週3次45分鐘揮拍運動可以減少全因死亡率47%,按其公式折算大概增加9年的預期壽命。運動除了增加預期壽命外,也能顯著減少很多疾病的發病率。堅持CodeReview如同堅持運動一樣,趨勢肯定是讓整個程式碼庫更為健康長壽。

CodeReview從長期來看,有幾個明顯的好處,接下來就一一講一下。

提升程式碼質量

  假如將一個系統比作一個生命體,一行行程式碼比作一個個細胞,不好的設計宛如癌細胞,會逐漸擴散,終將殺死系統。而CodeReview的過程就像是T細胞吞噬掉癌細胞,保證系統的健康成長。讓系統有更長久的生命力。

沒有人Review的程式碼,其程式碼水準就是寫程式碼人的水準,而被一個團隊Review過的程式碼,它的水準將接近甚至超過整個團隊的最高水準。

  因為單個人可能在某些方便做的比較好,集大家之所長就能在各個方面都做的比較好。另外,隨著CodeReview流程日常化,每個參與人的編碼能力也會逐步提升,無限趨近於團隊最高水準,因為在CodeReview的過程中,你可以看到別人做的好的地方,可以學習到經驗,也可以看到別人做的不好的地方,吸取到教訓。隨著時間的流逝,逐漸積累為參與者的能力。

提前發現問題

  在沒有CodeReview流程的時候,我們都是依賴於測試,甚至是依賴於功能上線後使用者暴露問題,這種發現方式已經偏晚了,尤其是讓使用者暴露問題的時候可能問題已經非常大了。問題暴露的越晚,風險也就越大。 而CodeReview一般是放在程式碼測試之前,如果能在這個階段就發現問題就能提前將各種風險扼殺在搖籃中。 但是,CodeReview真的能提前發現問題嗎?如果能的話,可以提前發現什麼樣的問題?

  我個人覺得CodeReview可以提前發現流程或者實現上的問題,尤其是在做業務需求的時候,不同工作經驗的人對同一個需求的理解肯定會有差異,程式碼實現上也會有很大的差異,有時候一點點的理解偏差,導致出現那種失之毫厘謬以千里的後果,寫程式碼的人也會因為處在自己的思維定式中發現不了問題,而別人可能因為經驗豐富或者需求相關背景瞭解更多,就能很輕易發現問題。像這種情況一般出現在團隊新人身上,他們對已有系統和業務瞭解不多,實現需求時很容易出現問題,這時候如果有人幫忙指出就能避免更嚴重的後果,另外也有助於新人瞭解業務和融入團隊。

  在CodeReview過程中另外一種問題也比較容易發現,就是那些別人之前踩過的坑。比如Java中SimpleDateFormat其實有執行緒安全的問題,不瞭解的人很容易就踩坑了。如果別人踩過這個坑,就可以在CodeReview過程中提前幫你指出來。 很多開源的類庫,甚至Jdk中很多包或多或少都有使用上的坑…… 這種例子就數不勝數了。

  當然CodeReview也不是萬能的,也有很多問題發現不了的,比如一些邊緣Case或者是程式碼計算結果的準確性,比如你程式碼實現了一個很複雜公式的計算,你總不能指望有人能通過看程式碼來發現問題吧。 這些還是老老實實去做測試吧!

經驗和知識的傳遞

  程式猿可以寫出能執行的程式碼,但真正的工程師才能寫出低bug、易擴充套件、易維護的高質量程式碼,更高階的工程師還能幫其他人成為一名合格的工程師。CodeReview的流程也是高階工程師來幫助其他人最直接的方式。

  在CodeReview中,你可以看到其他人寫出來的優秀程式碼、優秀的設計,甚至和改動相關的業務背景知識……,Review越多,學到的也越多。另外,即便是哪些不太好的程式碼,經過別人的Review,必然會留下很多評論或者是改動建議,甚至是別人之前踩的坑,你都能看到,從這些內容上你也可以學到很多新的知識。CodeReview不僅僅是一個提升程式碼質量方式,它也可以肩負起知識積累和訊息集散的功能。

  舉個我之前遇到過的例子,我們之前有個功能需要操作機器上的檔案,當然用Java的File也能實現,但是對操作多層級的資料夾時就很不方便了,需要自己寫很多的程式碼,搞不好還容易出bug。後來我發現apache-common包中提供了IOUtils類,可以很方便操作資料夾。我把這個寫到CodeReview的評論中,只要看到過的人都會知道原來有這個東西可以用。

  所以,Review別人程式碼時請毫不吝嗇地留下你的建議吧,另外,CodeReview時也不要忘記關注下別人的建議,說不定可以學到新的東西。

如何做好CodeReview

  說完了CodeReview的好處,相信你肯定已經躍躍欲試了,如何能做好CodeReview?這裡我根據我的知識和經驗分享下我的看法。

CodeReview的步驟

瞭解改動的背景

  CodeReview不是一上來就看程式碼,這樣有可能你會看的雲裡霧裡,純粹是浪費時間。 CodeReview雖然是Review程式碼,但是首先你的知道你要看的程式碼實現了什麼樣的功能,是在什麼樣的背景下去做的,清楚前因後果之後,你才能知道這個程式碼大概應該怎麼去寫,你才能更好的去Review別人的程式碼、去發現別人的問題。

縱觀全域性

  知道背景之後,在你腦海中就會有一個大概的編碼思路,也有個流程主線。這個時候可能有兩種情況,你和寫程式碼人的思路相同,那你就順著你們共同的思路去幫忙Review整個流程是否正確。另一種情況就是你們思路不同,你就得看程式碼去了解寫作者的思路,然後確認是誰的思路有問題,或者是誰的思路更好,然後同寫作者一起將這個流程優化到更優。

逐層細化

  確定完整個流程之後,就可以逐步深入到程式碼細節中了,細節可以Review的地方就很多了,可以看下下一節的內容,這裡就先不展開了。

CodeReview的關注點

  在CodeReview的過程中,如果有一些立足點的話可以幫助大家更好的完成CodeReview的過程,我大概總結出以下幾點:

  • 功能性: 程式碼所實現的功能是否和預期一樣,是否實現了所有必須的功能?
  • 複雜性: 功能實現是否過於複雜? 過於複雜的程式碼更容易出問題,而且可維護性也會更低。
  • 程式碼風格: 程式碼是否符合團隊編碼規範?
  • 文件&註釋: 如果程式碼功能有改動,關注下相關文件和註釋有沒有同步改動。 錯誤的註釋和文件可能會讓未來的開發者產生理解成本。
  • 程式碼亮點: 如果你看到變更中做得好的地方,也別吝嗇你的讚美。

上面描述更偏概括性,我們來舉一些更詳細的例子,幫助大家理解上面幾點。

  • 程式碼設計良好,可讀性和可維護性高。
  • 是否有執行緒安全的bug。
  • 程式碼沒有增加不必要的複雜性。
  • 開發者沒有寫一些目前沒有在用的程式碼,這種無用的程式碼未來大概率也不會用,所以不要假設。
  • 程式碼有適當的單元測試。
  • 測試邏輯設計良好。
  • 開發者使用了清晰明瞭的變數和函式命名。
  • 註釋清晰明瞭且實用,並且解釋清楚了為什麼這麼做,而不僅僅是做了啥
  • 程式碼有相應完善的文件。
  • 程式碼風格符合規範。
  • 程式碼有沒有更優的實現?(效能、資源佔用……)
  • ……
    更多更細節的內容,可以參考谷歌工程實踐——程式碼評審

注意事項

在這裡插入圖片描述

CodeReview的禮節

  首先CodeReview不是你個人炫技的舞臺,看到別人程式碼的問題需要禮貌指出,切忌diss別人。其次,CodeReview不要為了提問題而提問題,有些程式碼就是沒問題,你也沒必要糾結必須要提個問題,直接通過即可。 這裡在額外說一點,大部分Review別人的程式碼,只關注到別人做的不好的地方,而忽視了別人做的好的地方,遇到好的程式碼、好的設計也別忘記點個贊。

CodeReview應當及時

  別人提的CodeReview應當及時處理,在很多公司中CodeReview是開發流程中必要的一環,沒有Review通過的肯定是不能上線的,如果CodeReview長時間不處理可能會延誤後續的流程。 另外一點,CodeReview是相互的,今天你及時幫別人Review了,明天別人也會及時幫你Review,與人方便即與己方便

如何寫出對CodeReview友好的程式碼

程式寫出來是給人看的,附帶能在機器上執行。 ——Harold Abelson

時刻謹記上面這句話。

提交前先做好自審

  提前自己處理掉一些低階錯誤,可以先借助一些工具完成一些簡單的檢查。例如我們團隊會藉助checkstyle、spotbug、sonar、pmd等工具完成程式碼風格和一些潛在bug的檢查。然後自己做好功能的自測,儘可能先消滅一部分bug,為CodeReview減少一些負擔。

寫清楚變更描述

  這裡對應上文中CodeReview步驟中的瞭解改動背景,作為程式碼的寫作者,你不能一上來就讓別人從程式碼入手吧,可能看你程式碼的很多其他人都缺少程式碼改動相關的上下文資訊,完全理解程式碼成本很高,所以需要在變更描述中將這些資訊描述清楚,包括但不僅限於改動背景、改動點、流程、具體設計…… 一句話概括就是好的變更描述應該說清楚這次是做了什麼變更,以及為什麼要這麼做

  更詳實的變更描述可以讓Review程式碼人減少理解成本,更快完成CodeReview的流程,你程式碼改動也就能更快進入後續流程了。

單個變更儘可能短

  一個非常大的變更幾乎沒有Review,因為大的改動首先就很難Review,其實也更耗費時間,還有出問題的概率也更大,誰Review通過了程式碼但之後上線出問題,可能是要和你一起背鍋的。所以建議大家將大的程式碼變更儘可能拆小,因為小的變更有以下這些好處:

  • 程式碼評審更快 與比起花30分鐘評審一個大的變更相比,對Review程式碼的人來說花5分鐘審查一系列小的變更更加容易.
  • Review更加徹底。 進行較大的更改後,審閱者和作者往往會因大量詳細評論的來回移動而感到沮喪,有時這些評論會漏掉或遺漏重要的觀點。
  • 減少導致bug的可能性。 由於您所做的更改較少,因此您和您的審閱者更容易有效地推斷出CL的影響,並檢視是否導致bug。
  • 減少不必要的工作 當你寫了一個巨大的變更,然後評審者覺得你總體方向錯了,這會導致你浪費大量的工作。
  • 更方便合併程式碼 因為大型的變更會導致大量的衝突,因此合程式碼的時候會耗費很多時間,而且可能因合併程式碼導致問題,我們就出現過好幾次程式碼合併的時候沖掉別人程式碼的情況。
  • 有助於你作出更好的設計 優雅的設計並且完善一個小的改動比大的改動更加容易
  • 降低評審者的難度 提審部分改動,不會影響你繼續編碼。
  • 如果真出問題,回滾更容易 這個就不用多解釋了吧。

  對於那些很難拆分的變更,也不是需要完全禁止,有時候就是有改動非常大,而且無法再去拆分了,這時候還是建議通過其他方式來保證程式碼質量,比如全流程測試。 然後也建議做好出問題時的快速恢復方案。

注意事項

注意自己的情緒

  程式碼的寫作者在CodeReview的流程中是弱勢的一方,很多人程式碼在收到評論被要求要修改程式碼的時候,都有一種不太願意接受的心態,其實我自己也經常會有這種心態,尤其是在時間比較緊張的情況下。 其實這是很正常的心態,但是還是需要去控制自己的情緒的,從評論者的角度出發,他肯定也不是在刁難你,也是希望程式碼質量能更高。如果真遇到這種情況,首先可以和評論者討論清楚他的要求是否合理,如果確定了合理性當然還是要改的。但如果在時間比較緊張的情況下,協商是否可以不修改或者是之後再修改,確實有些錦上添花的修改沒必要阻塞之後的流程。

CodeReview流程應該儘早提交

  日常情況下,還是建議早點提交CodeReview,並留出一定的時間來做修改,儘可能不要讓這個流程變的匆忙。 大部分公司的實踐都是在進入測試流程的時候同時進入CodeReview流程。

關於CodeReview的幾個誤區

在這裡插入圖片描述

CodeReview是純浪費時間?

  如果你的團隊剛開始推行CodeReview流程,這個問題肯定是會被很多人問到的。 所以是不是呢? 這裡我拿鍛鍊身體類比下,大家都知道堅持運動對健康有益,但如果只是一時興起,偶爾過量的運動不僅對身體無益,可能還會起反作用。實際上,堅持運動對健康的影響其實很難量化,但我們現在都知道,運動是好的。同樣,CodeReview如同運動一樣,它可能會耗費一定的精力和時間,但長期堅持下來必然會讓整個程式碼庫、整個系統甚至這個團隊更加健康。

  另外,如果一個團隊CodeReview機制很成熟,寫程式碼的人隨著被Review的次數增加,其程式碼質量必然也會逐步提交,那麼程式碼中的問題肯定也會逐步減少,隨之而來的就是CodeReview的過程越來越輕鬆。

困難的路會越走越簡單,而簡單的路會越走越困難。

工期很緊,沒時間做CodeReview!

  這也是很多團隊不做CodeReview的藉口。 你不做CodeReview省下的時間,會在後面測試,甚至是在後面長期維護中花費更多的時間。 我們有句耳熟能詳的老話叫做“磨刀不誤砍柴工”,其實CodeReview和測試在軟體開發過程中就是“磨刀”的工作。

做好事前控制而不是事後彌補。 因為時候彌補的代價會非常高,

只有高階工程師才有資格Review別人程式碼?

  雖然事實上幾乎都是高階工程師在Review別人的程式碼,我理解造成這種現象的原因其實也比較簡單,高階工程師確實經驗豐富一些,也更容易Review出別人程式碼中的問題,所以會留下更多的Review記錄,造成只有高階工程師才參與CodeReview的假象。 但這絕不意味著初級工程師沒有資格Review別人的程式碼,所謂三人行必有我師,即便是初級工程師也有可能發現Review出別人程式碼中的問題。

  即便你暫時確實經驗不夠豐富,很難找出別人的問題,但你也可以從別人Review程式碼的過程中學到很多東西,就像上文所說CodeReview也是團妒經驗和知識的一種傳遞方式。 參與CodeReview也是成為高階工程師必不可少的一步。

都有測試流程了,為什麼還要做CodeReview?

  這個問題算是已經在上文中回答過了,CodeReview需要看很多個方面的內容,而測試只能保證其結果的準確性。

有了CodeReview就不需要測試了?

  問這個問題的人相比於問上個問題的人又走了另一個極端,又把CodeReview和測試放在了對立面上。雖然CodeReview如果做的比較好的話,確實能提前發現一些比較明顯的問題,但是做CodeReview的是人,人是無法大規模且精準的去校驗程式的執行過程,而這恰恰是自動化測試所擅長的,所以說CodeReview和測試並不是對立關係,而是一種互補關係。

只要我在團隊推行了CodeReview流程,程式碼質量就會迅速提高?

  首先可以肯定的是程式碼質量是會提高的,但也許沒有那麼快。我之前從一個沒有CodeReview流程的團隊加入到一個有嚴格CodeReview流程的團隊時,有較長一段適應期。我當時最大的感受就是CodeReview太耗時間了,尤其是我剛開始還沒適應新團隊開發規範的時候,寫程式碼和被Review後改程式碼所花費的時間基本上五五開,當然之後會好很多。另外,每個人也需要花20%左右的時間和精力去Review別人的程式碼,相當於花接近三分之一的開發時間來處理CodeReview相關工作,這個比例高到可能你們老闆都會懷疑,但我們當時就做到了,結果就是我們的程式碼質量非常高。

  以我的經歷來看,如果CodeReview想要有效果,時間投入肯定是少不了的,冰凍三尺非一日之寒,滴水石穿非一日之功。不要高估它的短期價值,也不要低估它的長期價值。

參考資料

  1. 谷歌工程實踐——程式碼評審

  今天的分享都到這裡了,大家也都知道現在外面大環境不好,這個時候更不能停下鬆懈的腳步。劉易斯·卡羅爾在《愛麗絲夢遊仙境》中借紅桃皇后之口說出了下面這句話,現在看來也是出奇的合適呢!

在我們這個地方,你必須不停地奔跑,才能留在原地。如果你要抵達另一個地方,必須以雙倍於現在的速度奔跑。
——【英】劉易斯·卡羅爾《愛麗絲夢遊仙境》