分类

Posted in

Code Review：提升代码质量与团队能力的利器_AI阅读总结 — 包阅AI

by baoyueai

包阅导读总结

1.

关键词：Code Review、代码质量、团队能力、实践步骤、技巧案例

2.

总结：本文介绍了 Code Review（代码审查），指出其可降低缺陷比例、促进知识共享等好处，也提到实际操作中的挑战，阐述了核心目标、基本原则、实践步骤与技巧，分享了相关案例和成果收益，最后总结并对未来发展进行展望。

3.

主要内容：

– 引言

– 介绍 Code Review 及重要性。

– 指出实际操作中的挑战。

– Code Review 的核心目标和基本原则

– 核心目标包括提高代码质量、风险管理、促进知识共享。

– 基本原则有专注于代码质量、保持一致性标准、保持尊重/建设性沟通。

– Code Review 的实践步骤与技巧

– 实践步骤分为准备、评审、修改及完成。

– 最佳实践技巧包括明确 checklist、避免完美主义、拆分小型 MR/PR/Commit、控制评审代码量、尽早频繁评审、保持尊重、度量和改进。

– 案例分享

– 列举异常及并发情况处理不周、设计与可观测性不足、代码复杂度、增加灰度策略控制、善用工具等案例。

– Code Review 的成果收益

– 指出 CR 虽非解决线上缺陷的唯一手段，但有助于降低缺陷数量，利于软件长期演化。

– 总结与展望

– 总结 CR 相关内容，展望未来更智能工具和 AI 辅助平台的发展。

思维导图：

文章地址：https://mp.weixin.qq.com/s/JLNQnqW450yjbPCkyKeWOQ

文章来源：mp.weixin.qq.com

作者：京东物流??韩旭

发布时间：2024/7/23 3:53

语言：中文

总字数：6194字

预计阅读时间：25分钟

评分：84分

标签：代码审查,代码质量,团队协作,最佳实践,AI辅助工具

以下为原文内容

本内容来源于用户推荐转载，旨在分享知识与观点，如有侵权请联系删除 联系邮箱 media@ilingban.com

引言

理解，首先 MCube 会依据模板缓存状态判断是否需要网络获取最新模板，当获取到模板后进行模板加载，加载阶段会将产物转换为视图树的结构，转换完成后将通过表达式引擎解析表达式并取得正确的值，通过事件解析引擎解析用户自定义事件并完成事件的绑定，完成解析赋值以及事件绑定后进行视图的渲染，最终将

Code Review（下文简称CR），即代码审查，是一种通过评审代码以发现并修正错误的实践。它不是一个新概念，但在软件开发中，它的重要性毋庸置疑。首先，它可以显著降低软件中的缺陷比例；其次，它促进了知识共享，通过评审的过程，团队成员可以相互学习，增强对系统的整体理解；最后，CR是一种预防措施，它有助于维护代码的清晰和统一，减轻技术债务，提升系统的稳定性。

尽管CR有诸多好处，实际操作中却面临不少挑战。例如，交付压力可能导致CR被忽视或流于形式；另一方面，缺乏有效技巧和工具支持，可能会使CR变得低效，甚至引发团队内的冲突；此外，一些团队可能会遇到参与度不足的问题，团队成员不愿意投入必要的时间和精力。
在接下来的内容中，我们将探讨如何克服这些挑战，优化流程，并分享一些实战经验，以帮助读者在自己团队中实施有效的CR。

Code Review的核心目标和基本原则

理解，首先 MCube 会依据模板缓存状态判断是否需要网络获取最新模板，当获取到模板后进行模板加载，加载阶段会将产物转换为视图树的结构，转换完成后将通过表达式引擎解析表达式并取得正确的值，通过事件解析引擎解析用户自定义事件并完成事件的绑定，完成解析赋值以及事件绑定后进行视图的渲染，最终将

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过程中的核心元素。所有的反馈都应该是建设性的，目的是改进代码而不是批评个人。作为评审者应针对代码给出具体、有用的反馈，并在表达时考虑代码作者的感受。

Code Review的实践步骤与技巧

理解，首先 MCube 会依据模板缓存状态判断是否需要网络获取最新模板，当获取到模板后进行模板加载，加载阶段会将产物转换为视图树的结构，转换完成后将通过表达式引擎解析表达式并取得正确的值，通过事件解析引擎解析用户自定义事件并完成事件的绑定，完成解析赋值以及事件绑定后进行视图的渲染，最终将

3.1 实践步骤

CR的实践步骤总体分为三步：准备、评审、修改及完成。

3.1.1 准备

在提交CR之前，应该先自行检查代码，以确保基本的代码质量且遵循代码规范。可以通过单元测试、静态分析插件（例如SonarLint、JD EOS）、借助AI分析插件（例如Copilot、JD JoyCoder）等来完成。
如果更改较大，考虑将其分割成几个小的、逻辑上独立的commit。这样不仅能使每次评审过程更高效，也便于追踪和管理更改。
提交评审的时机，越早进行CR则修改的代价越小，至少应保证在提测前提交CR及完成修改。
最后，确定适合的评审者，建议选择具有业务经验及较为资深的研发人员。

3.1.2 执行评审

在评审过程中，聚焦在代码质量方面（可参考下文提供的checklist）。控制好每次的时长，如果一次评审时间过长，则考虑是否应在准备阶段就拆分成多次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率等。

案例分享

理解，首先 MCube 会依据模板缓存状态判断是否需要网络获取最新模板，当获取到模板后进行模板加载，加载阶段会将产物转换为视图树的结构，转换完成后将通过表达式引擎解析表达式并取得正确的值，通过事件解析引擎解析用户自定义事件并完成事件的绑定，完成解析赋值以及事件绑定后进行视图的渲染，最终将

以下是身边真实发生的一些CR案例，与3.2.1章节中的checklist都有相应的对照，供大家参考。为了便于阅读，部分代码进行了删除简化。

4.1 案例1-异常及并发情况处理不周

问题：静态缓存先clear，再进行加载，如果解析过程异常会导致配置丢失、在高并发访问时读取到错误的配置。

改善：应使用覆盖更新的方式。

public class ReverseSwitch {    private static Map<String, Boolean> multiConfigAddress = new HashMap<>();
    public void setMultiConfigAddress(String multiConfigAddress){        ReverseSwitch.multiConfigAddress.clear();                for () {            ReverseSwitch.multiConfigAddress.put();        }    }
    public static boolean isMultiConfigSwitch() {            }}
public void setMultiConfigAddress(String multiConfigAddress){    log.info("ReverseSwitch.setMultiConfigAddress {}", multiConfigAddress);    Map<String, Boolean> newAddress = new HashMap<>();        for () {        newAddress.put();    }        ReverseSwitch.multiConfigAddress = newAddress; }

4.2 案例2-设计问题、可观测性不足

问题：

1. 本地缓存每小时失效一次，会集中产生大量RPC请求加载数据（容器数量*外部请求数），当依赖的RPC服务抖动时有可能导致雪崩；

2. do while语句在远程数据异常时，可能循环次数超出预期或产生死循环，导致tp99超时、阻塞或OOM；

改善：

1. 使用redis缓存并预加载；

2. while内设置最大分页次数进行break；

3. 上层调用增加监控埋点及日志。

@CacheMethod(key = "vrs.SpareQueryProxyCache.getAllSpareInfo",     cacheBean = "localGuavaCacheBean60m",     timeout = Constants.REDIS_KEY_TIMEOUT_MINUTES_60)public List<BaseStoreInfoDto> getAllSpareInfo() {    int pageNum = 0;    PageDto<List<BaseStoreInfoDto>> page;    List<BaseStoreInfoDto> returnList = new LinkedList<>();    do {        page = basicPrimaryWS.getBaseStoreInfoByPage(++pageNum);        if (page != null && CollectionUtils.isNotEmpty(page.getData())) {                                    returnList.addAll(page.getData());        }    }    while (page != null && page.getCurPage() < page.getTotalPage());    return returnList;}

4.3 案例3-代码复杂度

问题：代码不够内聚，可读性不好，开发追加需求时将多个校验的逻辑写到了校验方法外。
改善：将校验逻辑放到对应的校验方法内，保持代码整洁，降低理解难度。
public void buildWaybillCodeList(AfterSaleOrderReceiveContext afterSaleOrderContext) {    boolean useServiceCode = true;        if (condition_1) {        useServiceCode = false;    }        if (!canUseServiceCode(afterSaleOrderContext)) {        useServiceCode = false;    }        if (condition_2) {        useServiceCode = false;    }    List<String> waybillCodeList = new ArrayList<>();    if (useServiceCode) {                waybillCodeList.add(WAYBILLCODE_PREFIX + afterSaleOrderContext.getAfterSaleOrderReceiveDTO().getServiceCode());    } else {                waybillCodeList.add(this.preDeliveryId(afterSaleOrderContext));    }    }
private boolean canUseServiceCode(AfterSaleOrderReceiveContext afterSaleOrderContext) {    List<ProductDetailDTO> productDetailDTOList = buildMainGiftProductList(afterSaleOrderContext);        return productDetailDTOList.size() == 1 && Objects.equals(productDetailDTOList.get(0).getProductCount(), 1);}
public void buildWaybillCodeList(AfterSaleOrderReceiveContext afterSaleOrderContext) {    List<String> waybillCodeList = new ArrayList<>();        if (canUseServiceCode(afterSaleOrderContext)) {                waybillCodeList.add(WAYBILLCODE_PREFIX + afterSaleOrderContext.getAfterSaleOrderReceiveDTO().getServiceCode());    } else {                waybillCodeList.add(this.preDeliveryId(afterSaleOrderContext));    }    }
private boolean canUseServiceCode(AfterSaleOrderReceiveContext afterSaleOrderContext) {        if (condition_1) {        return false;    }        if (condition_2) {        return false;    }        List<ProductDetailDTO> productDetailDTOList = buildMainGiftProductList(afterSaleOrderContext);        return productDetailDTOList.size() == 1 && Objects.equals(productDetailDTOList.get(0).getProductCount(), 1);}

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

问题：CR过程中发现无法评估改动影响的业务范围，如有问题可能会影响100%的流量。
public void setConsigneeAddress(WaybillAddress targetAddress) {        }
public void setConsigneeAddress(WaybillAddress targetAddress) {        if (ThreadLocalRandom.current().nextInt(1000) < ducc.getAddressMontageSwitchRate()) {                        } else {                    }}

4.5 案例5-善用工具

在提交人工CR前，善用工具进行代码扫描，可能有意想不到的收获。

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());}

除目前流行的基于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());}

Code Review的成果收益

理解，首先 MCube 会依据模板缓存状态判断是否需要网络获取最新模板，当获取到模板后进行模板加载，加载阶段会将产物转换为视图树的结构，转换完成后将通过表达式引擎解析表达式并取得正确的值，通过事件解析引擎解析用户自定义事件并完成事件的绑定，完成解析赋值以及事件绑定后进行视图的渲染，最终将
笔者所在团队没有单独统计数据来佐证CR与线上缺陷的直接关联。线上质量与CR、单元测试、质量测试、SRE等各方面息息相关，CR并非银弹，但是做好CR非常有助于降低缺陷数量。
通过搜索公开数据显示，行业中使用CR的项目，潜在缺陷发现率约在50%~60%之间，大部分的测试，潜在缺陷发现率约在30%左右。同时，数据显示约75%的CR评审意见影响着软件的可维护性/可演化性，这表明CR利于软件系统的长期演化。

总结与展望

理解，首先 MCube 会依据模板缓存状态判断是否需要网络获取最新模板，当获取到模板后进行模板加载，加载阶段会将产物转换为视图树的结构，转换完成后将通过表达式引擎解析表达式并取得正确的值，通过事件解析引擎解析用户自定义事件并完成事件的绑定，完成解析赋值以及事件绑定后进行视图的渲染，最终将
本文探讨了CR的重要性，它可以提前发现缺陷，有助于知识共享及团队能力提升，同时分享了CR实践步骤、技巧、案例等内容。当然，本文仅是一份参考指南，每个团队根据其所处现状的不同，可以根据本文调整优化各自的实践流程。
如今，软件开发的格局在不断变化，围绕CR的实践也在不断发展。随着技术的进步，更智能的工具和 AI 辅助平台在不断涌现，这些工具能够提供更高级的静态分析、模式识别，甚至预测分析，在潜在问题出现之前识别它们。这种AI上下文感知的能力，将能够根据项目特定的编码风格、功能模块以及依赖关系，提供针对性的CR反馈，甚至不再需要人工评审的介入。

未来，CR将继续发挥其关键作用，我们期待AI+CR成为一个更加强大和智能的伙伴，使团队将能够保持竞争力，持续提升软件质量和交付速度。

参考资料

理解，首先 MCube 会依据模板缓存状态判断是否需要网络获取最新模板，当获取到模板后进行模板加载，加载阶段会将产物转换为视图树的结构，转换完成后将通过表达式引擎解析表达式并取得正确的值，通过事件解析引擎解析用户自定义事件并完成事件的绑定，完成解析赋值以及事件绑定后进行视图的渲染，最终将
《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

感谢京东物流平台技术部的同事们对本文撰写提供的帮助

底层能力：维护用户基础数据、行为数据建模、用户画像分析、精准营销策略的制定

▪功能支撑：会员成长体系、等级计算策略、权益体系、营销底层能力支持

▪用户活跃：会员关怀、用户触达、活跃活动、业务线交叉获客、拉新促活

‍‍