Code Review:提升代码质量与团队能力的利器

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

尽管CR有诸多好处,实际操作中却面临不少挑战。例如,交付压力可能导致CR被忽视或流于形式;另一方面,缺乏有效技巧和工具支持,可能会使CR变得低效,甚至引发团队内的冲突;此外,一些团队可能会遇到参与度不足的问题,团队成员不愿意投入必要的时间和精力。

在接下来的内容中,我们将探讨如何克服这些挑战,优化流程,并分享一些实战经验,以帮助读者在自己团队中实施有效的CR。



在此特别感谢JDL平台技术部王鑫、刘建设、刘风、杨宏强、鞠万奎等对本文撰写的帮助。



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



  1. Code Review的实践步骤与技巧
    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率等。



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

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

相关文章
|
19天前
|
算法
如何做代码Review?
10月更文挑战第3天
38 2
|
6月前
|
算法 Java 测试技术
卓越工程之如何做好Code Review
本文主要从我们为什么需要CR?CR面临哪些挑战?CR的最佳实践几个方面分析,希望可以给读者一些参考。
|
安全 UED
【代码review、审核开发工程师的设计与研发质量工作经验之谈】
【代码review、审核开发工程师的设计与研发质量工作经验之谈】
|
SQL 缓存 架构师
一文梳理 Code Review 方法论与实践总结
作为卓越工程文化的一部分,Code Review 其实一直在进行中,只是各团队根据自身情况张驰有度,松紧可能也不一,这里简单梳理一下 CR 的方法和团队实践。
630 12
一文梳理 Code Review 方法论与实践总结
|
SQL 缓存 架构师
一文梳理Code Review方法论与实践总结
相信Code Review每天都在进行,但代码评审应该怎么做,评审范围有哪些,评审标准在哪里,怎样可以避免流于形式,这里结合自己的实践来聊一聊。
29403 10
一文梳理Code Review方法论与实践总结
|
JavaScript 前端开发 安全
【软件工程】代码质量综合指南:最佳实践和工具
【软件工程】代码质量综合指南:最佳实践和工具
|
前端开发 JavaScript IDE
让团队代码像一个人写的
前端代码规范流程实践思路 1. 本地开发过程,提示、校验、更改 2. Git 提交过程,代码校验是否允许提交 3. 服务端校验,代码校验是否合并和发布
150 0
让团队代码像一个人写的
|
测试技术 开发者
|
前端开发 Web App开发 JavaScript