7 个建议让 Code Review 高效又高质

2023-11-03

简介:Code Review(CR) 的本质是什么?是为了查错?还是为了 KPI?本文分享阿里资深技术专家的看法:CR 是一种关于社会学的长期行为和组织文化,通过 CR,形成一种良性互动的技术氛围,传播和分享知识,提升代码质量,并给出了 7 个提高 CR 效率和质量的实践建议。

image.png
关于代码评审(Code Review)的文章也算是汗牛充栋了,代码评审也已经是许多组织的标准化实践。不过,许多团队在尝试代码评审实践时,却有如下疑问:

  • “政治正确” 的代码评审活动究竟有没有达到期望的实际效果?
  • 给了我一大堆代码,到底该从哪里看起?哪些方面是我该评审的?哪些不是?
  • 别人有没有认真评审我的代码?如何让别人更容易的评审代码?

这些都不是什么新问题,但是它们是如此的普遍,而且经年累月地在不同的上下文中被提起,不外乎两个方面:

  • 理解代码评审的核心目标,建立关于代码评审的正确预期。
  • 了解代码评审为什么可能无效,并采取有针对性的实践来提升代码评审的效果。

为什么要做代码评审

不少同学认为代码评审就是用来查错的,甚至希望用代码的缺陷数量来检验代码评审的效果。这低估了代码评审的价值。代码评审最本质的作用不是问题发现。除了代码评审,我们有更多更好的手段来发现问题。代码评审的作用更多是关于社会学的,是一种长期行为和组织文化。

CR 是代码规范性的保证

编码者视角:良性的社交压力

你正在紧张地编码,交付时间迫在眉睫。你的组织对代码的单元测试有一个要求:凡是新增的代码,必须有完整的自动化单元测试。但是,在压力之下,你想给自己降低一点要求,不写这部分的单元测试了,以后再编写吧,或者为了应付工具的覆盖率要求,先写一点不那么有用但是却能带来覆盖率的测试(例如没有断言的测试)。

但是,一旦想到你的代码将会发出去给你的同事做 Review,有没有为刚才的这种想法产生一丝丝压力?这种压力是良性的,它能给你带来一种即时的反馈,阻止你去选择那些短期收益、长期损失的 “投机” 行为。如果没有代码评审这个环节,或许你就会真的 “为所欲为” 了,其实最终还是要为这种取巧行为买单。

维护者视角:代码可读性的保证

有许多方式能实现同一个软件需求。有兴趣的读者可自行搜索 “Hello World 的 N 种写法”。尽管条条大路通罗马,但是,不同的道路代价是不一样的。小到变量命名,大到设计结构,如果你采用的是一种不那么常见的做法,往往就是给后来的代码维护者挖坑。这种维护活动可能发生在 1 个月以后,也可能发生在 1 年以后,甚至是更久之后。甚至那时候,作为作者的你,已经不在这个团队了,已经没有人能理解当时的软件为什么这样设计。

代码评审强制提前了这个反馈周期,代码编写完成之后,就立即有了一位或多位读者——他们是这个代码的 Reviewer。所以,这段代码在编码完成之后,立即经历了可读性的检验。更理想地,如果组织已经有了编码规范和设计规范,还能确保这段代码遵循了这些规范。如果 CR 过程中发现这段代码没有遵循规范,那更是好事,它指向了 CR 的另外一个关键价值:知识传播。

CR 带来了知识传播和设计共识

你可能是一个团队的 Leader,正在为如何提升团队成员的编程能力发愁。你希望他们去读书,所以你介绍了诸如《整洁代码》之类的入门书籍,你还介绍了经典名著《设计模式》,还推荐了《领域驱动设计》。你也希望团队成员能理解产品的业务逻辑,所以希望团队成员周期性地进行业务分享。

所有这些努力都很好。但是,也有可能你会被打击。一个月过去了,似乎团队成员对命名规范都建立了概念,但是在怎么命名这件事上,大家并未形成共识。不少同学已经了解了一些设计模式,但是有人过度运用模式,搞得代码臃肿不堪,有人则只知道singleton。团队成员为什么是实体对象,什么是值对象争的不可开交,没有人说得清楚聚合是什么,应该在什么场景下使用。

你真正缺乏的,是一个场景。抽象的概念如果不落到具体的事情上,就很难形成共识。有人或许知道海洋法系的 “判例”,这是在法律层面形成共识的一种非常好的方法。代码评审,其实也是在形成判例:哪一类设计是合理的,哪一类设计是不合理的。通过针对具体的问题进行分析,团队就会逐渐形成设计共识,在过程中,对这些共识不那么熟悉的新同学,也可以慢慢融入。

检验逻辑正确性

当然,CR 也能用来检验逻辑正确性。

保证代码逻辑正确,是设计者的责任

为了不让 CR 被滥用并被寄予过高期望,我们在此首先申明一点:保证代码的逻辑正确,是设计者的责任。出现了一个空指针错误,究竟是编码做的不好,是 CR 做的不好,还是测试做的不好?首先肯定是代码作者制造了这个问题。把这个板子打在 Reviewer 身上公平吗?或许,Reviewer 确实有责任发现这样的问题,但是,如果代码本来就错误多多呢?如果一次性 Review 了 1000 行代码,根本看不过来呢?我能找到一大把的理由,来说明为什么漏掉这么一个空指针错误。

发现逻辑错误的其他方法

你还有许多其他的方法来发现错误,它们的成本往往并不高,例如:

  • 编写自动化单元测试
  • 使用代码静态检查工具

无论是否存在 CR 活动,上述两点都是一名专业的开发者和开发组织应该大力倡导的行为。

发现错误的价值

在上述两点得到承认的前提下,代码评审确实也应该用于发现错误——它本质上建立了一种冗余机制,通过多人工作在同一段代码上,发现代码中可能发生的认知错误(这对于单个开发者往往是很难发现的)以及疏忽。

高效高质的代码评审

哪些因素阻碍了代码评审的效果

代码评审本身并不困难,但是,如果考虑到如下因素,可能就比较复杂了:

  • 你可能对要评审的代码的设计上下文一无所知
  • 你可能非常忙碌
  • 你一下子收到了几千行需要被评审的代码
  • ...

实际操作建议

建议一:小批量——每次 Review 的代码量要少

研究发现, 成功的 CR 活动一定是小规模的。例如,《Modern Code Review:A Case at Google》论文介绍说, Google 的 CR 活动中,有 35% 的 CR 仅仅修改了一个文件,90% 的 CR 修改的文件数在 10 个文件以内,甚至有 10% 的 CR 仅仅修改了1行代码。

代码量少的好处显而易见:修改了哪里非常清晰,问题也会一目了然。一次推给别人 1000+ 行代码,还想得到有价值的 Review,可能性微乎其微。

当然,一次 Review 它代表的功能应该是有意义的,是完整的,如果不是修复缺陷,这一定程度上也对开发者迭代地开发功能的能力提出了要求。

建议二:多批次——Review 要频繁发生

小批量必然导致了多批次。在微软 2013 年的一篇论文《iExpectations, Outcomes, and Challenges Of Modern Code Review》和前述的 Google 的论文中都提到了频繁 Review 的做法。其中,Google 的每周每 Developer 的代码变更中位数是 3 个,每周每 Reviewer 的 Review 中位数是 4 个。

建议三:找对人——合适的 Reviewer

谁适合 Review 你的代码?选一个和被 Review 的代码毫不相干的人肯定是不明智的。下面列出了一些潜在的候选人:

如果你的组织有 Owner 机制,Owner 应该是合适人选

和你工作在相同上下文的同事

近期修改过相同代码的同事

比你更资深的程序员,希望得到他们的专业反馈

现在已经有一些工具,能够根据上下文推荐 Reviewer,这也为选择合适的 Reviewer 提供了便利。

建议四:快速响应

当每次 Review 的粒度不大,Review 又比较频繁时,快速响应才能成为可能,也是必然的要求。在这个数据上,Google 的中位数是 4 小时。这个指标可以成为一个较好的参照。

建议五:使用现代工具

快速响应、高质量的 Review 离不开现代工具。现代的 Review 工具能自动集成进工作流,高亮变化,甚至能自动汇总变更。这方面已经有许多现代的工具可以使用,还没有选好工具的读者,可以自行搜索。

建议六:考虑结对编程

当我们提到 “小批量、多批次、快速反馈” 的时候,如果有过结对编程经验的同学,马上就会反映过来,结对编程和 CR 何其相似。

结对编程,共同编程的两位同学拥有完全相同的上下文,不存在上下文切换的烦恼,没有缺乏时间的烦恼,不需要借助额外的工具,反馈随时随地。事实上,在我的眼中,结对编程才是最好的 Code Review。

建议七:综合在线 Review 和线下 Review

在线 Review 应该是常态化的行为。考虑到 CR 的 “知识传播” 价值,线下 Review 是有益的补充。有经验的团队,会周期或者不定期的组织线下 Review,这样能获得比在线 Review 更为广泛的知识传播面,也能引起更为热烈的讨论和辩论,有助于形成更高质量的共识。

数字化的指标

研发行为的全面数字化,带来了一些有价值的数据洞察。如果工具支持,可以通过一些指标的观测,持续推进 CR 活动。我们把一些建议的指标和数据总结如下:

从 Author 角度:

  • 单次评审的代码行数 (主要指标)
  • 评审的频度 (参考)

从 Reviewer 角度:

  • 响应时间
  • 评论数和拒绝率

从 Reviewer 的团体角度,还可以发现 Author-Reviewer 之间的社群关系,也是一种有价值地了解知识传播的信息。

不要做什么:

避免用 CR 的数字化指标进行考核,即使动机良好。CR 的本质是文化建设,强烈建议仅仅把 CR 的指标用作提升指引,而不要用于和绩效有关的评价,无论是前述的几种指标,还是和 Review 的质量甚至是缺陷相关的数据。

实践

你的组织的代码质量和技术氛围如何?你们正在实施代码评审吗?

  • 如果暂时还没有,是否愿意做一些尝试?
  • 如果已经在实施,既有的实践是否达到了代码评审的真正目标?

希望本文的一些实践建议能带给你一些有益的帮助。如果你关于代码评审有更好的建议,也期待着你的留言分享。

原文链接:https://developer.aliyun.com/article/765485?

版权声明:本文中所有内容均属于阿里云开发者社区所有,任何媒体、网站或个人未经阿里云开发者社区协议授权不得转载、链接、转贴或以其他方式复制发布/发表。申请授权请邮件developerteam@list.alibaba-inc.com,已获得阿里云开发者社区协议授权的媒体、网站,在转载使用时必须注明"稿件来源:阿里云开发者社区,原文作者姓名",违者本社区将依法追究责任。 如果您发现本社区中有涉嫌抄袭的内容,欢迎发送邮件至:developer2020@service.aliyun.com 进行举报,并提供相关证据,一经查实,本社区将立刻删除涉嫌侵权内容。
本文内容由网友自发贡献,版权归原作者所有,本站不承担相应法律责任。如您发现有涉嫌抄袭侵权的内容,请联系:hwhale#tublm.com(使用前将#替换为@)

7 个建议让 Code Review 高效又高质 的相关文章

  • 红外人体感应传感器SR602模块使用说明

    一 HC SR602模块 红外人体感应传感器HC SR602是基于红外线技术的自动控制模块 专用于感应周围人体的存在 该模块相较于HC SR501 灵敏度较高 抗干扰能力大 且简单易用 二 HC SR602模块主要参数 工作电压 3 3V

随机推荐

  • 使用手柄控制Unity及效果展示(1)

    Unity支持手柄的控制 效果图如下所示 这是一篇针对手柄控制U3D入门的过程记载 主要以实现功能为目的 分四个部分进行过程展示 Input System包的下载 设备的查找 Input Actions控件的使用 主要代码的解释及编写 这里
  • 【css】落叶飘动效果,点击落叶飘动停止

    做项目有个需求是要求做落叶下落效果 做了一下 整体是依靠css中animation属性来控制的 keyframes move控制动画轨迹 使用伪元素checked控制动画运行和暂停 css3文档中有这样的例子 利用伪元素checked控制动
  • python 删除一个字符串内重复的内容

    使用py将 a dai liu dai 去掉重复的dai a dai liu dai a list a split 将字符串拆分成列表 a set set a list 将列表转换为集合 去除重复的元素 result join a set
  • jest搭建vue项目单元测试-vue-cli创建新项目

    说到项目会分为新建的醒目和老项目两种 我们先来说新项目 新项目加入jest单元测试 1 创建项目 使用vue脚手架创建项目 test vue jest vue create test vue jest 2 创建项目过程中配置选项 在配置项中
  • OK彻底解决ping主机ping虚拟机之间ping不通的问题

    时间轴 一个月前 2022 8 重新玩虚拟机 因为计算机网络这块不扎实 知识点模糊 不懂其中各种原由 当然现在也不是很明白 后续还需要系统的回顾 在那是一直有几个问题 遇到以下问题需要解决 1 怎么选择桥接 nat的连接方式 现在也不清楚
  • 第七章 单行函数

    MySQL系列文章目录 http t csdn cn YTPe9 文章目录 MySQL系列文章目录 前言 一 函数的理解 1 什么是函数 2 不同DBMS函数的差异 3 MySQL的内置函数及分类 二 数值函数 1 基本函数 2 角度与弧度
  • C 语言char类型与int类型的转化

    目录 一 char转int 法一 直接转换 ASSCII编码表 ASCII可显示字符 法二 利用库函数转换 二 数字换成字符串 1 用sprintf 2 用库函数 char和int的转换有两种方式 这两种方式适合于在输出时使用 一 char
  • Android studio实现个人体重指数计算

    Java代码 package com example work1 import android app Activity import android app AlertDialog import android content Dialo
  • 基于 Android NDK 的学习之旅----- C调用Java(附源码)

    许多成熟的C引擎要移植到Android 平台上使用 一般都会 提供 一些接口 让Android sdk 和 jdk 实现 下文将会介绍 C 如何 通过 JNI 层调用 Java 的静态和非静态方法 1 主要流程 1 新建一个测试类TestP
  • linux执行命令缺少共享库解决方法和ldd命令说明

    linux执行命令缺少共享库解决方法和ldd命令说明 root xiaogaokui which ldd usr bin ldd root xiaogaokui file usr bin ldd usr bin ldd Bourne Aga
  • ZStack-K8s三层互联互通方案

    1 准备条件 zstack社区版 kubernetes 1 15 0 zstack 创建私有网络 kubernetes 选择Calico网络方案 kubernetes创建在zstack创建的扁平网络中或者是kubernetes和zstack
  • web H5 调用高德地图 通过ip定位获取当前城市

    web H5 调用高德地图 通过ip定位获取当前城市 一 使用步骤注册高德账号 创建应该获取key 登录之后 点击 应用 头部导航栏 注册地址拿走 https lbs amap com dev id choose 注意这里有服务类型 提交完
  • 拉格朗日法插补数据缺失值(Python代码实现)

    1 查询缺失值位置 isnull for i in df columns for j in df index if df isnull loc j i isnull append j i isnull 2 拉格朗日插值 传入存在缺失值的列
  • PCL 三维点云边界提取(C++详细过程版)

    边界提取 一 概述 二 代码实现 三 结果展示 本文由CSDN点云侠原创 爬虫自重 如果你不是在点云侠的博客中看到该文章 那么此处便是不要脸的爬虫 一 概述 点云边界提取在PCL里有现成的调用函数 具体算法原理和实现代码见 PCL 点云边界
  • r语言插补法_R语言缺失值的处理:线性回归模型插补

    在当我们缺少值时 系统会告诉我用 1代替 然后添加一个指示符 该变量等于 1 这样就可以不删除变量或观测值 我们在这里模拟数据 然后根据模型生成数据 未定义将转换为NA 一般建议是将缺失值替换为 1 然后拟合未定义的模型 默认情况下 R的策
  • Wireshark网络封包分析软件使用心得

    Wireshark网络封包分析软件使用心得 Wireshark 前称Ethereal 是一个网络封包分析软件 网络封包分析软件的功能是截取网络封包 并尽可能显示出最为详细的网络封包资料 Wireshark使用WinPCAP作为接口 直接与网
  • 资源池 'default' 没有足够的系统内存来运行此查询

    今天 有个客户突然打电话跟我说他的系统出了点问题 一个查询查了几十秒才出来 有时候还出不来 于是我上去看了一下 打开数据库看了相关视图 发现查询确实很慢 运行多次之后出现 资源池 default 没有足够的系统内存来运行此查询 嗯 这句话的
  • selenium抓取元素排除某个特定的class标签

    排除某个因素 第一优选想到正则表达式 无奈折腾半天没有成功 感觉是对元素的attrs按search在操作 对字符串末尾检测都没什么用 语法如下 text match By XPATH tr 5 td 11 div r 0 1 1 0 9 6
  • 【问题及解决】ImportError: libpython3.7m.so.1.0: cannot open shared object file: No such file or directory

    报错 是说少了一个libpython3 7m so 1 0的库 解决参考 https github com deepmind acme issues 47 解决办法 export LD LIBRARY PATH path to libpyt
  • 7 个建议让 Code Review 高效又高质

    简介 Code Review CR 的本质是什么 是为了查错 还是为了 KPI 本文分享阿里资深技术专家的看法 CR 是一种关于社会学的长期行为和组织文化 通过 CR 形成一种良性互动的技术氛围 传播和分享知识 提升代码质量 并给出了 7