下载掌阅APP，畅读海量书库
立即打开
前面几节介绍了编码规范、单元测试、TDD等，同时介绍了一种使用静态代码分析工具进行静态代码扫描，以帮助我们更好地发现代码缺陷的方法。对于在上线前发现代码缺陷，还有一种十分重要及奏效的方法，那就是代码审查（Code Review）。本节详细介绍什么是代码审查、代码审查的好处及如何做好代码审查。
代码审查，指对计算机源代码进行系统审查，以找出并修正在软件开发初期未发现的错误，提升软件的质量。代码审查可以帮助开发者了解自己的代码缺陷，从而提醒开发者避免类似问题的发生。相较于静态代码扫描，代码审查主要借助开发者之间的相互阅读及代码审查，来发现代码中的Bug等问题。
代码审查的重要性是被普遍认可的，因为通过代码审查可以提升代码的质量，找出潜在的Bug，保证代码风格的统一，等等。但是，笔者认为，其最大好处往往被人忽略，即它是一个可以相互学习的好机会，比如审查者（Reviewer）可以在代码中发现他人的优秀编码习惯、对优秀设计模式的使用及使用了哪些好的API等，对了解整个系统和业务也有很好的帮助；另外，通过他人提出的一些改进意见，被审查者可以很好地学习到一些思考方式，在开发过程中有时“当局者迷”，审查者则可以从旁观者的角度重新审查代码及代码背后的设计思想。
所以，关于代码审查，笔者认为，其好处不仅仅是代码审查自身，还是在代码审查过程中产生的思考及思考之间的碰撞。
很多人都知道代码审查的重要性，也在项目中严格地进行代码审查，但是代码审查的效果并不明显。其实，这主要是因为进行代码审查的方法不对。接下来，笔者结合自己的经验介绍一下如何做好代码审查。
1.要有审查清单
代码审查这件事其实是可以标准化和流程化的，其效果在一定程度上也是可以量化的。要想做好代码审查，尤其是对一个开发团队来说，有一份审查清单是非常有必要的，这样大家就都有了一个标准，可以在代码审查过程中按照审查清单逐一检查。使用审查清单可以帮助审查者快速找到问题，甚至开发者在开发阶段就可以按照审查清单进行代码自查。
网上有很多代码审查清单，但是不同的团队可能需要不同的代码审查清单，这需要根据团队的实际情况进行调整和完善。
一般来说，一份代码审查清单应该包括如下几个大的类目。
◎ 代码结构：是否包含超长代码，代码层次嵌套是否过深，函数是否入参过多，循环条件是否有跳出点，if 语句是否有对应的 else 语句，是否存在重复的代码，等等。
◎ 代码安全性：I/O 流是否正常关闭，资金计算是否使用了 Double 数据类型，是否有超大的临时对象，线程池大小是否合理，异常是否被忽略，是否有详细的日志记录，是否存在并发问题，参数是否做了必要的检查，远程服务的入参出参是否实现了Serialization并且自定义了serialVersionUID，应用是否依赖了SNAPSHOT版本的类库，等等。
◎ 代码性能：是否有长SQL语句、SQL语句是否用到索引，是否有成熟的类库可以替换自己实现的代码，是否可以考虑单例模式，是否可以考虑线程池，是否可以考虑NIO，是否可以进行锁优化，等等。
◎ 代码注释：指类及方法是否有注释，注释是否可以表达其准确含义，在代码中是否存在 FIXME 及 TODO 等注释，注释是否包含边界值及对异常情况的说明，等等。
◎ 单元测试：代码是否有可测试性，新代码是否有单元测试，单元测试是否可以覆盖所有场景，等等。
◎ 代码优化：是否可以使用枚举代替自定义的常量，在代码中是否包含魔法值，是否可以使用Optional代替NPE的检查，是否可以使用Stream代替for循环，是否可以使用设计模式，等等。
◎ 其他：代码逻辑是否正确，是否实现了业务功能，代码是否有好的可读性及可测试性，等等。
2.选择高效的工具
工欲善其事，必先利其器，选择一款高效的工具，可以大大提升代码审查的效率。这里推荐几款代码审查工具，如下所述。
1)Phabricator
Phabricator最早是Facebook的内部审查工具，目前已经开源，可以在Apache许可证的第 2 版下作为自由软件分发。它是一套基于 We b 的软件开发协作工具，包括代码审查工具Differential、资源库浏览器Diffusion、变更监测工具Herald、Bug跟踪工具Maniphest和维基工具Phriction。Phabricator可与Git、Mercurial、Subversion集成使用，如图1.6所示。
图1.6
2)Gerrit
和Phabricator一样，Gerrit也出身名门。Gerrit是Google为Android系统研发量身定制的一套免费的开源代码审查系统，它与 Git集成并建立在 Git版本控制系统之上，允许在写完代码后将更改合并到Git存储库。它在传统的源码管理协作流程中强制引入了代码审查机制，通过人工代码审查和自动化代码验证过程，将不符合要求的代码屏蔽在代码库之外，确保对核心代码有多人校验、多人互备和自动化构建核验。
3)Code Striker
Code Striker是基于Perl语言的工具，和其他工具一样，需要安装在服务器上。它支持对CVS、Subversion、Clearcase、Perforce、Visual SourceSafe and Bugzilla等工具的集成。
3.重点关注的改动点
通过使用代码审查工具，我们可以很清楚地看出代码有哪些改动点（包括新增的代码），通过对照我们事先置顶的审查清单，重点关注改动点是一种比较高效的代码审查方式。原有的代码一般是经过代码审查的，但代码的变化往往会引入新的问题，所以，在审查代码的过程中应该重点关注代码的改动点。
但是，重点关注改动点并不是只关注改动点，其实，还有很多问题隐藏在非改动点处。这就需要负责代码审查的开发者通读改动点附近的全部代码，重点关注改动点，而不是只看改动点处的代码。通过通读全部代码，可以更好地发现问题，尤其是和业务逻辑相关的问题。
4.代码审查应该是日常性的工作
代码审查应该是日常性的工作，而不是代码上线前的集中性工作。换句话说，代码审查应该是伴随着代码提交的，而不是伴随着代码发布的。最好的实践是在每次提交改动较大的代码后都找到对应的开发人员进行代码审查，这样可以更早地发现问题，也可以使其他人更早地了解这部分新代码。如果等到项目即将发布才集中进行代码审查，就会有很多问题，这时会有很多代码需要审查，导致审查人员过于疲劳，无法集中精力审核代码，并忽略一些问题。而且，在项目即将发布时进行代码审查，即使在代码审查中发现一些问题，也不便于修改。
5.不要一次审查太多代码
2006年5月，Smart Bear针对Cisco进行了为期10个月的代码审查方面的研究，最终得到一份思科代码审查报告（ Code Review at Cisco Systems ，参见 http：//support.smartbear.com/support/media/resources/cc/book/code-review-cisco-case-study.pdf）。在该报告中指出，做代码审查，每次审查的代码行数最好在200行以内，不超过400行，否则查找代码缺陷的效果就会大打折扣。
图1.7展示了一次审查的代码行数和可发现的代码缺陷之间的关系。我们发现，当代码行数超过200时，缺陷密度（每千行代码的缺陷数，为缺陷数量与代码行或功能点数量的比值，其测量单位是defects/KLOC）就会急剧减小，超过400后，缺陷密度几乎为0。
图1.7
所以，每次代码审查的代码最好在200行左右，最多不要超过400行，这样更有利于代码审查者集中精力，最好每天都抽出固定的一段时间，来集中精力对少部分代码做代码审查。
6.进行一次代码审查的时间不要太长
这里建议一次不要审查太多代码，进行一次代码审查的时间也不宜过长。研究表明，进行一次代码审查的时长应控制在90分钟内，这里建议控制在60分钟以内，如果审查时长超过 90 分钟，则代码审查的效率会大大降低。也就是说，虽然花费了更多的时间，但得到了更糟糕的结果。
在进行代码审查之前，应该规划好需要审查的代码长度及时长，尽量控制在200行代码、60分钟左右，这样的效率是最高的。