|
這篇文章只是體現我以前寫代碼和做代碼審查時候的一些原則。供大家借鑒。歡迎大家補充。
正確性 (Correctness)
正確性是第一要求。不能解決問題的代碼是耍流氓。
- 結構 (Code Structure)
結構體現邏輯。第一步,第二步;需要什么數據,需要做什么處理,處理完了結果到那里去,都應該在結構中被很好的體現出來。
結構體現設計。 設計一定要清晰。我的經驗來看,一般來說,在design chart上面的每個component都對應著自己的class,然后之間或class內部的通信通過member function來完成。
一個可借鑒的做法 – 在一個大的feature implementation過程中,給出第一個diff的時候,可以只把結構當做一個diff,里面的函數可以是空的(place holder)。
把數據的生成和界面的展示分開來。典型的可以按照MVC的模式來,但也可以只把數據和UI分開來的比較輕量級的做法。結構應當是diff review時候最最關注的地方。最需要問的問題就是“這個diff號稱要解決的問題被正確解決了嗎?”
- test的重要性
不論你再正確,還是有錯誤的時候。通過(相對)公證的test來 1)減少自己被繞到代碼里的幾率;2)讓后續的或者別人的改動對自己代碼不經意的破壞被最快的展現出來。
test應該把class主要的function都測一遍
test也應該把class和其他class最重要的integration也測一遍。
可讀性 (Readability)
Readability leads to maintanence cost.
- diff的大小
bug修改,無所謂,該多大多大。一般bug fix不會超過100行。超過的要特別重視,想想究竟是什么原因造成。會不會是當初設計的問題。
一個diff,原則上不應該超過200-300行修改。但多了怎么辦,把一個diff變成多個 – split to multiple changes.
- 每個diff應該只做一件事情
每個diff盡可少的做一個改動。這樣可以盡可能的方便自己的管理(學會用git branch),和方便reviewer的代碼審查。如果diff越集中做一件事,審查代碼的人需要越短的時間來審查做出高質量的,整體效率越高。
- 一個function超過1屏 => split it, idiot.
- 統一的代碼規范
比如文件名,變量或函數名的命名規范,分行的前置空2個spaces或4個;每行的字數(不應超過80char);如何使用public/private/protected;用左右括號的原則;空行的使用;文件和代碼comments的位置 (比如,代碼comment只能單獨成行);對“// TODO:”的使用規范;macro,constant的使用;
等等等等。
這里沒有特別的哪一種style一定更對,但是需要有一個大家統一的guideline,一起遵守,讓整體的代碼有統一的風格和標準。
最大的好處就是有利于readability.
- object-oriented v.s function-oriented
Java本身就是面向對象,所以這個問題不大。但千萬不要出現披著面向對象的外皮,在class里面寫超長的面向函數的處理。這種情況下,盡可能的分流成helper function.
- crispy & sufficient的注釋
注釋應當簡潔但充分。有些人覺得代碼應該speak for itself。我不大同意,代碼是實現細節,適當的在意圖上給予說明,可以大幅度的減少讀代碼的人的煩惱。
diff發出去之前
- 與master做一次merge update,確保resolve all conflicts
- run一次所有涉及的test cases (需要工具)
- 考慮最可能做reviewer的人,可以是團隊伙伴,也可以是修改涉及到的源代碼的owner。但必須是關心該改動或和改動相關的人。
- 所有的manager應當自動subscribe自己的團隊里所有人的diff requests (做好filtering,但你不一定要看)
code-review之中應該做的
- 作為reviewer,一定要讀懂diff;所有被accept的diff必須是在讀懂的前提下。做reviewer的人要有“將來如果這些代碼線上出問題,我要幫助support”的心理準備。
- code review 應該被每個engineer當做工作的重要一部分。做Performance Review的時候應該把幫助做過的code review考慮,for both employee & manager.
- 應當在24小時內給回復,這應當變成共識。
- 感覺有問題的代碼,一定要在相應的行上做出評論 (inline comments),以讓作者明白問題所在。
- 盡可能把對修改的所有意見一次性給出,減少來來回回的次數。比較復雜的建議reviewer主動找author來進行線下溝通,達成一致。
- 一般的diff,來回次數不宜超過3次;如果超過3次,想想看,是不是diff 太大,太復雜了。
check-in之前應該做的
- 與master做一次merge update,確保沒有問題
- run一次code change涉及到的所有test cases(包括新增的和涉及到的test cases)
鄭重聲明:本文版權歸原作者所有,轉載文章僅為傳播更多信息之目的,如作者信息標記有誤,請第一時間聯系我們修改或刪除,多謝。