原文
What to look for in a code review (程式審核過程中要看些什麼?)
Design (設計)
程式審核裡最重要的一環是 CL 的整體設計
CL 內多個程式片段的改動之間的交互作用是否合理? 改動是屬於程式庫 (codebase) 還是函式庫 (library)?
此次改動是否能和系統其他部分很好地整合? 現在是加入該功能的適當時機嗎?
Functionality (功能性)
CL 是否真的有符合開發者意圖? 開發者的意圖對於使用者
們是好的嗎? 這裡使用者通常係指終端使用者 (受到改動而影響)
與開發者(未來會用到該程式的人)
雙方。
在大多數情況,我們會期望開發者徹底測試他們的 CL,並期望在接受審核時它能夠正常運作。但身為審核者你仍必須去思考可能的「極端案例 (edge cases)」、找出「併發問題 (concurrency problem)」、
嘗試「作為一個使用者去思考」,並且確保「在單純閱讀程式碼時沒有明顯的程式缺陷 (bug)」。
願意的話你也可以去親自去驗證 CL - 如果當改動會帶來直面使用者的影響 (user-facing impact) 時,比方說 UI 改動,驗證 CL 的變化便會是審核者在整個過程中最重要的事情。
單純閱讀程式碼有時很難體會它會如何影響使用者,面對如此的改動如果覺得不方便自己測試的話,可以要求開發者展示 (demo) 此次改動的功能性。
另外,審核功能性時格外重要的一點是,要注意 CL 中是否有「平行程式設計 (parallel programming)」的存在,並是否在理論上會造成 deadlocks 或 race conditions。
這類的問題較難透過執行程式來查覺,通常會需要有人 (開發者跟審核者) 一起仔細思考,確保該問題不會被引入程式庫內。
Note: 這也是不使用可能會造成 deadlocks 或 race conditions 的併發模型 (concurrency model) 很好的理由 - 它可能會讓程式審核或理解上造成困難。
Complexity (複雜性)
一個 CL 是否複雜到超過原本所必須的? 針對任何層級的 CL 請務必確認這幾點 - 單行程式是否過於複雜? 函式是否過於複雜? Class 是否過於複雜?
「複雜」通常代表著該程式很難快速被閱讀程式的人所理解
也意味著很有可能其他開發者在呼叫或修改這段程式時引入缺陷
其中就是一種複雜性便是「過度設計 (Over-engineering)
」,開發者會讓程式過度通用化 (more generic) 超過它原本所需的,或是新增現在系統不需要用到的功能 [1]。
審核者對於必須非常警惕過度設計的發生。
鼓勵開發者解決他們現在
需要解決的問題,而非
那些猜測未來可能會需要被解決的問題
可以在真正面臨到那些未來的問題時才解決它們,因屆時你才能更清晰看到問題的樣貌 (actual shape) 與需求。
- [1] 個人認為和 Donald Knuth 說得「過早最佳化是萬惡的根源 (Premature optimization is the root of all evil)」有類似意思。
自己在開發過程中也常常落入相同窠臼,認為未來可能會需要所以現在先寫起來放。但更常見的狀況是,當實際面臨問題時,狀況可能和原先大相徑庭導致需要重寫。