1. 程式人生 > >關於程式碼審查,那些你不曾關注的細節

關於程式碼審查,那些你不曾關注的細節

在工作中,我們都要進行程式碼審查。每個人都知道程式碼審查,每個人都會做程式碼審查(至少我希望你會做)。但如果花點時間討論一下,你就會發現在“良好的程式碼審查應該做些什麼”這個問題上,可謂仁者見仁智者見智。每個參與者應盡的義務是什麼?程式碼審查的最佳方式又是什麼?

 

什麼是程式碼審查?

 

首先讓我們來看一看什麼是程式碼審查?維基百科上的程式碼審查定義如下:

程式碼審查(有時稱為同行審查)是一項確保軟體質量的活動,是由一個或多個人通過檢視和閱讀部分原始碼來檢查程式的一種方式,程式碼審查通常發生在實現完成後或實現中間階段。參與審查的人員中必須至少有一人不是程式碼的作者。執行審查的人員稱之為“審查人”,但不包含作者本人。——維基百科

這段引文後面還寫了程式碼審查的目標,其中,除了找出被審查程式碼中的質量這一主要目標外,通過執行這些審查還可以實現:

提高程式碼質量保持專案的一致性發現 bug學習(通過程式碼被審查)和教學(通過審查其他人的程式碼)營造共同的責任感通過他方查詢程式碼中的小錯誤,防止這些小錯誤日積月累腐蝕程式碼尋找更好的解決方案的常見方法從上述定義中我們能看出什麼?程式碼審查是一種好方法,它可以保持軟體的可維護性,並在軟體投入生產之前發現 bug。除此之外,該方法還有助於教導團隊的新成員,即使沒有正式的培訓或訓練,我們也可以通過程式碼審查將一些技巧傳授給他人。但如果僅參照該定義展開程式碼審查的工作,我們仍然不清楚應該做些什麼?下文將闡述每個參與者應盡的義務,並從中總結出程式碼審查的最佳方式。

 

被審查者視角:不給審查者添麻煩

 

首先申明一點:程式碼審查不是為了評判或審查某個人的程式設計能力。“程式碼審查只關注程式碼”,無論這段程式碼是團隊中的高階開發人員寫的,還是新人實習生寫的——被審查者的職責在審查之前就開始了。在我看來,被審查的人應該儘量減輕審查者的工作,試著換位思考。我認為加大審查者工作難度的主要因素如下:

將重構的程式碼和新程式碼的實現混合在一起在提交功能變更時,重新格式化整個程式碼庫提交程式碼時不寫提交註釋(我會在後面討論該話題)在一次程式碼稽核(或一次程式碼提交)中提交多個功能需要考慮的因素在寫完上面這段話後,我想起在程式碼審查中最煩人的因素是“干擾”和“大小”。所謂干擾,我指的是與提交註釋無關的一些變更,它們往往會造成思想負擔。因為在審查過程中,你無法確定眼前的這些程式碼只是外觀上的變化還是非常重要的功能。其次是大小——一次只應該提交一個變更。如果你遇到地問題是提交過大,那麼對應地解決方法就是拆分之後再分別審查,然後再將這些分離的提交合併到臨時的功能分支上,最後再合併到主分支上。由此,我總結出了以下幾條程式碼審查的最佳方式:

1. 單獨提交程式碼清理命令(重新格式化或修復拼寫錯誤等)和重構

我甚至不建議將重新格式化與重構混合到一起。如果你想重構程式碼,那麼請注意用正確的格式。如果程式碼中只有與重構相關的變更,那麼程式碼審查會容易得多。當代碼中出現大量基本上只是清理命令的變更時,我們有時很容易忽略小的變化(還記得我說過的干擾嗎)。2. 編寫相關的提交說明務必確保你的提交註釋可以很好地向審查者說明提交的內容,還有尤其要說明程式碼變更的原因。如果你的設計受到了很大限制,也一定要寫好說明。如何書寫好提交註釋?下面是我在工作中提交程式碼時寫的提交註釋,這也是我們的團隊慣用的格式:

XXX:新增一些新功能(簡短的說明)對於程式碼實現更為詳細的描述你可以寫很多行,但還可以包括:* 專案符號* 不同的細節* 如果你有很多點,一行寫不下,那麼也可以在各個點之間插入空行引用與之相關的問題

其中 XXX 是問題的連結(例如 JIRA-007:標題),如果程式碼變更沒有相應的追蹤問題,那麼可以只是用關鍵詞替代,如 FIX、BUG 或 MAINT(維護)。上述有關提交說明的建議不僅針對程式碼稽核,更是普遍適用的最佳方式。提交說明中如果漏掉了什麼重要的資訊總是令人惱火,相反,清晰明瞭的提交註釋也會令人心情愉悅。通常在稽核程式碼遇到問題時,就可以試著看能否從提交註釋中找到答案。

3. 只提交準備好審查的程式碼這也是對他人的尊重。如果我希望得到另一個人的建議,就不會給他製造不必要的麻煩。因此,請確保你的程式碼通過了所有測試。另外,在讓別人審查你的程式碼前,先進行自我審查,仔細看看你提交的程式碼差異。4. 審查期間不要更改程式碼這種做法會給審查者帶來更多壓力,以致中斷審查進度。如果你想修改審查過程中發現的問題,那麼請確保在接受審查的程式碼基礎上再另建一份提交。如此一來,審查者就可以在現階段審查完成後,再來看你新修改的程式碼。最終,在所有審批都確認後,你可以將所有的提交壓縮成一個。回顧被審查者的工作,我們可以得出一個結論,即不要給審查者製造不必要的麻煩當屬程式碼審查過程中被審查者的最佳行為準則:

確保你的程式碼通過了自己的審查,並且你沒有發現任何明顯的問題,可以放心地合併程式碼(如果你發現了問題,並想討論某些內容,那麼提前跟你的稽核者打招呼);程式碼中沒有混合不相干的變更,不會太長也不會增加閱讀難度;針對程式碼變更寫好提交註釋,明確交待變更的目的

 

「對碼不對人」的審查者

 

首先再次申明:程式碼審查不是為了評判或審查某個人的程式設計能力。我覺得這點對審查程式碼變更的人而言更重要,我建議你在評論時更加謹慎,通常可能只是因為你沒有看到寫程式碼的人在實現過程中遇到的問題。那麼,在程式碼審查時究竟應該做些什麼?通常我會檢查幾個重點項,下面我將據此總結出程式碼審查的最佳方式。目的我的第一個檢查點是程式碼是否與提交註釋相符,如果兩者存在差異,那麼就很難驗證程式碼的正確性。實現在驗證了實現的目的後,我會驗證實現本身,在讀完提交說明後,我問自己的第一個問題就是:“我應該如何實現?”接下來,我會將審查的程式碼與我想象的解決方案做比較。如果我的想法與審查的程式碼之間存在很大差異,那麼通常我會再回頭檢視提交註釋,或前一次變更,並快速整理我設想的版本。通常在幾分鐘後,我就會知道自己的解決方案是否可行,此時我需要考慮的因素包括:

效能(如果是在同一個程式碼庫的話,通常不那麼重要)可讀性(恕我直言,這幾乎是最重要的)嘗試覆蓋比被審查的解決方案更多的極端情況是否可以用更好的程式碼樣式(比如更多可重用的程式碼)來實現相同的功能然而,實際情況可能往往有所不同——你想到了一個解決方案,同時驚訝於你所稽核的程式碼作者的睿智。如果真是如此,那簡直太棒了,因為你不僅學到了新東西,而且你需要做的就只是看看眼前的程式碼在格式和程式設計風格上是否與開發指南相符。可維護性對我來說,可維護性是最重要的因素之一(這可能有偏見,因為目前我正在開發一個長期專案),但若能不影響將來的實現速度也依舊非常了不起。出於可維護性的原因,我會直接檢視程式碼測試。如果沒有,那麼對我而言無疑是個壞訊息,因為這通常意味著我“需要動手寫一些測試用例”。如果程式碼中有測試用例,那麼我會驗證它們是否在測試正確的東西,還有變更後的 API 的使用方法以及測試用例是否合理。通過單元測試我們就能看出實現的使用方法。如果某個實現難以閱讀或難以使用,那麼大多數時候只能說明該實現不完美。接下來我會注意 API 級別的重大變化。如果 REST 控制器被修改了,我就會很擔心,因為我不想我們的變更牽扯到其他服務的部署。理想狀態下,這不是問題,因為我們兩邊會同時部署,但大多數情況下,我會避免這種做法,同時採用一個不會破壞客戶端的版本。負責服務間通訊的 DTO(資料傳輸物件,Data Transfer Objects)的變更亦是如此。如果DTO中引入了一個新值,我們就需要格外謹慎。下一步我會縱觀實現整體。如果其很難理解,我會想辦法(就像前面想象代替實現方案一樣)讓它更加容易閱讀和理解;如果我想到更好的方法,也會徵求程式碼作者的意見,大多數時候,我們會迅速達成共識或商量出一個折中方案。最後我需要檢查的是外部文件。如果有外部文件(例如外部 wiki 或 README.md),我會快速檢查文件是否反映了程式碼中的變更。安全一般來講,程式碼都需要擁有某種安全機制(例如 OAUTH 層,或 REST 控制器的 BASIC AUTH)。在新增新程式碼時,我會驗證這些程式碼是否得到了正確的保護。寫評語的方法在寫審查評語時,一定要很友好,還要讓你的評語儘量簡潔準確。請確保評語針對的是程式碼,而不是寫程式碼的人。儘可能避免使用所有格(比如你的、我的等等),這些詞語會讓人誤解,儘管你想說的是程式碼本身。你需要寫明你的評語是程式碼變更要求,還是可能需要討論的意見。如果你在一段非常好的程式碼中寫了很多挑剔的評語,那麼一定要給寫程式碼的人一些讚美的話。最後,總結程式碼審查中,審查者的最佳操作方式:

保持友好審查程式碼,而不是寫程式碼的人評語要簡潔準確在最後不要忘了為好的程式碼點贊表明你是否需要等程式碼修改完畢後再看一次(如果你們的審查工具不支援這一功能的話)

希望這篇文章可以幫助你瞭解“程式碼審查的最佳方式”。每天在工作中,我都會遵照這些準則,這對我和我的團隊都有很大的幫助。如果你對該主題有其他看法,也請告訴我。