代碼審查的實踐經(jīng)驗
數(shù)百萬年前,猿從樹上下來,進化出了對生拇指,最終,變成了人類。
我們以類似的眼光來看下強制性代碼審查(Code Review):好像是一種能在軟件開發(fā)這塊廣闊的領(lǐng)域里將人類從獸里分離出來的東西。
不過,我有時候會從我們的團隊成員里聽到下面這樣的評論:
- “這個項目的代碼審查根本就是浪費時間。”
- “我沒有時間做代碼審查。”
- “我的項目發(fā)布延期了,都是因為我那懦弱的同事還沒有做任何審查。”
- “你能相信我的同事竟想讓我在代碼中改點東西嗎?請向他們解釋:如果我那最初的優(yōu)雅代碼受到任何方式改動的話,那就意味著宇宙微妙的平衡將要遭到破壞。”
為什么我們要做代碼審查?
首先,讓我們謹記為什么要做代碼審查。對于任何專業(yè)的軟件開發(fā)人員來說,最重要的目標之一是能夠持續(xù)的提高他們的工作質(zhì)量。即使你的團隊里盡是優(yōu)秀的程序員,你也不能將你自己與一個有能力的自由從業(yè)者區(qū)分開來,除非你能夠作為一個團隊工作。代碼審查是達到這個目的的最重要方式之一。尤其,它們:
- 給予你第二雙眼睛來找到做某些事的瑕疵和更好的方法。
- 確保至少有一個其他人員熟悉你的代碼。
- 通過向新員工展示更有經(jīng)驗的開發(fā)者的代碼來幫助訓練他們。
- 通過讓審查者和被審查者互相展示好的想法和做法以促進知識分享。
- 鼓勵開發(fā)者在他們的工作中更加盡心盡力,因為他們知道自己的代碼將來要被他們的的某個同事審查。
做徹底深入的審查
不過,如果不在審查工作上傾注一定的時間和精力,這些目標都是無法實現(xiàn)的。僅僅滾動瀏覽下patch,確??s進正確、所有的變量采取小駱駝拼寫法并不能構(gòu)成一次徹底的審查。受到業(yè)界的啟發(fā)也可以考慮結(jié)對編程,這是一個相當流行的做法,但也在所有的開發(fā)時間上增加了100%的額外開銷來作為代碼審查工作的基準。你可能會在代碼審查中花費很多時間,但與結(jié)對編程相比,使用的總體工程時間仍少得多。
我認為花在代碼審查工作上的時間應該是原開發(fā)時間的25%左右。例如,如果一個開發(fā)者花兩天時間實現(xiàn)了個小項目,那么審查者應該花大致4個小時的時間來審查它。
當然,花在審查工作上多少時間并不是最重要的,只要審查能夠準確無誤的完成即可。特別地,你必須要能理解你正在審查的代碼。這不僅僅意味著你只要懂該代碼所采用語言的語法即可,它還意味著你必須了解該代碼如何適應于更大的應用環(huán)境、組件或庫下。如果你不抓住每一行代碼的全部含義,那么你的審查就不是非常有價值的。這也是為什么好的審查都不可能非常快的完成:因為還要花時間去調(diào)查觸發(fā)某個給定函數(shù)的不同代碼路徑,要去確保第三方API能夠正確使用(包括任何邊緣情況),等等。
除了尋找你所審查的代碼中的瑕疵或其它問題之外,你還應該確保:
- 包含所有必要的測試。
- 合適的設(shè)計文檔已經(jīng)寫完。
甚至擅長寫測試和文檔的開發(fā)人員也并不總能記得在代碼改動之后及時更新。在適當?shù)臅r候來自代碼審查人員的細微調(diào)整對于確保代碼在隨著時間的推移不會變質(zhì)是至關(guān)重要的。
防止代碼審查工作超負荷
如果你的團隊強制要求做代碼審查,那這是有風險的,因為你的代碼審查工作可能一直積壓,最終到無法管理的地步。如果你兩周之內(nèi)不做任何審查工作,你可以很容易的花上幾天時間來趕補它。不過這也意味著當你最終決定去處理它們的時候,你自己的開發(fā)工作將遭到一定的意外擱淺。這也使得做好審查工作更加困難,因為正確的代碼審查需要強烈、持續(xù)的腦力勞動,很難這樣數(shù)日保持下去。
因此,開發(fā)者每天應該竭盡全力的清空他們的審查積壓工作。一個方法是早晨的第一件事情就用來解決審查工作。在開始自己的開發(fā)工作之前先做完所有的優(yōu)秀審查工作,你可以防止以后的審查局面失控情況。有些人更喜歡在午休之前或之后或在一天結(jié)束后做審查工作。無論你什么時候做這些事情,通過將代碼審查作為正規(guī)的日常工作而不是作為一種分散注意力的工作,你可以避免:
- 沒有時間處理你的審查積壓工作。
- 因為你的審查工作還沒做完而延遲項目的發(fā)布。
- 做出一些不再相關(guān)的審查,因為在此期間代碼已經(jīng)改動的非常多。
- 因為趕在最后一分鐘處理它們而導致審查工作最終完成的很差。
寫易于審查的代碼
無法管理的審查積壓工作也不能全怪審查人員。如果我的同事不管三七二十一的花費一周的時間來給一個大工程項目添加代碼,那么他們發(fā)布的patch將真的很難審查,因為在一個階段里有太多的工作要處理,代碼的目的和底層架構(gòu)體系也會很難理解。
這是將你的工作切割為一個個可管理單元之所以非常重要的眾多原因之一,我們使用scrum管理方法,所以對我們來說合適的單元是重點。通過一起努力,用單元來組織我們的工作,并提交僅與我們正在進行的某個單元相關(guān)的審查,我們可以寫出更加易于審查的代碼。你的團隊可能使用另一種管理方法,但是原則都是一樣的。
為了寫出易于審查的代碼,還有一些其它的必備條件。如果要做出一些很棘手的架構(gòu)決策,為滿足審查者的要求,事先進行討論是合理的。這將使得審查者更加容易的理解你的代碼,因為他們將知道你在代碼中試著達到什么目的以及怎么計劃來達到該目的。這也有助于避免這樣一種情況:在審查者提出一個不同的更好的方法后,你必須要重寫你的大段代碼。
在你的設(shè)計文檔里項目架構(gòu)應該要詳細的描述。這無論如何都是很重要的,因為它能讓一個新的項目成員很快的趕上進度并理解現(xiàn)有的代碼庫。它還能幫助審查者更好的做好自己的工作,這是另一個好處。單元測試也有助于向?qū)彶檎哒f明組件應該如何使用。
如果你的patch里包含了第三方代碼,請單獨提交。例如當jQuery的9000行代碼被插入代碼中間時,要做好代碼審查工作就難上加難了。
寫出易于審查的代碼的最重要步驟之一是給你的代碼審查部分添加注釋。這表示你可以自己瀏覽審查部分,并在任何你覺得有助于審查者理解代碼意思的地方添加注釋。我發(fā)現(xiàn)這樣的注釋僅花費相對較少的時間(經(jīng)常僅幾分鐘的時間)卻能產(chǎn)生巨大的作用,能讓代碼審查工作完成的更快、更好。當然,代碼注釋也有許多相同的優(yōu)點,應該在合適的地方使用,但是通常來說審查注釋更為明智。最后可以說是一個獎勵吧, 研究表明,當開發(fā)者重新閱讀和注釋代碼時,竟然發(fā)現(xiàn)他們自己的代碼里有很多的瑕疵。
龐大的代碼重構(gòu)
有時有必要重構(gòu)能影響許多組件的某個代碼庫。對于一個龐大的應用程序,這個過程可能花費好幾天(甚至更久)且導致龐大的補丁。在這些情況下一個標準的代碼審查工作可能是不切實際的。
最好的解決方法是遞增式重構(gòu)代碼。在工作代碼庫的合理范圍內(nèi)找到能達到你目的的某個改動點。一旦改好了,review通過了,接著進行下一個改動,直到整個重構(gòu)工作完成。這個方法可能并不是每次都行得通,但是有想法和計劃,在重構(gòu)時要避免巨大的補丁通常是實際可行的。像這樣來重構(gòu)代碼可能要花開發(fā)人員更多的時間,但它同時也產(chǎn)生了更好的代碼質(zhì)量和更容易的審查工作。
如果真的實現(xiàn)不了遞增式重構(gòu)代碼(這可能要說一些關(guān)于如何寫好和組織好源代碼的事情),一個可能的解決方案是當進行重構(gòu)工作時用結(jié)對編程來代替代碼審查。
解決爭議
你的團隊無疑是由一群聰明的專業(yè)人士組成。當大家對某個確定的編碼問題觀點不同時,基本上都會產(chǎn)生爭議。作為一名開發(fā)人員,保持開放的心態(tài),在你的審查者更傾向于一個不同的方法時要隨時準備妥協(xié)。不要對你的代碼持專有的態(tài)度,也不要帶個人審查意見。如果僅僅是因為有人覺得你應該將一些重復的代碼重構(gòu)為一個可重復利用的函數(shù)時,這并不能表明你就不是一個有吸引力的、出色的和有魅力的人。
作為一個審查者,一定要機智。在改變建議之前,認真考慮下是否你給的提議真的更好或僅僅只是你個人風格問題。如果你選擇的戰(zhàn)場集中在一些源代碼中明顯需要改進的區(qū)域,你將能獲得更多的成功。說一些諸如“考慮下……可能是值得的”或“有人建議……”的話更適合,而不是“連我的寵物倉鼠都能寫出一個比這更高效的排序算法”。
如果達不到一個中間立場(即雙方都不愿意妥協(xié)),那么就邀請一個雙方都尊敬的第三方開發(fā)人員過來看看,讓他們給出一些觀點和建議。