做 code review 的原則

6 min readMay 3, 2020

近期因為在團隊的角色轉換，工作上做 code review 的時間越來越長，有時候工作一整天沒有寫到一行程式碼。正因為這樣的轉變，讓我意識到現在似乎是一個不錯的時間點，可以把當下我對於 code review 所注重的幾個原則給記錄下來，算是對於自己的反思，也歡迎交流分享。

image source: https://unsplash.com/photos/qAjJk-un3BI

本篇適合閱讀的對象有：專注功能開發的開發者、平常會做 code review 的開發者們。目前的技術團隊使用 GitHub 做版本控制，本篇文章的相關詞彙會以 GitHub 為主。

底下會介紹幾個我個人注重的原則，我深信在做任何事之前，掌握原則是重要的第一步。將原則把握住後，剩下的事情將會水到渠成。

原則一：不 break 既有的功能

身為一個做 code review 的人，每一個 approve 都代表自己的投票。試想某 PR (pull request) 自己已經 approve，那就代表這個 PR merge 進去後，如果會讓既有的功能壞掉，那就代表自己也有部分責任。因此在 review 每一個 PR 的時候，我都會問自己：這樣改之後，會不會 break 既有的功能？

在這樣的原則之下，凡是一個 function 的 input 參數改動、一個 API endpoint 的調整、甚至是一個相依的套件的升級，都要細心的去確保這樣改，不會讓原本的功能壞掉。

舉個簡單的例子，某個 PR 內容把 foo 這個 function 的 input 參數改變，要確認過整個 codebase 裡面所有用到這個 foo function 的地方都有同時修正。聽起來非常容易，但在幾次的經驗下來，在 review 的時候，一樣都要確認過整個 codebase 都有被改過，這個 PR 才能 approve。

我們團隊提倡 open source，也很善用 open source 的資源，大部分我們使用的套件都有持續在維護的狀態，也因此我們鼓勵團隊每一個人都能升級相依性的套件。當套件是 major 版號異動的時候，發 PR 的人要確認有看過 changelog，有把整個 codebase 用到的部份一起做修改。做 code review 的時候，對於相依性的套件升級保持開放，只要做到確保發 PR 的人都有看過 changelog，有做相對應的調整即可。

原則二：避免 magic

這裡的 magic 包含了 magic number 和特別難懂的程式碼。

有時候 code review 到一半，突然出現一個從沒有定義過的整數（例如：42），這時候 review code 的人內心就會疑問，這個 42 是什麼意思？誰定義這個值？要避免這種讓 code review 的人疑問，好的實作是在出現這個數字的當下就賦於一個變數，讓變數命名有意義且能自我解釋。

舉例：

// bad
if ( 
  differenceInMilliseconds(untilDate, sinceDate) >= 
  2000 // magic number
) {
 return untilDate;
}

底下將 2000 這個 magic number 賦予變數。

const FACEBOOK_ACCEPTED_MINIMUM_DATE_RANGE_IN_MS = 2000;if (
   differenceInMilliseconds(untilDate, sinceDate) >=
   FACEBOOK_ACCEPTED_MINIMUM_DATE_RANGE_IN_MS
) {
  return untilDate;
}

上述這樣針對特別的數字加上變數命名，既好懂又能有識別度，會讓 review 這段 code 的人更加順利。

另外，特別難懂的程式碼（像是正規表達式），在使用到它的時候，一樣就先給個變數即註解的表示方式，會更清楚易懂。

// magic regexp
/^[+]*[(]{0,1}[0–9]{1,4}[)]{0,1}[-\s\./0–9]*$/.match(0800001922);

直接給難懂的 RegExp 賦予變數：

const PHONE_REGEXP = /^[+]*[(]{0,1}[0–9]{1,4}[)]{0,1}[-\s\./0–9]*$/;PHONE_REGEXP.match(0800001922);

原則三：小步快跑

在我的經驗中，一個 PR 如果超過 200 行左右（實際異動行數，不包含自動 build 出來的程式碼，抓個概略），其實閱讀起來就會比較吃力。假設自己是發 PR 的人，發出一個破千行的 PR，無形之後會讓做 code review 的人有壓力，review 的人可能會選擇先去處理其他更快可以解決的事情，如此一來這個巨型 PR 就會一再被拖延。如果 PR 可以拆的更小就發出來，對於團隊整體運作絕對是比較有幫助的。

小的 PR 容易被 review，做 code review 的人可以很快給 feedback，發 PR 的人也可以 base 既有的 PR 再下去做其他功能，彼此是不會互相卡住的。而正向循環就會發生，讓團隊運作更加順暢。

另一個能讓 PR 更快被 review 的作法，是寫 code 的人在發 PR 時候，隨手附上截圖、GIF 或是測試的方式。原因是寫 code 的人手邊往往都有相關的程式和資料，在發 PR 的當下隨手做這件事情效益最高，如果讓 review的人去做驗證是沒有效率的，通常會花十倍以上的時間。

結論

在剛開始接觸程式開發時，會羨慕做 code review 的人，總覺得看一看 code 就可以順手按下 approve 好輕鬆。在自己長時間做 code review 過後，才真正體會到做 review 的人並不會比寫 code 的人來的輕鬆。不論是做 review 的人，還是發 PR 的人，雙方都是建立在互信的基礎上合作，因此團隊擁有一致的共識是很重要的。

如果團隊情況允許，應該讓團隊每一個人都擁有 review 其他的 code 的權力和義務。有了權力，代表你可以這樣做；有了義務代表你必須這樣做，review 別人的 code 就是工作的一部分。當你開始在 review 其他人的 code，你會開始反思對方寫這段 code 是什麼意思？他的表達方式易懂嗎？漸漸的自己也可以從中學習到如何溝通。這種換位思考的訓練，是建立一個強大的技術團隊必要的過程。

程式碼不僅僅是人和機器的溝通方式，也是團隊彼此之間的溝通方式。

做 code review 的原則

原則一：不 break 既有的功能

原則二：避免 magic

原則三：小步快跑

結論

Written by Daniel Tseng