程式碼評審又稱為（Code Review，簡稱CR），關於CR，開發同學其實都不陌生，現在大部分公司的專案開發流程中，它都是必不可少的一個環節，CR的好處也都耳熟能詳。不同的公司對於CR的方式、質量要求標準都不一樣。這篇文章主要講的是在程式碼評審的過程中，會對哪些程式碼和方向進行評審，給大家在接下來的CR中作為參考。
一、評審節點
很多專案一般把CR的時間節點放在上線前，如果專案越大，CR的節點越靠前越好，越靠後臨近上線前，開發同學越不願意去修改，因為測試過的程式碼只要出現改動，就有發生問題的風險，測試也需要重新迴歸，有可能會產生新的缺陷，或者將問題帶到生產。下面是我們專案流程中CR的節點，一般會在聯調前、提測後、上線前各進行一次程式碼評審。
二、評審內容
1、設計方案
設計方案一般是開發前期定好的方案，在擁有一份完美的設計方案的理想狀態下，在CR階段只需在閱讀程式碼時判斷，是否遵循設計方案進行開發即可。
一份相對完整設計方案通常會包括：
但是現實情況是，現在很多公司沒有設計評審環節，專案開發前並沒有進行方案設計。可能有以下幾個原因：
第一：專案太小，認為沒有必要。
第二：沒有意識到設計方案的好處，覺得浪費時間，在方案設計的過程中，需要調研、設計、畫圖、出方案，少則一兩天，多則三四天。在緊張的開發週期內，專案都希望越早開發完成越早上線。
第三：即使開發同學做了設計方案，也不會進行設計方案的評審。因為每次專案都需要將相關人員和評審負責人拉到一起，進行半個小時或者一個小時的方案評估。所以因為各種原因，最終設計方案都成為了需求文件的變種。
一般註釋是什麼時候寫？
經常聽到這樣的回答：註釋應該寫於程式碼之前，提前捋清楚邏輯和思路，將註釋寫好，這樣也無需反覆修改，程式碼能更加整潔清晰。其實設計方案也像是一個超級大的註釋，提前整理好開發的思路，這樣也能夠事半功倍。但是提前做好技術設計方案不僅僅是為了在開發的過程中更加順利，同時也是程式碼評審過程中的一大助攻。
很多時候我們拿到需要評審的程式碼都是通過檔案的順序從上而下進行逐行閱讀，這種方式往往每個檔案的上下文都無法銜接，理解了第一個檔案但是切換到第二個檔案依然不知所云，往往需要藉助註釋、口述或自行查閱上下文檢視程式碼等方式進行理解。但是通過一份完整的設計方案我們便可以瞭解大致的需求、開發者的設計思路，並且跟隨者設計方案的結構和思路對程式碼的主線進行閱讀。
在寫了設計方案並設計方案已通過評審的專案中，在程式碼設計方面更多的側重主要在於程式碼是否遵循設計方案進行開發，設計方案的合理性、可行性、可擴充套件性等在設計方案評審階段已經通過了討論有了結論。而對於沒有設計方案或沒有進行設計方案評審的專案，在理解程式碼的同時，也需要對整體方案的設計進行評估，而，即使方案設計並不合理，一旦經過測試面臨即將上線，也沒有時間和條件去進行調整。
在設計上，一般會關注下面幾個方面：
複用：功能、元件是否可以提取公共方法？是否重複造輪子？同一程式碼不應該重複兩次以上。可擴充套件：當需求發生改動，是否能夠使用少量的改動達到我們的目標，適應未來的變化？過度設計：很多時候由於早期為了能夠擁有更好的擴充套件性，進行了很多抽象和封裝，使其複雜化，造成了過度設計。依賴倒置：模組之間的依賴是否合理？一旦模組發生修改，影響面是否能得到有效的控制？
2、可維護性
可維護性這個詞其實意味著很多，比如，符合團隊規範，複雜度善可、可讀性較高、可擴充套件性也還不錯、結構合理，前面做的很多優化設計其實都在為之後的可維護做鋪墊。
每個團隊都有自己的一套編碼規範，在評審的過程中需要注意是否符合團隊的編碼規範。但是大部分的編碼規範都可以用工具進行約束，能夠使用工具約束的內容，儘可能不必在評審時花時間去關注。可以將關注點放在工具約束之外的規範上，比如：
命名：首先格式（中劃線、小駝峰、大駝峰、下劃線）需要符合規範，不管是檔案命名或者變數命名，儘可能符合其功能特性，能夠通過命名知道它的含義，無須增加註釋去特意說明。註釋：是否在關鍵程式碼內增加註釋說明？是否符合正確的規範？複雜度：複雜度是否在合理範圍閾值，推薦文章前端程式碼質量-圈複雜度原理和實踐不合理的程式碼：每個專案都有一些難以維護的舊程式碼，在這個基礎上繼續新增程式碼，也許可以很快的解決當下問題，但對於日後來說，只會讓它更加難以維護。及時對不合理的舊程式碼進行重構和優化就顯得尤為重要。
3、健壯性
程式碼是否具備安全性和健壯性，對任何一個團隊來說，無疑都是非常重要的。
XSS：XSS攻擊詳細內容，推薦文章前端安全系列（一）：如何防止XSS攻擊？CSRF：CSRF攻擊詳細內容，推薦文章前端安全系列（二）：如何防止CSRF攻擊？邏輯邊界處理：是否有考慮到程式碼的邊界邏輯？互動邏輯是否全面？異常錯誤處理：一旦丟擲異常或者錯誤，頁面或者執行的程式碼是否會崩潰？資源釋放：定時器是否及時清空？記憶體及時清理？相容性：是否有瀏覽器版本相容？手機機型相容？歷史資料相容？介面相容？小程式版本相容？資料展示：對於資產、購物車金額等關鍵資料的展示，儘可能直接展示後臺返回資料，前端不做計算。資料校驗：對傳輸/接收的資料都進行校驗、認證，確保資料的來源和正確。校驗有效位、計算精度、完整性、一致性、時效性（獲取時機是否正確、快取是否更新）資料轉換：資料轉換處理一定要經過充分的測試驗證，並且儘量選取源資料進行傳輸，而並非轉換後的資料。
4、功能範圍
很多人認為功能特性的範圍是測試應該去保障的，程式碼評審時不需要去關注開發了什麼功能。但其實現狀是，我們上線的程式碼往往有很多屬於夾帶私貨，比如，上個迭代有一個影響不大的小Bug，趁著還沒被發現，偷偷將它帶上線，或者，發現上一次寫的程式碼太蠢了，還有更好的解決思路，於是潔癖發作，默默地改了，還覺得自己棒呆了。但是測試只知道本次迭代的功能特性，除了迴歸主功能之外，並不知道還有其他需要重新測試的地方。如果開發同學剛好對自己的程式碼非常自信，覺得一定沒問題，沒有通知到測試迴歸。根據墨菲定律，這種往往覺得沒有問題的程式碼，最後...都能夠引發線上故障。
影響範圍：底層架構、元件或者方法的修改，是否確認影響範圍，每個受影響的依賴都能正常使用。修改範圍：是否屬於本次迭代正常上線的功能範圍，有沒有對本次範圍進行變更，是否通知到測試同學。
5、監控/埋點覆蓋
新增監控的前提是公司有一套監控系統，除了定義好的異常監控場景以外。通常新增的一些關鍵功能、頁面等也需要加入監控，提前加好監控程式碼，無需等到要查問題時，才記起來忘記加監控了。
監控：監控一般用於監控資料的異常的情況，頁面的渲染異常、資料的一致性、正確性。比如：在一些關鍵資料的邏輯上，如果介面返回的資料與原有約定不一致，新增了監控之後，我們就能快速的響應、解決問題。不至於等到引發更多的錯誤之後，才能看到問題。埋點：埋點一般用於統計使用者操作行為的資料，大部分場景下需要埋點的資料產品經理都會提供。
6、合規
合規這個詞在金融行業非常普遍，但也因為隨著人們越來越注重隱私和安全，法律法規日漸完善，對合規這個詞也不再陌生。如果應用不合規，就將面臨被下線的風險，而有一部分不合規的內容，可能無法通過測試測出。所以在CR的過程中對合規性問題的審查，就尤為重要。任何使得使用者隱私洩露的操作都需要禁止。
敏感資訊展示：使用者關鍵敏感資訊是否直接展示。敏感資訊明文上報：使用者關鍵敏感資訊是否直接明文傳給後臺，沒有做加密處理等操作。敏感資訊儲存：保證使用者資訊的安全，對使用者的敏感資訊不儲存在不安全的地方，比如web storage等。
7、效能
C端應用對前端的效能要求會比較高，程式碼評審時也可以關注一下比較常見的問題。前端效能優化 24 條建議（2020）
圖片大小：圖片是否有進行過壓縮處理，非頁面級的圖片一般不要超過200kb。http請求：頁面初始化請求過多？白屏時間過長？初始化載入資料是否在合理範圍內？懶載入：是否可以通過懶載入或者按需載入進行優化？快取資料：需要重複載入資料時，可以通過快取資料減少請求。
8、tips
checklist：在review的過程中，可以發現很多TODO List，比如增加了配置，在上線前需要先發布配置平臺，比如增加片，要記得釋出CDN，比如測試環境新增了測試程式碼，需要生產重新測試等等，所以在每次CR時，可以生成一份上線前的checklist，每次上線前檢視並執行，這樣能夠確保不會遺漏。少吃多餐：經過很多次的CR能夠感覺到，每次評審的程式碼越多，質量就會降低，評審時間過長，都會產生疲勞感，並且一些小細節都會更容易忽略。所以每次評審的時間最好控制在30min~60min左右為佳，可發起多次評審，少吃多餐~好的程式碼：每次CR都在像是找茬，找到不合理的地方。但其實，找到優秀合理的程式碼也可以促進大家的進步。大多數專案CR並不是全員參加，所以將好的程式碼整理出來，生成一份最佳實踐，可以供大家學習參考，擴充套件一些新思路。
三、常見問題
前段時間聽了我司一位資深CR大佬的講座，列了很多平時在CR過程中發生的一些常見問題，我在此基礎上進行了一些新增和擴充套件，供大家參考。
第三方庫
第三方庫的新增、刪除、版本號的修改（包括新增小箭頭），都要確認好修改範圍，確保瞭解庫升級所帶來的影響。不得隨意修改版本號，第三方庫哪怕是小版本的升級也不能保證對當前專案或者當前依賴包沒有影響，為了避免造成線上問題，最好鎖死版本號。
// package.json
// before
{
"dependencies": {
"eslint": "7.27.0",
"eslint-plugin-vue": "7.15.1",
},
}
// after
{
"dependencies": {
"eslint-plugin-vue": "^7.15.1",
"typescript": "^4.3.2"
},
}命名
命名不統一，導致閱讀的成本過高，同表示數量，但是在a檔案命名quantity，b檔案命名amount，c檔案命名count。
// a.js
const quantity = 1;
// b.js
const amount = 1;
// c.js
const count = 1;迴圈引用
JavaScript 模組的迴圈載入
檔案的相互引用，可能會導致引用報錯undefined。儘量避免檔案之間的相互依賴，可以使用eslint或者webpack進行約束。
// a.js
import b from b.js'
// b.js
import a from 'a.js'複雜的判斷條件
一般邏輯判斷非常複雜一般前期沒有想得很清楚，或者後期的維護不斷的迭代，持續往上疊加，在這種情況下，邏輯會變得越來越複雜，在開發或CR時可以考慮重新梳理清楚，重點檢視，進行優化。
if (!somethingA || somethingB && (!somethingC || somethingD)) {
...
}
or
if (...) {
if (...) {
...
} else if (...) {
...
} else {
...
}
} else {
...
}邏輯範圍變化
此問題一般出現在增量程式碼上，因為前面的條件判斷的範圍變小了，導致後面的邏輯處理的範圍變大了。此時要注意這種範圍的變化，是否真的符合預期，還是隻想修改一處邏輯，卻不小心影響到了後面的邏輯。
// before
if (type === 1) {} else {}
// after
if (type === 1 && isShow) {} else {}異常處理
確保所有的邊界邏輯都已經處理或者無需處理。
// else為空，確認無需處理
if (conditionA) {}
// 報錯catch
UserService.getList().then()
// try...catch，未處理catch
try {...} catch() {}⼤概率正確 !== 正確
前面說過的墨菲定律（墨菲定律真好用）：凡事只要有可能出錯，那就一定會出錯。下面setTimeout的取值方式相信很多人都見過，使用500毫秒的延遲，使偶現的問題，“不再出現”。但其實只是將出現的概率變小了，該出現的問題還是會出現。治標不治本。一定要找到出現問題的原因，真正解決它。
setTimeout(() => {
...延時拉取介面 or do something
}, 500)
=> 正確 !== 小概率出錯object鏈式取值
監控平臺總是會出現很多這樣的報錯xxx is undefined，大部分的原因主要是因為我們在物件取值時，喜歡直接點點點。建議使用lodash的get或者?.??。
const a = productObj.something.a;
=> productObj.something是否一定有值？
// lodash
const a = get(productObj, 'something.a')
// 或者雙問號操作符
?.??隱式型別轉換
隱式型別轉換容易出現很多問題，==可以使用eslint避免。
if ( amount == '22' ) {}
+new Date()css重複樣式
很多時候CR時都會忽略css，覺得它翻不出什麼浪花，但是在像小程式這種對程式碼包的體積有嚴格要求的專案中，CSS的精簡就顯得非常重要了。很多時候在寫樣式時，都會根據視覺稿一股腦全部寫完，但其實很多可以繼承的屬性，是不需要重複書寫的。
.page {
font-size: 18px;
font-family: sans-serif;
line-height: 18px;
color: #000;
...
.box {
font-size: 18px;
font-family: sans-serif;
line-height: 18px;
color: #000;
...
}
}每個團隊對CR的要求和規範都不一樣，大家可以選擇適合自己的。如果還有更多建議歡迎留言補充~