程式碼審查的實踐經驗

edithfang發表於2014-10-24



數百萬年前,猿從樹上下來,進化出了對生拇指,最終,變成了人類。

我們以類似的眼光來看下強制性程式碼審查(Code Review):好像是一種能在軟體開發這塊廣闊的領域裡將人類從獸裡分離出來的東西。

不過,我有時候會從我們的團隊成員裡聽到下面這樣的評論:
  • “這個專案的程式碼審查根本就是浪費時間。”
  • “我沒有時間做程式碼審查。”
  • “我的專案釋出延期了,都是因為我那懦弱的同事還沒有做任何審查。”
  • “你能相信我的同事竟想讓我在程式碼中改點東西嗎?請向他們解釋:如果我那最初的優雅程式碼受到任何方式改動的話,那就意味著宇宙微妙的平衡將要遭到破壞。”
為什麼我們要做程式碼審查?
首先,讓我們謹記為什麼要做程式碼審查。對於任何專業的軟體開發人員來說,最重要的目標之一是能夠持續的提高他們的工作質量。即使你的團隊裡盡是優秀的程式設計師,你也不能將你自己與一個有能力的自由從業者區分開來,除非你能夠作為一個團隊工作。程式碼審查是達到這個目的的最重要方式之一。尤其,它們:
  • 給予你第二雙眼睛來找到做某些事的瑕疵和更好的方法。
  • 確保至少有一個其他人員熟悉你的程式碼。
  • 通過向新員工展示更有經驗的開發者的程式碼來幫助訓練他們。
  • 通過讓審查者和被審查者互相展示好的想法和做法以促進知識分享。
  • 鼓勵開發者在他們的工作中更加盡心盡力,因為他們知道自己的程式碼將來要被他們的的某個同事審查。
做徹底深入的審查
不過,如果不在審查工作上傾注一定的時間和精力,這些目標都是無法實現的。僅僅滾動瀏覽下patch,確保縮排正確、所有的變數採取小駱駝拼寫法並不能構成一次徹底的審查。受到業界的啟發也可以考慮結對程式設計,這是一個相當流行的做法,但也在所有的開發時間上增加了100%的額外開銷來作為程式碼審查工作的基準。你可能會在程式碼審查中花費很多時間,但與結對程式設計相比,使用的總體工程時間仍少得多。

我認為花在程式碼審查工作上的時間應該是原開發時間的25%左右。例如,如果一個開發者花兩天時間實現了個小專案,那麼審查者應該花大致4個小時的時間來審查它。

當然,花在審查工作上多少時間並不是最重要的,只要審查能夠準確無誤的完成即可。特別地,你必須要能理解你正在審查的程式碼。這不僅僅意味著你只要懂該程式碼所採用語言的語法即可,它還意味著你必須瞭解該程式碼如何適應於更大的應用環境、元件或庫下。如果你不抓住每一行程式碼的全部含義,那麼你的審查就不是非常有價值的。這也是為什麼好的審查都不可能非常快的完成:因為還要花時間去調查觸發某個給定函式的不同程式碼路徑,要去確保第三方API能夠正確使用(包括任何邊緣情況),等等。
除了尋找你所審查的程式碼中的瑕疵或其它問題之外,你還應該確保:
  • 包含所有必要的測試。
  • 合適的設計文件已經寫完。
甚至擅長寫測試和文件的開發人員也並不總能記得在程式碼改動之後及時更新。在適當的時候來自程式碼審查人員的細微調整對於確保程式碼在隨著時間的推移不會變質是至關重要的。
防止程式碼審查工作超負荷
如果你的團隊強制要求做程式碼審查,那這是有風險的,因為你的程式碼審查工作可能一直積壓,最終到無法管理的地步。如果你兩週之內不做任何審查工作,你可以很容易的花上幾天時間來趕補它。不過這也意味著當你最終決定去處理它們的時候,你自己的開發工作將遭到一定的意外擱淺。這也使得做好審查工作更加困難,因為正確的程式碼審查需要強烈、持續的腦力勞動,很難這樣數日保持下去。

因此,開發者每天應該竭盡全力的清空他們的審查積壓工作。一個方法是早晨的第一件事情就用來解決審查工作。在開始自己的開發工作之前先做完所有的優秀審查工作,你可以防止以後的審查局面失控情況。有些人更喜歡在午休之前或之後或在一天結束後做審查工作。無論你什麼時候做這些事情,通過將程式碼審查作為正規的日常工作而不是作為一種分散注意力的工作,你可以避免:
  • 沒有時間處理你的審查積壓工作。
  • 因為你的審查工作還沒做完而延遲專案的釋出。
  • 做出一些不再相關的審查,因為在此期間程式碼已經改動的非常多。
  • 因為趕在最後一分鐘處理它們而導致審查工作最終完成的很差。
寫易於審查的程式碼
無法管理的審查積壓工作也不能全怪審查人員。如果我的同事不管三七二十一的花費一週的時間來給一個大工程專案新增程式碼,那麼他們釋出的patch將真的很難審查,因為在一個階段裡有太多的工作要處理,程式碼的目的和底層架構體系也會很難理解。

這是將你的工作切割為一個個可管理單元之所以非常重要的眾多原因之一,我們使用scrum管理方法,所以對我們來說合適的單元是重點。通過一起努力,用單元來組織我們的工作,並提交僅與我們正在進行的某個單元相關的審查,我們可以寫出更加易於審查的程式碼。你的團隊可能使用另一種管理方法,但是原則都是一樣的。

為了寫出易於審查的程式碼,還有一些其它的必備條件。如果要做出一些很棘手的架構決策,為滿足審查者的要求,事先進行討論是合理的。這將使得審查者更加容易的理解你的程式碼,因為他們將知道你在程式碼中試著達到什麼目的以及怎麼計劃來達到該目的。這也有助於避免這樣一種情況:在審查者提出一個不同的更好的方法後,你必須要重寫你的大段程式碼。

在你的設計文件裡專案架構應該要詳細的描述。這無論如何都是很重要的,因為它能讓一個新的專案成員很快的趕上進度並理解現有的程式碼庫。它還能幫助審查者更好的做好自己的工作,這是另一個好處。單元測試也有助於向審查者說明元件應該如何使
用。

如果你的patch裡包含了第三方程式碼,請單獨提交。例如當jQuery的9000行程式碼被插入程式碼中間時,要做好程式碼審查工作就難上加難了。

寫出易於審查的程式碼的最重要步驟之一是給你的程式碼審查部分新增註釋。這表示你可以自己瀏覽審查部分,並在任何你覺得有助於審查者理解程式碼意思的地方新增註釋。我發現這樣的註釋僅花費相對較少的時間(經常僅幾分鐘的時間)卻能產生巨大的作用,能讓程式碼審查工作完成的更快、更好。當然,程式碼註釋也有許多相同的優點,應該在合適的地方使用,但是通常來說審查註釋更為明智。最後可以說是一個獎勵吧, 研究表明,當開發者重新閱讀和註釋程式碼時,竟然發現他們自己的程式碼裡有很多的瑕疵。
龐大的程式碼重構
有時有必要重構能影響許多元件的某個程式碼庫。對於一個龐大的應用程式,這個過程可能花費好幾天(甚至更久)且導致龐大的補丁。在這些情況下一個標準的程式碼審查工作可能是不切實際的。

最好的解決方法是遞增式重構程式碼。在工作程式碼庫的合理範圍內找到能達到你目的的某個改動點。一旦改好了,review通過了,接著進行下一個改動,直到整個重構工作完成。這個方法可能並不是每次都行得通,但是有想法和計劃,在重構時要避免巨大的補丁通常是實際可行的。像這樣來重構程式碼可能要花開發人員更多的時間,但它同時也產生了更好的程式碼質量和更容易的審查工作。

如果真的實現不了遞增式重構程式碼(這可能要說一些關於如何寫好和組織好原始碼的事情),一個可能的解決方案是當進行重構工作時用結對程式設計來代替程式碼審查。
解決爭議
你的團隊無疑是由一群聰明的專業人士組成。當大家對某個確定的編碼問題觀點不同時,基本上都會產生爭議。作為一名開發人員,保持開放的心態,在你的審查者更傾向於一個不同的方法時要隨時準備妥協。不要對你的程式碼持專有的態度,也不要帶個人審查意見。如果僅僅是因為有人覺得你應該將一些重複的程式碼重構為一個可重複利用的函式時,這並不能表明你就不是一個有吸引力的、出色的和有魅力的人。

作為一個審查者,一定要機智。在改變建議之前,認真考慮下是否你給的提議真的更好或僅僅只是你個人風格問題。如果你選擇的戰場集中在一些原始碼中明顯需要改進的區域,你將能獲得更多的成功。說一些諸如“考慮下……可能是值得的”或“有人建議……”的話更適合,而不是“連我的寵物倉鼠都能寫出一個比這更高效的排序演算法”。

如果達不到一箇中間立場(即雙方都不願意妥協),那麼就邀請一個雙方都尊敬的第三方開發人員過來看看,讓他們給出一些觀點和建議。
相關閱讀
評論(1)

相關文章