干货｜如何做有效的代码走查

点击上方“中兴开发者社区”，关注我们

每天读一篇一线开发者原创好文

▍作者简介

丁一：无线研究院敏捷教练，技术爱好者，喜欢阅读和分享，致力于让敏捷管理实践和技术实践在团队中更好的落地。

代码走查，英文词语叫：Code Review，也叫“代码审查”，它是我们公司的一项传统保留项目。记得一位工作超过20年的老员工说过：“我加入中兴的时候就有代码走查了。”，可见这项实践的悠久历史。

1.代码走查的形式

代码走查的形式有很多种，主要有以下几种形式：

•  每日走查：只针对每日提交的内容进行评审，走查时间和地点都比较灵活。

• 专项走查：针对某个具体问题或者专题进行走查。评审人需要提前发送评审内容给大家进行预审，然后安排专门的会议室进行评审，时间较长。

•  结对互查：提交代码前指定某位同事进行线上评审，评审通过后才能合入代码。

本文要介绍的是每日代码走查，就是大家围在一台开发机周围，逐一轮换讲解所有提交的内容。

就即使是每日代码走查，也被我们团队玩出了花样：

• 谈心式走查

• 批判式走查

• 半蹲式走查

• 伴侣式走查

2.代码走查的好处

持续、有效的开展代码走查，将会收获许多收益，具体表现在：

• 能及时发现代码中的Bug，保证版本质量。

• 提升代码的可读性、可维护性，建立团队共同的编码风格。

• 有利于知识共享，打破技能壁垒，避免单点故障。

• 通过展示自己的优秀代码和设计思路，提升了个人成就感。

• 通过讲解自己的代码，对个人沟通能力也是一种提升。特别是对于平时比较内向或者不太喜欢发言的同学。

• 给大家提供了一个每天交流、沟通的平台。工作一天了，也挺累的了，是时候停下手工的编码工作，一起说说话，交流一下了。

3.代码走查中的“坏味道”

虽然代码走查有这么多好处，可在实施的过程中并不会像想象中的那么美好，会遇到各种各样的问题，总结起来的“坏味道”有：

• 开发的时间本来就不多，再加上代码走查，会打乱开发节奏。

• 评审的同事对代码不熟悉，发现不了问题。

• 讨论发散或者纠缠在某个具体细节中，导致时间把控不好。

• 评审量大。只能走查部分同事的代码，其他同事的内容没有覆盖。

• 提问题的总是那几个人，其他人都是围观群众。

4.如何做有效的代码走查

虽然代码走查很多团队都在做，但要想真正做好它并不是件容易的事情。我们团队经过长期实践，摸索出一些经验和大家分享：

4.1代码走查的时间

代码走查建议在固定时间举行，当团队养成习惯后，就会很自然成为团队日常工作的一部分。以我们团队为例，走查时间安排在：

• 每天下午16:30--17:30；

• 第二天上午9:00--9:30。

分两块时间走查是考虑一次走查的内容会很多，有些内容可能走查不完，就分两次进行。如果当天下午能把全部过完，第二天上午就再进行走查活动。   

另外，在实践的过程中，如果有同事反馈代码正写在“兴头”上或者正在定位紧急故障，可以申请延缓或者不参加本次走查活动。

4.2代码走查的内容

我们团队的走查内容已经超越了代码本身，还会涉及方案审查、演示等活动。

具体评审内容为；

• 新增代码：bug修改，新特性或重构引入的代码变更。

• 设计方案：一般为增量式方案评审。

• Mini showcase：针对每日实现的功能进行展示。

4.3代码走查的关注点

l 静态编码问题

主要关注静态编码错误、编程风格、命名合理性、Clean Code等内容。

虽然这部分检查项也有有意义，但完全可以在合入代码时，通过静态检查工具（比如Findbugs，PMD等）消除掉。还是要尽量留给团队成员走查业务或者设计等问题。

l 功能问题

代码的行为是否与预期一致，其逻辑是否是正确无误？

l 设计问题

针对现有的设计提出不同的思路，多问问为什么这么做，有没有更有效的方法，这样通过集思广益可以提供更加优良的设计方法。

l 测试问题

v 测试用例是否完备?

v 测试用例实现是否有效？

l 性能问题

新增功能或者修改功能是否对已有性能测试结果有不良影响？

l 安全问题

v 开源软件协议风险

v 是否有安全漏洞

4.4角色

代码走查中的角色需要从不同的维度考虑。

l 从业务角色考虑：

业务角色尽量完备：至少包括BA、Dev、QA等角色。特别是BA，无论再忙，还是要参加到团队的走查活动中，从设计层面上保证实现没有走偏，也能从开发人员那里获取最新的问题反馈，从而及时对方案进行修改。

切记：代码走查不只是开发人员的事情！需要多种角色同时参与，因为走查活动不仅仅要看功能是否实现了，还要看代码和设计是否一致？测试用例是否完备和有效？

l 从走查活动的角色考虑：

  一般包括如下角色：

角色	职责
主持人	负责主持整个走查活动，控制时间和进度。
讲解人	负责对走查的代码进行讲解。
评审人	负责对走查代码提出问题
记录人	负责对发现的问题进行记录
观众	参与但不发表意见

这里要提的一个关键角色是主持人，

我们团队之前走查代码是没有主持人的，走查时经常出现讨论发散、时间控制不好的情况。

在回顾会上，大家针对这个问题做了讨论，最后选举了一位值得信赖的女主持人。

有了主持人之后，情况果然有了很大改善。一旦发现讨论发散、时间过长的情况，主持人会及时喊停，把大家拉回到正确的节奏上。

主持人要求有敏锐的洞察力，良好的大局观，并且有一定的影响力，最好由团队选举产生，可以是SM，也可以不是。

(找找女主持人在哪里？)

4.5走查过程

4.5.1走查前

• 每个人介绍下要今天走查的内容和预计时间，便于主持人做好规划。

• 优先走查自己认为风险较大的地方。

4.5.2走查中

l 讲解人先介绍业务背景和代码关键流程，

避免一上来就讲代码，这种方式跳跃性比较强。只有对这部分代码非常熟悉的同事才能发现问题，而那些第一次接触的同事很难做到这一点，于是很快就会失去走查的兴趣。     

l 一次走查的代码尽量少。

走查的代码行数控制在200--400行。

在审查大的修改时，不仅要看很多行代码，还要查看大量的依赖代码才能理解。

将待审查的代码隔离为小的修改可以降低审查者的精神负担并让审查过程更加顺畅。

l 代码走查一页纸规范

很多团队都制定了代码走查一页纸规范，比如资源使用完要释放，多线程并发问题等。有了走查清单后，便于团队快速识别问题，提高走查效率。

l 记录发现的问题

对于代码中发现的问题，可以由记录员记录到文档中。而我们团队的做法是让讲解人直接在代码中以to-do的方式进行标记。

 

4.5.3走查后

对于走查中记录的问题或者代码中标示为“todo”的条目进行修改，然后在下次走查时检查是否修改完成。

4.6沟通

在代码走查时，有时会看到为了争一个问题，双方搞的面红耳赤。在我们强调良性对抗的同时，也要尽量避免由于沟通问题导致的不必要冲突。

前面也提到过，代码走查问题的本质是一个沟通问题。所以这里要提一下代码走查中的沟通技巧：

l 对于评审人

1.避免出现“你为什么”，“你为什么不”这样的问题。

它会使人对立，“为什么把它声明为全局变量？”可以更好地表达为“我不明白为什么这里用一个全局变量”。

2.不要提出可能听起来带指责的要求或言论。

例如:“你没有遵循标准XYZ”，更好的方式是：“你对标准 XYZ 有什么看法，它是否适用于这里？”。

3.不要吝啬自己的赞扬。

当发现有好的代码实现或者设计思路时，请及时给与表扬。

l 对于讲解人

保持好心态，别人是针对代码提问题，而不是针对人。