高效代碼審查的八條準則和十個經驗
代碼審查(Code Review)是軟件開發中常用的手段,和QA測試相比,它更容易發現和架構以及時序相關等較難發現的問題,還可以幫助團隊成員提高編程技能,統一編程風格等。
**1. 代碼審查要求團隊有良好的文化**
團隊需要認識到代碼審查是為了提高整個團隊的能力,而不是針對個體設置的檢查“關卡”。
“A的代碼有個bug被B發現,所以A能力不行,B能力更好”,這一類的陷阱很容易被擴散從而影響團隊內部的協作,因此需要避免。
另外,代碼審查本身可以提高開發者的能力,讓其從自身犯過的錯誤中學習,從他人的思路中學習。如果開發者對這個流程有抵觸或者反感,這個目的就達不到。
**2. 謹慎的使用審查中問題的發現率作為考評標準**
碼審查中如果發現問題,對于問題的發現者來說這是好事,應該予以鼓勵。但對于被發現者,我們不主張使用這個方式予以懲罰。軟件開發中bug在所難免,過度苛求本身有悖常理。更糟的是,如果造成參與者怕承擔責任,不愿意在審查中指出問題,代碼審查就沒有任何的價值和意義。
**3. 控制每次審查的代碼數量**
根據smartbear在思科所作的調查,每次審查200行-400行的代碼效果最好。每次試圖審查的代碼過多,發現問題的能力就會下降,具體的比例關系如下圖所示
(我想這是根據實現情況而定,如結合文檔來評審,那么代碼多一點也沒關系)
我們在實踐中發現,隨著開發平臺和開發語言的不同,最優的代碼審查量有所不同。但是限制每次審查的數量確實非常必要,因為這個過程是高強度的腦力密集型活動。時間一長,代碼在審查者眼里只是字母,無任何邏輯聯系,自然不會有太多的產出。
**4. 帶著問題去進行審查**
我們在每次代碼審查中,要求審查者利用自身的經驗先思考可能會碰到的問題,然后通過審查工作驗證這些問題是否已經解決。一個竅門是,從用戶可見的功能出發,假設一個比較復雜的使用場景,在代碼閱讀中驗證這個使用場景是否能夠正確工作。
使用這個技巧,可以讓審查者有代入感,真正的沉浸入代碼中,提高效率。大家都知道看武俠小說不容易瞌睡,而看專業書容易瞌睡,原因就是武俠小說更容易產生代入感。
有的研究建議每次樹立目標,控制單位時間內審核的代碼數量。這個方法在我們的實踐中顯得很機械和流程化,不如上面的方法效果好。
**5. 所有的問題和修改,必須由原作者進行確認**
如果在審查中發現問題,務必由原作者進行確認。
這樣做有兩個目的:
(1)確認問題確實存在,保證問題被解決
(2)讓原作者了解問題和不足,幫助其成長
有些時候為了追求效率,有經驗的審查者更傾向于直接修改代碼乃至重構所有代碼,但這樣不利于提高團隊效率,并且會增加因為重構引入新bug的幾率,通常情況下我們不予鼓勵。
**6.利用代碼審查激活個體“能動性"**
即使項目進度比較緊張,無法完全的進行代碼審查,至少也要進行部分代碼的審查,此時隨即抽取一些關鍵部分是個不錯的辦法。
背后的邏輯是,軟件開發是非常有創造性的工作,開發者都有強烈的自我驅動性和自我實現的要求。讓開發者知道他寫的任何代碼都可能被其他人閱讀和審察,可以促使開發者集中注意力,尤其是避免將質量糟糕,乃至有低級錯誤的代碼提交給同伴審查。開源軟件也很好的利用了這種心態來提高代碼質量。
**7.在非正式,輕松的環境下進行代碼審查**
如前所述,代碼審查是一個腦力密集型的工作。參與者需要在比較輕松的環境下進行該工作。因此,我們認為像某些實踐中建議的那樣,以會議的形式進行代碼審查效果并不好,不僅因為長時間的會議容易讓效率低下,更因為會議上可能出現的爭議和思考不利于進行如此復雜的工作。
**8.提交代碼前自我審查,添加對代碼的說明**
所有團隊成員在提交代碼給其他成員審查前,必須先進行一次審查。這次自我修正形式的審查除了檢查代碼的正確性以外,還可以完成如下的工作:
(1)對代碼添加注釋,說明本次修改背后的原因,方便其他人進行審查。
(2)修正編碼風格,尤其是一些關鍵數據結構和方法的命名,提高代碼的可讀性。
(3)從全局審視設計,是否完整的考慮了所有情景。在實現之前做的設計如果存在考慮不周的情況,這個階段可以很好的進行補救。
我們在實踐中發現,即使只有原作者進行代碼審查,仍然可以很好的提高代碼質量。
**9.實現中記錄筆記可以很好的提高問題發現率**
成員在編碼的時候應做隨手記錄,包括在代碼中用注釋的方式表示,或者記錄簡單的個人文檔,這樣做有幾個好處:
(1)避免遺漏。在編碼時將考慮到的任何問題都記錄下來,在審查階段再次檢查這些問題都確認解決。
(2)根據研究,每個人都習慣犯一些重復性的錯誤。這類問題在編碼是記錄下來,可以在審查的時候用作檢查的依據。
(3)在反復記錄筆記并在審查中發現類似的問題后,該類問題出現率會顯著下降
**10. 使用好的工具進行輕量級的代碼審查**
“工欲善其事,必先利其器”。我們使用的是bitbucket提供的代碼托管服務。
每個團隊成員獨立開發功能,然后利用Pull Request的形式將代碼提交給審查者。復審者可以很方便在網頁上閱讀代碼,添加評論等,然后原作者會自動收到郵件提醒,對審閱的意見進行討論。
即使團隊成員分布在天南海北,利用bitbucket提供的工具也能很好的進行代碼審查。
二、代碼審查:大家都應該做的事情
Google的代碼之所以優秀原因其實很簡單:他們非常重視代碼審查。代碼審查并不是Google獨有的,它被公認為是一個很好的(提高代碼質量的)手段,很多人已經在日常開發中采用代碼審查。但我還沒有看到哪一家大公司(像Google這樣)應用得如此廣泛。在 Google,任何的產品或者項目代碼在檢入(代碼倉庫)之前都需要進行有效的審查。
每個人都要參與代碼審查,而且這里我指的不是非正式的審查:它是軟件開發環節中非常重要而且通用的規則。不僅是產品代碼,所有的代碼都需要進行審查。審查代碼不需要投入很多的精力,但是(與不做審查相比)產生的效果卻是天壤之別。
關于代碼審查(code review),Jonathan Danylko 的看法是“代碼要經常檢查(包括自查和其他同事檢查)。不要把別人的檢查,看成是對代碼風格的苛求。應該把它們看作是有建設性的批評。對個人來說,經常檢查你的代碼并且自問,“我怎樣才能寫得更好呢?” 這會加速你的成長,讓你成為一個更優秀的程序員。”
**你能從代碼審查中收獲什么?**
事實顯而易見,有另外一個人檢查即將提交的代碼,能夠幫助找到bug。這是代碼審查眾所周知且經常被提及的好處。但依據我的經驗,這是最沒有價值的一個好處。人們確實可以在代碼審查中找到bug。然而坦率地說,在代碼審查中找到的bug絕大多數都是一些代碼作者花上幾分鐘就能找到的小bug。那些真正需要花時間才能找到的bug在代碼審查中是檢查不到的。
代碼審查最大的好處在于它是一種社交的途徑。如果你編程的時候就知道會有同事檢查你的代碼,那么你的程序會有所不同。你寫的代碼會更加整潔,有著較好的注釋,結構也組織的不錯——因為你知道會有人來檢查你的代碼,而且你很在意他們的意見。如果沒有代碼審查,你知道代碼會在最后才會審查。因為不是馬上就要檢查,所以對你而言并不緊迫,因而你不會想著先自檢一遍。
代碼審查還有一個更大的好處,就是可以分享知識。在很多的開發團隊中,每個人都會負責并且專注于一個核心模塊。除非別的同事負責的模塊出現問題導致自己的代碼不能運行,否則他們是不會去關注別人的工作。這樣產生的結果是,每一個模塊的代碼只有一個人比較熟悉。假如事不湊巧,那位程序員正好休假或者離開了公司,那么沒有人了解那些代碼了。如果有代碼審查的環節,那么至少會有兩個人熟悉代碼——代碼的作者和審閱者。審閱者雖然沒有作者對代碼那么了解 ——但是他同樣熟悉代碼的設計和結構,這些信息是無價之寶。
當然,沒有什么事情是那么簡單的。以我的的經驗看來,要做好代碼審查需要一段時間練習。我注意到經驗不足的審閱者通常會落入一些代碼審查的陷阱,這些陷阱往往會造成很多的麻煩,給那些希望嘗試代碼審查的人們留下了壞印象,成為了他們采納代碼審查的一個主要障礙。
代碼審查最重要規則是對即將提交的代碼中查找問題——你需要做的就是確認代碼是正確的。而通常會犯的一個錯誤,也是剛剛接觸代碼審查的新手容易犯的一個錯誤,即審閱者會判斷這段代碼是否按照自己思路來實現。
當有一個問題需要解決時,通常會有幾十種的辦法。當選定一個解決方法時,會有百萬種代碼實現。因此,作為一個審閱者,你的工作不是確保代碼是按照你的方式來編寫的——因為這是不可能的事情。審閱者的工作是確保原作者編寫的代碼是正確的。如果你沒有遵守這個規則,你可能會到處碰壁,審查結束時你的心情很糟糕,對你來說肯定不是一件好事情。
**問題在于這是不自覺就會犯的一個錯誤。**假定你是一個程序員,當你在看一個問題的時候,你會得到一個自己的解決方案——并且你認為你看到的就是這個問題(應該采用的)解決辦法。如果想要成為一名好的審查者,你需要知道這是不對的。
**第二個誤區就是人們感覺一定要說點什么(才算是做了代碼審查)。**代碼的作者花了很多的時間和精力來編寫代碼——你難道不應該說點什么嗎?
答案是:你不應該。
如果只是說“哦,這看起來這不錯!”,這永遠沒錯。反之,如果你不斷地去查找一些“問題”并加以指責,那么我肯定你的信譽會蕩然無存。如果你不斷地去制造一些事情來說些什么,那么代碼的作者會認為,當你的言論只是為了避免冷場。從此,你的意見不會受到重視。
**第三個誤區就是速度。**你不應該匆忙完成一次代碼審查——但是也不要拖延。你的同事在那里等著你的審查結果。如果你和同事不愿意抽出時間來做代碼審查或者一直拖延,大家會對這次的審查感到厭煩,也會認為以后的代碼審查也只會帶來麻煩。看起來好像代碼審查會打斷你的工作,其實不必如此。你不必要在別人要求你審查的時候馬上丟掉手頭上的事情。但是在幾個小時之內,當你工作中間休息的時候——喝杯茶,去一下洗手間或者聊聊天,散散步。當你再回來工作的時候,你可以開始并完成這個代碼審查。如果你這么做了,沒有人會站在你身邊一直等著你給出審查結果。
21世紀的代碼審查
在軟件工程領域里代碼審查可以結束程序員之間無謂的爭執。開發者常常會因為一些愚蠢的小事斗嘴,冒犯對方,甚至是在Q&A問答之前抓住 Bug而喋喋不休,爭執總是圍繞在你左右。OK,千萬不要誤會我的意思,因為我們有理由相信代碼審查絕對是個不錯的好方法。原因如下:
1. 越早發現bug也就意味著可降低項目成本。無須釋放一個修復補丁,因為它正處在開發階段。
2. 代碼變得越來越重要。
3. 知識貫穿于你的團隊中,不再像以前那樣一大塊代碼只有某一個人知道。
4. 開發者需要加倍的努力。如果開發者知道別人要對他的工作進行評估時,就會采取額外的努力做好工作,同時他還喜歡用文檔注釋標出異議。
如今,在21世紀的今天很多項目都沒有使用代碼審查。**本文將提供8條準則**,供開發者學習與參考。
1. 永遠別忘了TDD
再確認測試代碼前,先找別人幫你檢查下是否無誤。在別人做之前盡量檢查出bug并且將其處理好。代碼審查最重要規則是對即將提交的代碼中查找問題——你需要做的就是確認代碼是正確的。
2.盡可能的自動化
這里有幾個非常好的Java工具比如:PMD, Checkstyle, Findbugs等等。問題是當利用這些工具查找后人們還肯花時間去做代碼審查嗎?
使用這些工具前,為這些工具制定一套細則是非常重要的。這能夠確保你使用同一個代碼審核標準從而區別于那些常被用于20世紀老式的代碼審查規范。在理想的狀態下,這些工具可運行在各種版本控制系統上通過hook審查每個代碼。如果該代碼不好將被阻止在外。
3.尊重設計
在我開始從事Java項目早期時,用代碼審查的方式已為時已晚。因為當你檢查代碼問題時實際上給你的設計造成了缺陷。設計模式被誤解,一些繁雜的附屬物質混入進來或者開發者脫離了主題。
審查會混亂你的觀點。或許你會反駁:“這是代碼審查而不是設計審查”。這時一些爛攤子必然會接踵而至。為了避免這些問題發生,我們改變了設計的初衷。代碼審查會牽連到很多面,無論是設計還是設計審查。事實上,我們通過設計審查要比代碼審而得多的沖擊要多的多。設計需要更高的質量和靈感,我們應該避免一些復雜的思維。
4. 統一的風格指南
即使是使用自動化工具(諸如Checkstyle,Findbugs 等)也應避免不必要的風格沖突,你的項目應該具備有風格指南。(在盡可能的情況下)堅持Java協議的規范標準。嘗試著為你的項目介紹制定一個“詞典”,這就意味著,當涉及這個代碼時,查看該代碼的用法和環境是否適宜,這些都很容易被檢測出。
5. 挑選適宜的工具
如果開發者都在使用Eclipse開發工具( Eclipse IDE插件Jupiter),你可以通過你的方式來查看代碼、調試代碼甚至可使用Eclipse IDE上的一切東西當來幫助你在審查代碼時更加的便捷。但是,如果大家沒有使用同一個IDE(或者該IDE沒有給你的工作帶來方便)你可以考慮 Review Board. ,它是個不錯的選擇。
6.請記住每個項目都不同
也許你在采用以前的項目方法工作,但是,請記住每個項目之間是不同的。每一個項目都有特定的架構(高并發或是高分散),有特定的文化(或許很多人喜歡使用Eclipse),并使用特定的工具(maven or ant)。難道你想照葫蘆畫瓢?OK,請記住,不同的項目有不同的工作方法。
7.懂得取舍
代碼審查需要積極和細致而不是賣弄學問。你會因為一些細微的瑣事讓你緊張而導致項目失敗或是花費公司成本嗎?記住,千萬不要這樣。理清頭緒,換個角度想想,改變自己的心態而不是記掛著去改變別人。
8. Be buddies
在我看來,稱之為“buddy reviews”(別人會叫“over the shoulder”)非常好。A buddy review是指與其他團隊成員每隔一到兩天以非正式的形式討論,并且快速的瀏覽(5-10分鐘)對方的代碼。這種方法可以幫助你:
1. 及早的發現問題
2. 總是很快的知道該干什么
3. 代碼審查無須過長,因為你只需要查看新的代碼,舊的代碼會很快趕上
4. 這種非正式的場合——沒有緊張感,很有趣!
5. 可以定期的交換想法
buddy reviewing在團隊中是一種很好的工作方式,當某人在團隊中出現問題時可以及早的發現。這不僅可以幫助大家,還可以交換彼此的進度和想法。
總之,如果你的項目正在進行代碼審查,應該做到快速、有效、不浪費別人的時間。正如文章所說的,這幾點非常重要。代碼審查用意是在代碼提交前找到其中的問題。
代碼審查
代碼審查可以幫助提高代碼質量,避免由于代碼習慣而造成的 bug。下面列出的這些要點因該可以作為大部分代碼審查的指導,如果是 Java 應用的話,這些建議應該被視作最佳實踐。
文檔
-
Javadoc 應該在每一個類和方法中添加。
-
如果是修復某個 bug,應該添加 bug ID。
-
走捷徑的方法或者復雜的邏輯要有解釋。
-
如果代碼會被公開,每個文件頭都要標注版權信息。
-
復雜的 HTML,JavaScript,CSS 應該包含文檔。
功能
-
如果類似的邏輯被使用了多次,應該把它寫成一個幫助類,然后在多出調用。
-
鼓勵使用 API 而不是重復編寫代碼解決相同的問題。
-
要強調代碼的單元測試。
-
任何新加的代碼不應該破壞已有的代碼。
-
假如是 Web 應用,JSP 不應該包含 Java 代碼。
安全
-
任何代碼都不能執行用戶的輸入,除非轉義過了。這個常常包含 JavaScript 的 eval 函數和 SQL 語句。
-
禁止那些在短時間內提交非常多請求的 IP。
-
任何類,變量,還有方法都應該有正確的訪問域。
-
盡量避免使用 iframe。
性能
-
所有數據庫和文件操句柄在不需要的時候都應該被關閉。
-
SQL 語句的寫法會導致性能千差萬別。
-
鼓勵創建不可變(immutable)的類。
-
類似的邏輯代碼,盡量通過 if else 語句來實現更多的重用。
-
盡量避免使用重對象(heavy objects)。
-
如果是 Web 項目,請檢查是否使用了合適的圖片尺寸,CSS sprites 和瀏覽器緩存等技術。
-
全局都需要的信息保存在 application context 中。
編碼習慣
-
沒有被使用的變量要刪除。
-
針對不同的 Exception 要用不同的 catch 語句,而不是一個 Exception 解決所有問題。
-
針對變量,方法和類要用相同的命名方法。
-
常量應該被寫在獨立的常量類中。
-
每行代碼的尾部不要有多余的空格。
-
對于括號,循環,if語句等等要用統一的格式。
-
每一個單獨的方法不應該超過100行。
-
一個單獨的語句不應該超過編輯器的可視區域,它可以被拆分成幾行。
-
檢查 String 對象既不是null也不是空的最好方法是 if(“”.equals(str))
-
假如類有很多成員變量,并且實例化的時候只需要少數變量傳入的話,最好使用靜態工廠方法,而不是重載構造函數。
-
給方法添加適當的訪問控制,而不是所有都是 public。
-
遵守項目中使用的框架的最佳實踐建議,例如 Spring,Struts,Hibernate,jQuery。
以上的某些注意點可以通過靜態代碼檢查工具完成,例如 CheckStyle,FindBugs 和 JTest。