Code Review：提升代碼質(zhì)量與團(tuán)隊能力的利器

作者：京東物流 韓旭

1. 引言

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

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

在接下來的內(nèi)容中，我們將探討如何克服這些挑戰(zhàn)，優(yōu)化流程，并分享一些實戰(zhàn)經(jīng)驗，以幫助讀者在自己團(tuán)隊中實施有效的CR。

在此特別感謝JDL平臺技術(shù)部王鑫、劉建設(shè)、劉風(fēng)、楊宏強(qiáng)、鞠萬奎等對本文撰寫的幫助。

2. Code Review的核心目標(biāo)和基本原則

2.1 核心目標(biāo)

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

2.1.1 提高代碼質(zhì)量

CR的首要目標(biāo)是提高代碼質(zhì)量。這包括識別缺陷、識別性能問題、確保代碼遵循一致的設(shè)計原則、提高代碼的可讀性和可維護(hù)性。

2.1.2 風(fēng)險管理

CR的次要目標(biāo)是發(fā)現(xiàn)潛在風(fēng)險。通過CR盡早發(fā)現(xiàn)并解決潛在的代碼問題，以降低未來的修復(fù)成本，降低大型項目返工及上線失敗的風(fēng)險。

2.1.3 促進(jìn)知識共享

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

2.2 基本原則

對應(yīng)CR的核心目標(biāo)，遵循以下幾個基本原則有助于做好CR。

2.2.1 專注于代碼質(zhì)量

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

2.2.2 保持一致性的標(biāo)準(zhǔn)

遵循團(tuán)隊或項目的編碼標(biāo)準(zhǔn)、風(fēng)格指南和最佳實踐。CR應(yīng)該確保代碼更改都符合這些標(biāo)準(zhǔn)，以便于團(tuán)隊成員理解和維護(hù)代碼，保持一致性還有助于減少錯誤和提高代碼質(zhì)量。

2.2.3 保持尊重/建設(shè)性溝通

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

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

3.1 實踐步驟

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

3.1.1 準(zhǔn)備

在提交CR之前，應(yīng)該先自行檢查代碼，以確保基本的代碼質(zhì)量且遵循代碼規(guī)范?？梢酝ㄟ^單元測試、靜態(tài)分析插件（例如SonarLint、JD EOS）、借助AI分析插件（例如Copilot、JD JoyCoder）等來完成。

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

提交評審的時機(jī)，越早進(jìn)行CR則修改的代價越小，至少應(yīng)保證在提測前提交CR及完成修改。

最后，確定適合的評審者，建議選擇具有業(yè)務(wù)經(jīng)驗及較為資深的研發(fā)人員。

3.1.2 執(zhí)行評審

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

3.1.3 修改及完成

開發(fā)者根據(jù)收到的反饋進(jìn)行代碼調(diào)整，改動較大時可能會進(jìn)行多次反復(fù)評審，當(dāng)修改完成后，由具有權(quán)限的負(fù)責(zé)人將代碼合并至相應(yīng)分支。

3.2 CR的最佳實踐技巧

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

3.2.1 有一份明確的checklist

每次評審時，評審者應(yīng)該檢查哪些內(nèi)容？對照一份明確的checklist，有助于我們專注于代碼質(zhì)量，并保持一致性的標(biāo)準(zhǔn)。以下是一份可供參考的checklist。

?設(shè)計：主要評審整體設(shè)計，例如，API設(shè)計簡單清晰，代碼交互、系統(tǒng)交互恰當(dāng)，技術(shù)組件、中間件使用得當(dāng)?shù)取?/p>

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

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

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

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

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

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

?測試：是否有適當(dāng)?shù)膯卧獪y試？需要修改已有的單元測試？

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

?文檔：是否需要更新相關(guān)API說明、Readme等文檔？

3.2.2 避免完美主義

在評審中發(fā)現(xiàn)問題固然重要，但也應(yīng)結(jié)合實際約束及現(xiàn)狀進(jìn)行權(quán)衡，并非所有代碼均要達(dá)到理論上的最優(yōu)解及最佳實踐。只要這次修改讓代碼有所改善，或是向著正確的方向前進(jìn)，那么代碼就是可以接受的。（調(diào)研報告顯示61%的CR沒有發(fā)現(xiàn)缺陷）

3.2.3 拆分為小型MR/PR/Commit

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

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

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

3.2.5 盡早進(jìn)行小而頻繁的評審

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

3.2.6 保持尊重

保持開放的心態(tài)，拋開自負(fù)，不要將個人偏好帶入到CR中。作為代碼審查者，應(yīng)意識到代碼作者更了解其編寫的代碼，并不是每次評審都需要進(jìn)行代碼調(diào)整?；谑聦嵓按a規(guī)范來提出改進(jìn)建議，會使代碼作者更容易接受。作為代碼提交者，提交高質(zhì)量的代碼，是對評審者和團(tuán)隊最基本的尊重。保持開放的心態(tài)，將評審當(dāng)做自我學(xué)習(xí)和提升的過程。

3.2.7 度量和改進(jìn)

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

4. 案例分享

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

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

問題：靜態(tài)緩存先clear，再進(jìn)行加載，如果解析過程異常會導(dǎo)致配置丟失、在高并發(fā)訪問時讀取到錯誤的配置。

改善：應(yīng)使用覆蓋更新的方式。

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-設(shè)計問題、可觀測性不足

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

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

@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內(nèi)容進(jìn)行篩選等邏輯處理代碼
            // ......
            returnList.addAll(page.getData());
        }
    }
    while (page != null && page.getCurPage() < page.getTotalPage());
    return returnList;
}

4.3 案例3-代碼復(fù)雜度

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

改善：將校驗邏輯放到對應(yīng)的校驗方法內(nèi)，保持代碼整潔，降低理解難度。

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：單號規(guī)則
        waybillCodeList.add(WAYBILLCODE_PREFIX + afterSaleOrderContext.getAfterSaleOrderReceiveDTO().getServiceCode());
    } else {
        // 場景2：單號規(guī)則
        waybillCodeList.add(this.preDeliveryId(afterSaleOrderContext));
    }
    // ......
}

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

CR修改后：

public void buildWaybillCodeList(AfterSaleOrderReceiveContext afterSaleOrderContext) {
    List waybillCodeList = new ArrayList();
    // 將多次需求變更的邏輯點聚合到職責(zé)明確的方法內(nèi)
    if (canUseServiceCode(afterSaleOrderContext)) {
        // 場景1：單號規(guī)則
        waybillCodeList.add(WAYBILLCODE_PREFIX + afterSaleOrderContext.getAfterSaleOrderReceiveDTO().getServiceCode());
    } else {
        // 場景2：單號規(guī)則
        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);
    // 只針對一單一品一個數(shù)量的返回true
    return productDetailDTOList.size() == 1 && Objects.equals(productDetailDTOList.get(0).getProductCount(), 1);
}

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

問題：CR過程中發(fā)現(xiàn)無法評估改動影響的業(yè)務(wù)范圍，如有問題可能會影響100%的流量。

改善：增加灰度策略開關(guān)。

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前，善用工具進(jìn)行代碼掃描，可能有意想不到的收獲。

對如下代碼使用JD JoyCoder進(jìn)行自動評審建議。

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給出的評審意見：“代碼中存在一個潛在的邏輯錯誤，這是由于條件判斷中的邏輯運(yùn)算符 && 和 || 的優(yōu)先級沒有被明確區(qū)分。在Java中， && 操作符的優(yōu)先級高于 || ，這意味著在沒有適當(dāng)?shù)睦ㄌ柕那闆r下， && 綁定的條件會先被評估，然后才是 || 綁定的條件。 ”

修改后：

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());
}
// 此段代碼可以進(jìn)一步優(yōu)化，將if里面的條件提前抽取到有明確業(yè)務(wù)語義的變量中，提升可讀性

除目前流行的基于LLM實現(xiàn)的AI掃描工具外，使用傳統(tǒng)代碼掃描也可以發(fā)現(xiàn)潛在問題。

以下代碼通過靜態(tài)掃描工具發(fā)現(xiàn)問題：直接使用“==”進(jìn)行包裝類型Integer的比較，當(dāng)遇到[-128, 127]范圍外時比較結(jié)果會不符合預(yù)期。

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的成果收益

筆者所在團(tuán)隊沒有單獨統(tǒng)計數(shù)據(jù)來佐證CR與線上缺陷的直接關(guān)聯(lián)。線上質(zhì)量與CR、單元測試、質(zhì)量測試、SRE等各方面息息相關(guān)，CR并非銀彈，但是做好CR非常有助于降低缺陷數(shù)量。

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

6. 總結(jié)與展望

本文探討了CR的重要性，它可以提前發(fā)現(xiàn)缺陷，有助于知識共享及團(tuán)隊能力提升，同時分享了CR實踐步驟、技巧、案例等內(nèi)容。當(dāng)然，本文僅是一份參考指南，每個團(tuán)隊根據(jù)其所處現(xiàn)狀的不同，可以根據(jù)本文調(diào)整優(yōu)化各自的實踐流程。

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

未來，CR將繼續(xù)發(fā)揮其關(guān)鍵作用，我們期待AI+CR成為一個更加強(qiáng)大和智能的伙伴，使團(tuán)隊將能夠保持競爭力，持續(xù)提升軟件質(zhì)量和交付速度。

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

審核編輯 黃宇