2018
Apr
16
Code Review 是一件很重要的事，很多 Bugs 都可以經由 Code Review 而發現，學會怎麼 Review 別人的程式更是一門學問， 每個專案大小與未來性不同，不能永遠都使用同一套標準，小專案或是暫時的 Script ，不需要看得太細，只能程式能動就好，而目標是跑 5 年以上的大專案，就得多花點眼力來檢查，除了 review 程式邏輯之外，其它還有很多地方要特別留意，以下是我多年來 code review 的經驗。
- Beta / Production 環境：一般軟體開發都會有區分多個環境，最少有 Beta(staging) ， production 兩個環境 ，工程師很容易只針對其中一個環境來開發測試， Commit 的程式也只測過 Beta，並沒有想到 production 的部分 ，所以 review 時要看看新增的程式是否有處理不同環境的設定，最常見的就是加了一個 Beta 測試用的設定值，而少加了 production 所需的設定。
- 是否有 Require library, Import lib path，有些工程師寫程式都不測試的，程式邏輯寫完了，卻沒有載入對應的 library ，一執行就直接掛點，建議要求所有 RD 寫 unit test or Integration test 來避免這種問題，若是公司內沒有這些規定，那麼 review 時就多看兩眼。
- Package dependency：如果有升級 Library / Package 要記得改版號，工程師很常自已的環境測試完，就忘了改 pom.xml / package.json 等版號。
- Reuse function： 想想看是否有其它工程師曾經實作過相同的程式邏輯，盡可能重複使用同一段程式，不要寫一模一樣的 code。
- 極端值： 程式是否能夠處理各式各樣奇怪的數值，例如負數，0 ，big number ，null，空字串，undefined，large array。
- 可讀性：保持程式簡單易懂，如果程式邏輯著實複雜得誇張，可以把複雜而且不常更動的部分打包藏進 Library 裡，讓外部程式執行流程乾淨好懂 (Facade pattern)。
- 程式是否會影響到其它的 service ，input / output 改變或是底層 library 變更，有可能會產生其它服務 Bugs 。
- 程式一致性：不要創造新的變數名稱，保持原來程式的命名，但是如果原程式命名有很大的問題，那就把有問題的變數全部改掉，不要只改你有動到的地方。
- 把自已當成一個測試工程師，測試一下程式邏輯是否有 Bug。
- 想一想是否有機會提升效能，雖然程式不一定有效能問題，但是多研究效能方面的知識，可以提升自已的能力，以應付未來流量更大的系統。
- 是否有安全性漏洞，例如 SQL Injection, XSS, File Injection, Buffer overflow。
- 前置作業： 新程式是否需要搭配修改 DB Schema，或是必需先修改 production settings，是否需要多增加一個開關設定上線日，時間到才自動啟用新功能，如果沒有事先設定好這些，程式 release 當天很可能就立馬倒站，以下幾種狀況都會造成系統出錯。
- 修改 Code SQL 語法，但是 DB Schema 沒有改。
- 程式需搭配 DB table index，如果沒有先建好 index 會造成 Slow Query 。
- Production 需增加新的設定，而設定與程式邏輯分別在不同的 Repositories，甚至是安裝在不同的機器，無法同時 Release 。
- Release：程式是否能夠 release ，有些功能還在開發當中，不能公開給外部的 User 使用，如果 merge 這段程式，是否會造成提前 release 。
- Rollback 機制：如果新版程式在線上運作一段時間後，出現問題要如何 rollback。
- Release 流程：一般線上機器不會只有一台，一般來說至少有 3 台以上，在 Release 的過程中，會一台一台慢慢安裝，造成線上有一部分機器是舊的程式，一部分機器已經安裝新的程式，新機器建立的資料格式，舊版的程式無法正確處理，而造成系統 crash。
- 資料格式改變：移除即有欄位，很可能會造成系統錯誤。
- Cache 內容變更：memcached, redis, http cache, browser cache, session cache 以上 cache 都會存在系統一段時間，資料格式改變時，並不會把即有的 cache 一並更新，這時線上會同時存在新舊兩種 cache，需確定程式讀取 cache 時，不會因格式改變而出錯，另外在測試的過程中就有可能因為新版的 cache 資料造成線上環境出錯，很可能在 review 的當下就已經有新版的 cache 格式出現在線上環境。
- 回傳格式改變，前端是否有相對應的變更，Release 流程是否要調整。
- 是否需要 On the fly data migration
- Merge 順序：如果同時有多個 Pull Request，每個 PR 相依性是否有正確設定，底層 Library 優先 Merge 。
良好的 Code Review ，可以讓系統更加穩定，不會因為突發性的 Bug 而被 Call 上來緊急修復。