深度主題文
如何做好 Code Review? 如何寫出更快通過 Code Review 的程式碼?
2024年11月11日
💡 E+ 成長計畫深度主題文
本文為 E+ 成長計畫的深度主題文,開放免費閱讀。E+ 的訂閱讀者除了每週會收到一篇新內容外,也享有過去所有內容的閱讀權限。
如果你對於這類深度內容感興趣,歡迎加入 E+ 成長計畫,除了全系列所有完整深度文章外,也可觀看過去所有的直播回放。
Code Review 是軟體工程中,非常重要的環節。在工作中重要之餘,在面試中,有不少公司的面試官會問「你過去都會如何做 Code Review?」藉此來了解候選人是否知道嚴謹的 Code Review 流程該怎麼做、會照顧程式碼的哪些面向 (例如可讀性、可維護性、效能等等)? 以及藉此了解你平常會如何接收別人給的回饋、以及你都會用什麼方式來給回饋?
在這篇主題文中,我們將探討 Code Review 相關的注意事項。會先從個人的角度出發,談在 Code Review 時,可以如何做好 Code Review;以及反過來說,自己在寫程式時,有什麼注意事項,可以寫出讓別人更快通過你提交的程式碼。
為什麼要 Code Review? 有什麼好處?
所謂的 Code Review,是指在寫完程式碼後,會先提交程式碼,讓團隊中的其他人幫忙看、給回饋,原作者基於回饋來修正,直到通過某個標準後,程式碼才會正式被合併。
但是為什麼要 Code Review? 有什麼好處? 以下從幾個角度切入來討論:
- 確保程式碼庫的品質不會下降:很多時候開發者可能為了趕時間,或者因為過去沒有養成良好的洗寫程式習慣,所以會寫出品質不佳的程式碼。Code Review 可以作為一個關卡,確保品質不佳的程式碼不會被合入
- 除了確保當前的程式碼庫品質不會下降外,透過 Code Review 也能確保程式碼未來的可維護性是足夠好的。很多時候自己寫程式會有盲點,可以透過其他人的角度來看程式是否好維護
- Code Review 是團隊資深工程師,來協助初階工程師成長的好時機。從對程式語言的理解,到框架的使用,再到各種設計模式,都可以透過 Code Review 傳承
- 更進一步來說,Code Review 是團隊文化的塑造。如果做得好,能協助發展出好的團隊文化,例如正向的團隊交流氛圍;反之,如果做不好,可能讓團隊充斥有毒的文化。
- 反過來說,對於剛加入團隊的人,參與 Code Review 是個學習機會,在 Review 的過程中,能學習系統中不同部分的,也可以去看別人怎麼寫,贈加對於寫程式的理解
Code Review 不該包含什麼?
上面談了 Code Review 可以帶來幫助,但與此同時,有些事情不該是由 Code Review 來達成,在這邊展開來討論。
首先,如果是能自動化處理的東西,不該由 Code Review 來做。舉例來說,縮排要多少個空格、要不要加 ;
到每行程式的尾端、要用單駝峰還是雙駝峰格式。這些東西都可以透過 Linting 工具來達成。工程師的時間寶貴,不該花在這些事上面。如果你發現 Code Review 出現這類討論,那推薦可以主動去設置 Linting 來一勞永逸解決。
第二,Code Review 的核心目的不是幫忙看有沒有 bug。前面有談到,Code Review 作用之一是確保程式碼庫的品質,但這不代表幫忙看有沒有 bug。事實上,先前微軟有發表一篇研究,發現 Code Review 對於找到 bug 的功用來說不大。要確認沒有 bug,還是需要透過測試來做到,包含自動化測試與人工手動測試。
Code Review 的流程
實際開始看程式碼前,推薦要先了解脈絡,了解要 Review 的改動是什麼,可以透過看相關的產品文件、技術設計文件,或者是 Code Review 附上的描述。另外,也要去了解這次 Review 的重點是什麼,例如是一般性的 Review,或者需要特別著重某些面向,又或者因為時間趕,某些面向可以不是這次著重的要點。
接著,開始看程式碼後,可以依照以下的流程進行:
- 先掃過去,對於整體改動有基本理解,並看有沒有重大的問題。
- 接著,針對最核心的部分,切入給予回饋 (下面會談可以切入的角度)。在看核心程式碼前,也可以先看測試案例,藉此來更了解核心程式碼想做到的事情。
- 最後,重新掃過程式碼,看除了核心的部分外有哪些其他可以給的回饋。
Code Review 可以從哪些角度切入?
把自己當成程式碼庫的守護者,守護什麼呢?
以下幾點是具體看程式碼前,就可以先檢查的項目 (如果你是寫程式的人,要讓別人幫你 Code Review 前,推薦要先替自己檢查一次以下的項目)
- 過去寫的測試都有通過,沒有未通過的既有測試
- 與其他分支的衝突都有先被解完,是在合入主要分支的狀態
- 如果是前端 (網頁或行動端),有附上相關的 UI 改動截圖
在檢查完上面的項目都通過後,接著就可以詳細開始看程式碼。這時可以從以下角度切入。
程式設計面向
- 實作的功能都有正確嗎?
- 沒有會影響到不該影響的副作用 (side effects)?
- 可能有錯誤的地方,都有做錯誤處理 (error handing) 嗎?
- 有考量兼容性 (backward compatability)? 新的改動會不會導致其他功能出問題? 例如:如果 API 加上新的輸入參數,就要思考會不會讓線上已經在呼叫該 API 的客戶端呼叫失敗。
- 確認是否複雜度沒有過高、沒有過度工程 (over engineering)? 換個角度想,可以思考是否有更簡單的寫法? 如果有的話,可以在 Code Review 提出
可讀性
- 程式碼是否造成過大的心智負擔? 如果身為讀者沒辦法輕易看懂,就可以給原作者回饋,請原作者重構。當可讀性提升,未來要維護的人,也相對不容易因為誤解而出錯。
- 命名 (naming) 是否合理、好懂? 命名用單駝峰或雙駝峰這種可以靠 Linting 工具,但在給定風格下,是否選擇易懂的命名,則需要靠 Code Review
一致性
- 新增的程式碼是否與原程式碼庫一致? 風格上的一致性應該由 Linting 工具來解決,但有些一致性是 Linting 工具沒辦法覆蓋到的。例如選擇用 A 依賴而非 B 依賴,這種實作選擇,通常需要已經對程式碼庫很熟悉的人來給回饋,確保整體程式碼庫一致性高,這樣對未來要維護的人心智負擔會比較小
- 是否遵照規範 (convention)? 例如程式語言的風格規範、框架的使用規範
最佳化
- 有沒有時間與空間複雜度上可以優化的地方?
註解
- 有寫註解的部分,是否真的都需要註解? 是否可以重構讓程式碼更清楚,已免去註解? 舉例來說假如
let d = 0 // 總共花費的時間
,可以被改成let totalDaysSpent = 0
,讓命名直接代表意思,就不用額外的註解 - 有寫著註解的部分,是否都是針對「為什麼」來寫?
- 不夠直觀的部分,是否加上註解有助於讀者理解?
測試
- 測試案例是否都合理,能夠測到該測的嗎?
- 該測試到的極端案例 (edge case) 都有被覆蓋嗎?
文件更新
- 許多團隊在更新程式碼的時候,往往沒有同步更新文件 (例如技術設計文件),導致後面要維護的人,發現文件與程式碼對不在一起。針對這種情況,在 Code Review 時也可以特別確認一下相關文件是否更新了
如何寫 Code Review 留言 (comment)?
在 Code Review 時,當你有不同於程式碼作者的觀點時,會留言給回饋。寫留言是一門藝術,有些注意事項要特別放心上,讓我們一起來看看給 Code Review 的留言,要注意什麼。
寫理由加上給範例
很多人在寫 Code Review 的留言,可能只會寫一段看到的問題,例如說「這邊不該用 ABC 而是要用 XYZ」。這種留言有兩個問題,一個是沒講「為什麼」,第二個是沒有給範例。沒講為什麼會難以讓人信服,而沒有給修改的範例,可能會讓對方不知道具體要怎麼改。因此,推薦理由與範例,是在留言時務必要加上的。
針對程式碼,不要針對人
針對人的留言可能是像這句「你沒有檢查空值 (null)」,這裡把焦點放在「你」。如果要換句話說,可以改成純粹針對程式碼,像是「這個輸入可能會有空值 (null),這可能會導致錯誤,推薦在這邊加上空值檢查」。或者是用問句,來引導程式碼原作者思考「如果這邊是空值的情況,會導致潛在的問題嗎?」
保持友善
很多人在 Code Review 時,雖然是針對程式碼,但因為語氣不友善,對於團隊整體是很傷的。舉例來說,有人會說「當 XXX 顯然沒辦法有任何好處,為什麼要用?」這種比較銳利的語氣,可以被改成「XXX 在這邊似乎沒有額外的作用,但這樣寫讓程式碼變比較複雜,或許我們可以考慮改成 OOO,你覺得呢?」。當語氣友善,團隊的協作氛圍也會比較好。
Nit
個人觀點與非必要修改
同樣的功能,可能會有很多種不同的實作方式。除非是在上一個段落提到的功能、可讀性、一致性、最佳化等面向有問題,是非改不可;不然很多時候可能是個人觀點,不一定是非改不可。
這時可以在 Code Review 的留言中加上 Nit
,這代表雞蛋裡挑骨頭 (Nitpick),換句話說就意味「這是個小問題,能改的話很好,不改也可以接受」。
給予稱讚
除了寫可以改進的東西外,如果讀到某些寫特別好的地方,也不要吝於給予好的回饋,例如「這段 XXX 這樣寫特別精妙,學到了一課」這類留言。Code Review 是一件團隊的事,適時給予稱讚,可以營造良好的團隊氛圍,雖然看似小事,但非常有幫助。
如果自己先前的留言不精準,記得要修改
寫留言是個迭代的過程,換句話說可能會修改自己前面寫過的留言。一般流程上,會邊看程式碼邊寫留言,但有可能在讀更多後,發現前面有的留言可能不盡然正確,這時候不要猶豫,回頭修改自己原本留的東西。
一般的程式碼庫工具 (例如 GitHub 或 GitLab),都有功能是可以留多個言後,一次送出所有留言。推薦用這種功能,這樣在送出前,可以回頭看前面寫下的留言,確認都無誤後再一次送出。
LGTM
當經過幾輪 Code Review,程式碼都沒問題後,就可以給 LGTM (Looks Good To Me) 的留言,然後點下通過,結束 Code Review 的流程。
如何讓人更快通過你提交的程式碼?
講完 Code Review 可以怎麼寫留言後。在本篇的最後,想來聊在實務上可以怎麼讓別人更快通過你寫的程式碼。
除了上面提的都要做到外,一個核心的關鍵,是要有同理心。同理心意味著在提交程式碼時,你要先站在幫你看程式碼的同事的角度,然後想怎沒要讓他們能夠更輕鬆地完成 Code Review。
舉例來說,在提交的程式碼 PR (Pull Request),一定要有相關描述、附上相關的功能說明文件,或者是附上相關的改動比較、截圖等。讓人可以很輕易理解該 PR 是在做什麼的。
又或者每次的提交,都不要過大。以業界來說,每次提交的程式碼量,若能在 50 到 300 行程式碼,會是比較理想的。超過 250 行的程式碼,就會讓人看得比較吃力。假如你有某個很大的功能,可以考慮拆成多個 PR,讓每個 PR 的量不會太大。
最後就是在寫程式時,務必要想「未來要維護的人能快速理解這段程式碼嗎?」。舉例來說,下面這段程式碼,會讓人相對難以理解,未來要維護的人,負擔就會大。讓人 Code Review 也會負擔大。
function getTasks() {
// 命名模糊
let incompleteTasks = [];
for (let i = 0; i < tasks.length; i++) {
if (!tasks[i].completed && tasks[i].type === "CODING") {
incompleteTasks.push({
title: tasks[i].title,
user: tasks[i].user.name, // 命名規範不一致 (userName vs user)
});
}
}
incompleteTasks.sort((a, b) => {
// 多餘的變數
return a.user.localeCompare(b.user); // 讓人難以一眼看出在做什麼的比較
});
return incompleteTasks;
}
假如我們要調整,可以改成下面的版本。把一段相對多操作的函式,拆出成為幾段小的、明確的程式碼。讓人在讀 getIncompleteCodingTasksSortedByUser
的內容時,幾乎像是在讀英文一樣簡單,這會讓要幫忙 Code Review 的人讀起來也比較輕鬆。
function getIncompleteCodingTasksSortedByUser(tasks) {
return tasks
.filter(isIncompleteCodingTask)
.map(extractTaskTitleAndUser)
.sort(sortByUserName);
}
function isIncompleteCodingTask(task) {
return !task.completed && task.type === "CODING";
}
function extractTaskTitleAndUser(task) {
return {
title: task.title,
userName: task.userName,
};
}
function sortByUserName(task1, task2) {
return task1.userName.localeCompare(task2.userName);
}
小結
以上是關於如何做好 Code Review 的介紹。相信透過上面這些方式,不論你是要幫別人看程式碼的人,或者是寫完程式碼要給人看的,都能夠讓整個 Code Review 變得更順暢。
💡 E+ 成長計畫深度主題文
本文為 E+ 成長計畫的深度主題文,開放免費閱讀。E+ 的訂閱讀者除了每週會收到一篇新內容外,也享有過去所有內容的閱讀權限。
如果你對於這類深度內容感興趣,歡迎加入 E+ 成長計畫,除了全系列所有完整深度文章外,也可觀看過去所有的直播回放。