说说代码评审

 关注公众号【1024个为什么】，及时接收最新推送文章！ 

本文内容： 1、代码评审的目的 2、评审准备工作 3、评审过程中容易出问题的点 4、共同成长

近一段时间以来，组内严格实行代码评审制度。参与过多次评审后发现，一次有效的代码评审并不简单，把一些思考做一下总结。

||  代码评审的目的

| 提前发现问题

问题发现越早，解决成本越小，带来的损失也越小。

| 保证代码库的健康状况

随着时间推移，代码库的健康状况往往会下降。通过代码评审，如果能使代码库整体健康状况不快速下降，就算达到目的了。

||  评审准备工作

| 评审规范

团队最好要有一套评审规范，如果没有，可以在一段时间的评审后沉淀一套规范。

有了规范，评审就可以按规范一条条过，节省时间。

有了规范，产生意见分歧时，依据规范，化解分歧。

| 评审时间节点

很多观点是每次提交代码前都要评审，但实际工作中并不可行。

自测后期就可以发起评审，最晚到测试前期。如果有问题，还有充足的时间修改、测试。

新的模块可以分阶段多次评审，优先评审核心代码，及时发现并纠正问题，避免后期大量返工。

评审不通过有大改动的，改动后要再次评审。

| 评审形式

有条件的一定要面对面评审，效率高。下文也都是基于此形式的讨论。

跨地域的可以邮件评审或者评审工具评审。

| 发起人要做什么

邀请评审人（必须包含熟悉此业务的人），提前发会议邀请，协调评审人的时间。

需求背景提前同步出来，便于评审人提前了解，节省评审时间。

准备好CL（Change List），要了然于心，确定一个合理的讲述方案。建议零星改动按列表讲，其他最好按业务流程讲。

多说一句：CL 不仅仅是改动的代码，还可能包括DB脚本、配置信息、单测、数据现状、调研方案及结论...

| 评审人要做什么

参加评审，就要对其负责（假设出现事故有问责制），做好前期必要的准备工作。时间允许的话，提前了解需求，熟悉此部分业务，预判可能出现问题的节点，重点关注哪些部分，带着问题去评审。

| 评审工具

IDE集成的对比工具即可。使用工具有很多好处：

1）可以一条一条的点，不会漏代码。千万不要滚鼠标，很容易漏；

2）有前后对比，使评审人更直观的看到原来代码什么样，你这次改成什么样；

3）可以只看改动代码，节省时间。直接看代码很容易扯到没改动的代码去。

||  评审过程中容易出问题的点

评审时，千万不要上来就讲改了哪些代码... 这只会使评审效果大打折扣。

发起人要先花上几分钟，快速讲一下本次的需求背景，原来什么样，这次什么样，要改哪些模块。让评审人快速准确的了解本次业务范围，以便接下来能够给出合理的评审意见。

| 粗心类错误

这类错误和技术水平没有太大关系。

比如 if 条件里的取反；

配置是否漏配了环境，是否拷贝后忘了修改环境相关的信息；

数据库字段长度够不够用（一般都是扩展字段容易超）；

测试结束后改动的代码忘提交了；

这些可以通过和同桌交叉检查的方法来避免。

| 方案过度设计

1）过度考虑未来业务

设计满足当前需求，以及合理业务扩展即可。

经验表明，之前很多当时认为极具前瞻性的设计，后面多数都是没用的，或者用到了，也要做很大改动。

2）过度考虑极端场景

比如某个后台人工操作的场景（一个月也不一定操作一次，正好又赶上服务挂了），要不要失败系统自动重试？

类似这种通过简单的人工补救方案（业务重试、重新发起）可以达到目的，就没必要设计复杂的技术补救方案，因为每引入一个方案又会带来新的问题。

3）过度的错误兼容逻辑

为了追求数据的正确性、一致性，可能会出现各种异常数据校验、各种异常处理逻辑，层层自我重试。

很多情况下，完全可以采取 “约定大于开发” 的准则，就能减少很多校验，很多重试的处理。比如约定入参格式、返回结果，约定由谁触发重试等等。

各个行业都有约定，比如 USB、网线的排线顺序，我们不可能在提供USB口、网线口的设备上考虑各种错误线序的兼容方案、错误反馈。

| 前后业务流程有逻辑漏洞

这种问题不好发现，需要对业务足够熟悉，流程足够清楚。

可能单看 A、B 两个接口都没有问题，但 A、B 在前后流程上有依赖关系。整个流程一串起来问题就暴露出来了。

| 事务

日志类的操作不要回滚，要留痕。

事务回滚是否影响重试的请求？ 

| 冗余的数据库访问、服务调用

同一流程中前后2个方法都查询了同一数据。

循环逻辑内查库、调服务。

（假设每次访问都是要收费的，是不是就能省则省了）

| 定时任务

是否幂等？

是否能重复执行？

是否能暂停？

是否能够异常继续？

是否有超时风险？

是否超过下游承载能力？

禁止limit m,n 分页。

| 影响范围有遗漏

梳理影响系统的方案是否科学。

服务治理统计出的是否全面，有没有直接调用的？

从日志、端口统计，时间范围是否够广？

一些边缘系统、定时任务是否会遗漏？

||  共同成长

| 心态要正

代码评审不是挑毛病，能有效实现功能无严重问题即可。可以给出自己的建议，但允许每个人有自己的风格。

对事不对人，发起人和评审人都要有这个意识，大家的共同目的是一致的。

| 沉淀

评审发现的问题要留心记一下，避免重复采坑。

经过一个阶段的评审之后，很多东西是可以直接拿来复用的。

| 点赞学习

好东西要点赞、相互借鉴学习（好的技术方案，好的编码习惯，或者好用的工具类），共同提高编码水平。

扯两句

评审一次费时费力，尽量让其效果最大化

一个合格的程序员绝不仅仅是写好代码这么简单

搜寻资料的过程发现了谷歌的代码评审规范，可以参考学习。
官方：https://google.github.io/eng-practices/review

中文：https://jimmysong.io/eng-practices