谷歌最佳實踐 - 程式碼稽核方法
來源
程式碼稽核時我們應該稽核什麼
注意:在考慮下面的原則時,切記要根據《程式碼稽核標準》進行考慮。
設計
程式碼稽核中最重要的事情就是考慮一下變更提交的整體設計。變更提交中各個部分的關聯互動是否合理?這些變更是應該在程式碼基線中,還是應該提交到支援庫中?這些變更是否能夠與系統內的其他部分很好的整合?現在是不是加入這個功能的合適時機點?
功能性
變更提交是否實現了開發者的真正目標?開發者期望的對於程式碼的使用者是否有好的效果?這裡的使用者包含終端使用者(會受到變更提交的影響)和開發者(會在將來使用到這些程式碼)。
大部分情況下,我們都希望開發者在提交程式碼稽核前能夠事先做好充分的測試。但是作為稽核者你還是要考慮邊界情況,併發問題,嘗試像使用者一樣思考,確保你稽核的這些程式碼中不會產生任何bug。
當有必要
另外一方面在稽核程式碼,要特別重視類似併發等特別重要的概念,防止存在引發死鎖或者競爭條件的寫法。這類的問題即使執行程式碼也是比較難發現的,常常需要開發者和稽核者仔細思考確保問題不會發生。(當可能存在競爭條件或者死鎖時,儘量不要採用併發模型。這會導致程式碼稽核或者理解都變得更加複雜。)
複雜度
變更提交的內容是否必須這麼複雜?每一級都要這樣檢查:單行程式碼是否過於複雜?方法函式是否過於複雜?類是否過於複雜?“過於複雜”通常意味著”不能被讀程式碼的人快速理解“。同樣也意味著“當開發者嘗試修改程式碼時更容易引入bug”。
一種常見的複雜化就是過度設計,將程式碼過度通用化,或者加入目前系統不需要的特性。稽核者需要特別警惕這種過度設計。鼓勵開發者針對當下
測試
針對變更內容,應該提供單元測試、整合測試或者端到端測試。大體上,應該在變更提交中包含測試程式碼,除非變更提交是因為處理緊急問題。
確保程式碼中的測試是正確、合理、有效的。測試程式碼不是為了測試本身,我們基本不會針對測試寫測試程式碼,我們自己來保證測試程式碼是有效的。
當代碼異常時測試是否真的會失敗?如果基於測試進行程式碼變更,是否能夠產生負的正向反饋?是不是每個測試方法內的斷言都是簡單有用的?測試程式碼在不同的測試方法中的分佈是否合理?
切記測試程式碼也是程式碼,也同樣需要維護。不要因為這些測試程式碼不在主程式碼中就允許它非常複雜。
命名
開發者有沒有設計良好的命名?良好的命名能夠充分體現自己是什麼或者做了什麼,反之則會造成程式碼不具備可讀性。
註釋
開發者有沒有用良好表述的語言寫了清楚的註釋?是否所有的註釋都是必須的?通常有用的註釋是解釋某些程式碼存在的原因,而不是某些程式碼在做什麼。因為如果程式碼不能清楚的自描述,你就應該把程式碼處理的更簡單一些。可能存在一些例外(例如:正則表示式,複雜演算法常常需要註釋來描述具體的工作內容),但是大部分的註釋應該是說明程式碼不能包含的內容,比如做這個決定的原因。
在稽核變更前先看一下注釋也可能會有幫助。也許有一個待辦項應該移除掉,一個註釋的建議與實際的變更衝突等等。
注意:註釋和類、模組、方法等的文件不同,文件是為了表達程式碼的目的,如何使用,使用時會產生何種效果。
樣式
在谷歌,我們提供了所有主要語言的《樣式指南》,甚至包括大部分的低頻語言。要確認變更提交是遵守了樣式指南的。
如果你希望能夠改進某些在樣式指南中沒有提到的樣式點,在你的評論前加上”Nit:“的字首,這樣開發者就知道這是一個改進點而不是強制的。不應當因為個人的樣式喜好而拒絕變更提交。
開發者不應當在主樣式調整中混合其他提交。這樣會使得變更提交中具體變化了什麼內容,使得合併與回滾也更加複雜,還會引發其他問題。例如一個開發者希望重新格式化程式碼,那就應當將重新格式化的部分提交一次,其他的變更在這次之後另外提交一次。
文件
如果一次變更影響到使用者的構建、測試、相互引用、釋出程式碼等,需要確認有沒有更新關聯的文件,包括 READMEs(專案自帶文件),知識庫頁面,以及其他關聯生成的文件。如果變更刪除或者過期了程式碼,確認相關的文件是否被刪除。如果文件沒有更新或提交,要求要補齊。
每行程式碼
仔細檢視你分配到稽核的每行程式碼。類似資料檔案,生成程式碼,或者很大的資料結構之類的有時可以大概看一下,但是手寫的類、方法或者程式碼段絕對不要大概看一下,假設不會有太多問題。有些程式碼就是比其他程式碼需要更加仔細的稽核,這就是你作為一個稽核者需要決定的,但是至少確保你理解了程式碼的邏輯。
如果發現看程式碼很困難並且導致稽核的速度變慢,你應該讓開發者明白問題所在並且等他們澄清整理之後再嘗試稽核。在谷歌我們只僱傭像你這樣優秀的軟體工程師。如果你都無法理解程式碼,那麼其他的開發者也同樣很難理解。當你要求開發者理清程式碼時,也是在幫助今後其他開發者能夠理解程式碼。
如果你能理解程式碼,但是你覺得自己還不夠資格來做某部分程式碼的稽核,確保有另外一位有資格的稽核者能夠負責,例如安全、併發、可訪問性、國際化等複雜問題。
整體
將變更提交放到一個完整的上下文中來檢視一般會更有幫助。程式碼稽核工具一般都是展示變更附近的程式碼。有時候你是需要檢視整個檔案來確認這些變更是合理的。例如,你只看到添加了四行程式碼,但是當你檢視整個檔案會發現這四行是在一個五十行的方法中,而這個方法應該要拆分到更小的方法中。
同樣將變更提交放到系統的上下文中作為一個整理來思考也是很有幫助的。這份變更是否會提高程式碼的健康程度還是是的系統更加複雜、更少的可測試之類的問題?決不接受降低系統健康程度的變更提交。大部分系統都是因為不斷新增的小變更使得整體系統的複雜度不斷提高,所以阻止新增提交中的複雜度提升是很重要的。
閃光點
如果在稽核中發現了程式碼中的閃光點,請務必告訴開發者,特別是他們用很棒的方式實現了你評審中的要求。稽核者往往關注於問題,但是他們也應該欣賞和鼓勵一些優秀的實踐,這些行為在有些情況下更有價值,換言之就是要告訴開發者哪些做對了比告訴他們哪些做錯了更重要一些。
總結
當稽核程式碼時,你應該確保:
- 程式碼應該是經過良好設計的。
- 功能都應該是對於使用者有幫助的。(引用程式碼的開發者和終端使用者)
- 任何UI修改都是合理並且展示良好。
- 所有的併發操作都要是安全的。
- 程式碼不應該過於複雜。
- 開發者專注於眼前的問題,而不要考慮實現將來可能要解決的問題。(不要過度設計)
- 程式碼中包含合理的單元測試。
- 測試的設計都是合理的。
- 所有的命名都很清晰合理。
- 註釋清晰有效,大部分都是解釋為什麼,而不是做了什麼。
- 程式碼有合理的文件(谷歌內部文件平臺是g3doc1)。
- 程式碼符合樣式指南。
確保你稽核了每一行程式碼,聯合了整體上下文,確保你通過的程式碼時提高了程式碼的健壯性,並且在開發者的閃光點上給了足夠的鼓勵和關注。
下一篇:變更提交稽核時的建議路徑
參考Quora的這篇問題What is g3doc,谷歌內部的工程文件平臺,並且和版本控制系統結合。↩