作者：京東物流 韓旭

1. 引言

Code Review（下文簡稱CR），即代碼審查，是一種通過評審代碼以發現并修正錯誤的實踐。它不是一個新概念，但在軟件開發中，它的重要性毋庸置疑。首先，它可以顯著降低軟件中的缺陷比例；其次，它促進了知識共享，通過評審的過程，團隊成員可以相互學習，增強對系統的整體理解；最后，CR是一種預防措施，它有助于維護代碼的清晰和統一，減輕技術債務，提升系統的穩定性。

盡管CR有諸多好處，實際操作中卻面臨不少挑戰。例如，交付壓力可能導致CR被忽視或流于形式；另一方面，缺乏有效技巧和工具支持，可能會使CR變得低效，甚至引發團隊內的沖突；此外，一些團隊可能會遇到參與度不足的問題，團隊成員不愿意投入必要的時間和精力。

在接下來的內容中，我們將探討如何克服這些挑戰，優化流程，并分享一些實戰經驗，以幫助讀者在自己團隊中實施有效的CR。

在此特別感謝JDL平臺技術部王鑫、劉建設、劉風、楊宏強、鞠萬奎等對本文撰寫的幫助。

2. Code Review的核心目標和基本原則

2.1 核心目標

首先，CR并不是走馬觀花，也并不需要面面俱到，我們先要明確以下幾個核心目標。

2.1.1 提高代碼質量

CR的首要目標是提高代碼質量。這包括識別缺陷、識別性能問題、確保代碼遵循一致的設計原則、提高代碼的可讀性和可維護性。

2.1.2 風險管理

CR的次要目標是發現潛在風險。通過CR盡早發現并解決潛在的代碼問題，以降低未來的修復成本，降低大型項目返工及上線失敗的風險。

2.1.3 促進知識共享

最后，通過CR促進團隊知識共享。CR過程鼓勵團隊成員之間的交流和協作，讓團隊成員相互學習對方的代碼和設計思路。這種交流有助于提高團隊的整體技能水平，同時減少代碼庫中知識的單點問題。

2.2 基本原則

對應CR的核心目標，遵循以下幾個基本原則有助于做好CR。

2.2.1 專注于代碼質量

CR的核心目的是提升代碼質量。這包括但不限于代碼的清晰性、可維護性、性能、安全性和可測試性等，在評審過程中應時刻專注于這些方面。

2.2.2 保持一致性的標準

遵循團隊或項目的編碼標準、風格指南和最佳實踐。CR應該確保代碼更改都符合這些標準，以便于團隊成員理解和維護代碼，保持一致性還有助于減少錯誤和提高代碼質量。

2.2.3 保持尊重/建設性溝通

溝通是CR過程中的核心元素。所有的反饋都應該是建設性的，目的是改進代碼而不是批評個人。作為評審者應針對代碼給出具體、有用的反饋，并在表達時考慮代碼作者的感受。

3. Code Review的實踐步驟與技巧

3.1 實踐步驟

CR的實踐步驟總體分為三步：準備、評審、修改及完成。

3.1.1 準備

在提交CR之前，應該先自行檢查代碼，以確?；镜拇a質量且遵循代碼規范。可以通過單元測試、靜態分析插件（例如SonarLint、JD EOS）、借助AI分析插件（例如Copilot、JD JoyCoder）等來完成。

如果更改較大，考慮將其分割成幾個小的、邏輯上獨立的commit。這樣不僅能使每次評審過程更高效，也便于追蹤和管理更改。

提交評審的時機，越早進行CR則修改的代價越小，至少應保證在提測前提交CR及完成修改。

最后，確定適合的評審者，建議選擇具有業務經驗及較為資深的研發人員。

3.1.2 執行評審

在評審過程中，聚焦在代碼質量方面（可參考下文提供的checklist）?？刂坪妹看蔚臅r長，如果一次評審時間過長，則考慮是否應在準備階段就拆分成多次commit，進行多次評審，而不是在提測前進行一次大型評審。

3.1.3 修改及完成

開發者根據收到的反饋進行代碼調整，改動較大時可能會進行多次反復評審，當修改完成后，由具有權限的負責人將代碼合并至相應分支。

3.2 CR的最佳實踐技巧

遵循以下的最佳實踐技巧，有助于解決CR中遇到的各種問題，并保持高效。

3.2.1 有一份明確的checklist

每次評審時，評審者應該檢查哪些內容？對照一份明確的checklist，有助于我們專注于代碼質量，并保持一致性的標準。以下是一份可供參考的checklist。

?設計：主要評審整體設計，例如，API設計簡單清晰，代碼交互、系統交互恰當，技術組件、中間件使用得當等。

?功能性/非功能性：評審代碼的行為是否符合預期？大多數時候，僅靠評審并不能發現每一行代碼是否如期運行，我們應特別關注一些異常的極端情況，例如，邊界處理、異常死循環、非法的輸入輸出、大報文處理、兼容性問題、線程安全/并發問題、Exception處理等。

?性能/穩定性：對于一些高吞吐量的系統，響應性能尤其重要。例如，確保依賴服務SLA符合預期，超時和重試配置得當，避免產生慢SQL、大量鎖等待、線程阻塞/耗盡等。

?可觀測性：是否在上線后可觀測代碼運行的行為，發生異常時可及時感知？例如，確保方法添加了必要的監控埋點、有正確的日志級別及日志內容。

?復雜度：代碼實現足夠簡單嗎？是否有過度設計？作為評審者應讓代碼盡量保持簡潔，以便讓其他的開發者可以快速理解，降低未來修改時引入新錯誤的風險。

?命名：是否為變量、類、方法等選擇了清晰的名稱？命名應遵守代碼規范，且能夠準確表達代碼的意圖，而又不至于過長難以閱讀。

?注釋：注釋清晰無歧義，應解釋代碼“為什么”，而不是“是什么”。注釋更應解釋一些代碼外的隱含信息，例如，設計的取舍、業務背景、某些看起來很tricky的實現，以及解釋正則表達式、特定算法等內容。

?測試：是否有適當的單元測試？需要修改已有的單元測試？

?風格：是否遵循一致的代碼風格？風格無所謂好壞，但保持一致性的風格，會讓其他團隊成員更容易理解。

?文檔：是否需要更新相關API說明、Readme等文檔？

3.2.2 避免完美主義

在評審中發現問題固然重要，但也應結合實際約束及現狀進行權衡，并非所有代碼均要達到理論上的最優解及最佳實踐。只要這次修改讓代碼有所改善，或是向著正確的方向前進，那么代碼就是可以接受的。（調研報告顯示61%的CR沒有發現缺陷）

3.2.3 拆分為小型MR/PR/Commit

小型的changelist，擁有降低評審難度、縮短評審時間、減少引入錯誤的可能性、易于合并等諸多好處。通常認為將changelist控制在只解決一件事（可以只是feature的一部分），視作合適的大小。我們可以按層進行水平拆分、按功能進行垂直拆分，亦或是結合兩者，有興趣的讀者可以閱讀文章最后引用的google關于CR工程實踐文章。

3.2.4 一次不要評審過多的代碼

建議將每次評審的代碼控制在100~300行，最多不超過500行，每次評審時間不超過1.5小時（調研報告顯示超過這些閾值會導致CR質量及效率大幅降低）。不過根據實際場景不同，讀者可以根據代碼實際的復雜度進行調整。

3.2.5 盡早進行小而頻繁的評審

盡早評審有助于提前發現問題，減少后期修正的成本。編碼階段，在IDE環境安裝靜態代碼檢查工具，提前預先檢查代碼風格、格式等基本錯誤，可減少人工評審的工作量。面對大型代碼變更，將代碼分為更小而獨立的多次commit，盡早進行多次評審，也可提升評審質量，減少返工成本。

3.2.6 保持尊重

保持開放的心態，拋開自負，不要將個人偏好帶入到CR中。作為代碼審查者，應意識到代碼作者更了解其編寫的代碼，并不是每次評審都需要進行代碼調整。基于事實及代碼規范來提出改進建議，會使代碼作者更容易接受。作為代碼提交者，提交高質量的代碼，是對評審者和團隊最基本的尊重。保持開放的心態，將評審當做自我學習和提升的過程。

3.2.7 度量和改進

設定一些度量指標，并持續追蹤趨勢，有助于我們持續不斷改進CR過程。以下是一些可以用作度量的指標，例如，審查時長、缺陷密度、CR率等。

4. 案例分享

以下是身邊真實發生的一些CR案例，與3.2.1章節中的checklist都有相應的對照，供大家參考。為了便于閱讀，部分代碼進行了刪除簡化。

4.1 案例1-異常及并發情況處理不周

問題：靜態緩存先clear，再進行加載，如果解析過程異常會導致配置丟失、在高并發訪問時讀取到錯誤的配置。

改善：應使用覆蓋更新的方式。

public class ReverseSwitch {
    private static Map multiConfigAddress = new HashMap();
    
    public void setMultiConfigAddress(String multiConfigAddress){
        ReverseSwitch.multiConfigAddress.clear();
        // 以下是解析字符串配置映射到Map配置中，省略具體過程
        for (/*.....*/) {
            ReverseSwitch.multiConfigAddress.put(/*.....*/);
        }
    }

    public static boolean isMultiConfigSwitch() {
        // .....
    }
}

CR修改后：

public void setMultiConfigAddress(String multiConfigAddress){
    log.info("ReverseSwitch.setMultiConfigAddress {}", multiConfigAddress);
    Map newAddress = new HashMap();
    // 省略解析過程
    for () {
        newAddress.put();
    }
    // 使用覆蓋更新的方式
    ReverseSwitch.multiConfigAddress = newAddress; 
}

4.2 案例2-設計問題、可觀測性不足

問題：1. 本地緩存每小時失效一次，會集中產生大量RPC請求加載數據（容器數量*外部請求數），當依賴的RPC服務抖動時有可能導致雪崩；2. do while語句在遠程數據異常時，可能循環次數超出預期或產生死循環，導致tp99超時、阻塞或OOM；3. 缺少必要的日志及監控埋點。

改善：1. 使用redis緩存并預加載；2. while內設置最大分頁次數進行break；3. 上層調用增加監控埋點及日志。（由于修改不止一處文件，未一一列出修改后的代碼）

@CacheMethod(key = "vrs.SpareQueryProxyCache.getAllSpareInfo", 
    cacheBean = "localGuavaCacheBean60m", 
    timeout = Constants.REDIS_KEY_TIMEOUT_MINUTES_60)
public List getAllSpareInfo() {
    int pageNum = 0;
    PageDto page;
    List returnList = new LinkedList();
    do {
        page = basicPrimaryWS.getBaseStoreInfoByPage(++pageNum);
        if (page != null && CollectionUtils.isNotEmpty(page.getData())) {
            // 省略對page內容進行篩選等邏輯處理代碼
            // ......
            returnList.addAll(page.getData());
        }
    }
    while (page != null && page.getCurPage() < page.getTotalPage());
    return returnList;
}

4.3 案例3-代碼復雜度

問題：代碼不夠內聚，可讀性不好，開發追加需求時將多個校驗的邏輯寫到了校驗方法外。

改善：將校驗邏輯放到對應的校驗方法內，保持代碼整潔，降低理解難度。

public void buildWaybillCodeList(AfterSaleOrderReceiveContext afterSaleOrderContext) {
    boolean useServiceCode = true;
    // 條件1
    if (condition_1) {
        useServiceCode = false;
    }
    // 其他條件
    if (!canUseServiceCode(afterSaleOrderContext)) {
        useServiceCode = false;
    }
    // 條件2
    if (condition_2) {
        useServiceCode = false;
    }
    List waybillCodeList = new ArrayList();
    if (useServiceCode) {
        // 場景1：單號規則
        waybillCodeList.add(WAYBILLCODE_PREFIX + afterSaleOrderContext.getAfterSaleOrderReceiveDTO().getServiceCode());
    } else {
        // 場景2：單號規則
        waybillCodeList.add(this.preDeliveryId(afterSaleOrderContext));
    }
    // ......
}

private boolean canUseServiceCode(AfterSaleOrderReceiveContext afterSaleOrderContext) {
    List productDetailDTOList = buildMainGiftProductList(afterSaleOrderContext);
    // 只針對一單一品一個數量的返回true
    return productDetailDTOList.size() == 1 && Objects.equals(productDetailDTOList.get(0).getProductCount(), 1);
}

CR修改后：

public void buildWaybillCodeList(AfterSaleOrderReceiveContext afterSaleOrderContext) {
    List waybillCodeList = new ArrayList();
    // 將多次需求變更的邏輯點聚合到職責明確的方法內
    if (canUseServiceCode(afterSaleOrderContext)) {
        // 場景1：單號規則
        waybillCodeList.add(WAYBILLCODE_PREFIX + afterSaleOrderContext.getAfterSaleOrderReceiveDTO().getServiceCode());
    } else {
        // 場景2：單號規則
        waybillCodeList.add(this.preDeliveryId(afterSaleOrderContext));
    }
    // ......
}

private boolean canUseServiceCode(AfterSaleOrderReceiveContext afterSaleOrderContext) {
    // 條件1
    if (condition_1) {
        return false;
    }
    // 條件2
    if (condition_2) {
        return false;
    }
    // 條件3
    List productDetailDTOList = buildMainGiftProductList(afterSaleOrderContext);
    // 只針對一單一品一個數量的返回true
    return productDetailDTOList.size() == 1 && Objects.equals(productDetailDTOList.get(0).getProductCount(), 1);
}

4.4 案例4-增加灰度策略控制

問題：CR過程中發現無法評估改動影響的業務范圍，如有問題可能會影響100%的流量。

改善：增加灰度策略開關。

public void setConsigneeAddress(WaybillAddress targetAddress) {
    // 100%修改至新邏輯：A
    // ......
}

CR修改后：

public void setConsigneeAddress(WaybillAddress targetAddress) {
    // 選取可控的特征屬性，例如按流量比例或商家切量
    if (ThreadLocalRandom.current().nextInt(1000) < ducc.getAddressMontageSwitchRate()) {    
        // 新邏輯：A，使用灰度策略，控制本次改動影響范圍
        // ......
    } else {
        // 原邏輯：B
        // ......
    }
}

4.5 案例5-善用工具

在提交人工CR前，善用工具進行代碼掃描，可能有意想不到的收獲。

對如下代碼使用JD JoyCoder進行自動評審建議。

if (response != null
        && response.getCode() != 0
        && String.valueOf(response.getCode()).length() > 2
        && (KK_PARAM_PREFIX_CODE.equals(String.valueOf(response.getCode()).substring(0, 2)))
            || KK_BIZ_PREFIX_CODE.equals(String.valueOf(response.getCode()).substring(0, 2))) {
    throw new BusinessException(StringUtils.isNotBlank(response.getSubMsg()) ? response.getSubMsg() : response.getMsg());
}

JD JoyCoder給出的評審意見：“代碼中存在一個潛在的邏輯錯誤，這是由于條件判斷中的邏輯運算符 && 和 || 的優先級沒有被明確區分。在Java中， && 操作符的優先級高于 || ，這意味著在沒有適當的括號的情況下， && 綁定的條件會先被評估，然后才是 || 綁定的條件。 ”

修改后：

if (response != null
        && response.getCode() != 0
        && String.valueOf(response.getCode()).length() > 2
        && ((KK_PARAM_PREFIX_CODE.equals(String.valueOf(response.getCode()).substring(0, 2))
        || KK_BIZ_PREFIX_CODE.equals(String.valueOf(response.getCode()).substring(0, 2)))) {
    throw new BusinessException(StringUtils.isNotBlank(response.getSubMsg()) ? response.getSubMsg() : response.getMsg());
}
// 此段代碼可以進一步優化，將if里面的條件提前抽取到有明確業務語義的變量中，提升可讀性

除目前流行的基于LLM實現的AI掃描工具外，使用傳統代碼掃描也可以發現潛在問題。

以下代碼通過靜態掃描工具發現問題：直接使用“==”進行包裝類型Integer的比較，當遇到[-128, 127]范圍外時比較結果會不符合預期。

if (!(request.getSkuList().stream().allMatch(
        sku -> sku.getPreProduce() != null && 
        sku.getPreProduce() == request.getSkuList().get(0).getPreProduce()
    ))) {
    throw new DOSException(ResultEnum.PRE_PRODUCE_UN_SAME.getCode(), ResultEnum.PRE_PRODUCE_UN_SAME.getMessage());
}

修改后：

if (!(request.getSkuList().stream().allMatch(
        sku -> sku.getPreProduce() != null && 
        sku.getPreProduce().equals(request.getSkuList().get(0).getPreProduce())
    ))) {
    throw new DOSException(ResultEnum.PRE_PRODUCE_UN_SAME.getCode(), ResultEnum.PRE_PRODUCE_UN_SAME.getMessage());
}

5. Code Review的成果收益

筆者所在團隊沒有單獨統計數據來佐證CR與線上缺陷的直接關聯。線上質量與CR、單元測試、質量測試、SRE等各方面息息相關，CR并非銀彈，但是做好CR非常有助于降低缺陷數量。

通過搜索公開數據顯示，行業中使用CR的項目，潛在缺陷發現率約在50%~60%之間，大部分的測試，潛在缺陷發現率約在30%左右。同時，數據顯示約75%的CR評審意見影響著軟件的可維護性/可演化性，這表明CR利于軟件系統的長期演化。

6. 總結與展望

本文探討了CR的重要性，它可以提前發現缺陷，有助于知識共享及團隊能力提升，同時分享了CR實踐步驟、技巧、案例等內容。當然，本文僅是一份參考指南，每個團隊根據其所處現狀的不同，可以根據本文調整優化各自的實踐流程。

如今，軟件開發的格局在不斷變化，圍繞CR的實踐也在不斷發展。隨著技術的進步，更智能的工具和 AI 輔助平臺在不斷涌現，這些工具能夠提供更高級的靜態分析、模式識別，甚至預測分析，在潛在問題出現之前識別它們。這種AI上下文感知的能力，將能夠根據項目特定的編碼風格、功能模塊以及依賴關系，提供針對性的CR反饋，甚至不再需要人工評審的介入。

未來，CR將繼續發揮其關鍵作用，我們期待AI+CR成為一個更加強大和智能的伙伴，使團隊將能夠保持競爭力，持續提升軟件質量和交付速度。

7. 參考資料

《Google Engineering Practices Documentation》：https://google.github.io/eng-practices/review/?

《Code Review at Cisco Systems》：https://static1.smartbear.co/support/media/resources/cc/book/code-review-cisco-case-study.pdf?

Wikipeida：https://en.wikipedia.org/wiki/Code_review

審核編輯 黃宇