我们如何进行代码审查

本文来源于我在InfoQ中文站原创的文章，原文地址是： http://www.infoq.com/cn/news/2014/01/how-we-do-code-review

Jim Bird是一位经验丰富的软件开发经理、项目经理与CTO，专注于软件开发与维护、软件质量与安全等领域中疑难问题的解决。在过去的15年间，Jim曾管理过团队建设并主导过高性能的财务系统的建设。他的主要兴趣在于如何提升小团队的效率以构建真正的软件：高质量、安全、可靠、高性能及适应性强。近日，Jim撰写了一篇 博文，谈到了代码审查的价值，如何进行代码审查，代码审查的过程以及在代码审查中需要注意的问题，希望能为大家平日的代码审查带来一些启示。

开始代码审查

从一开始，开发者就会互相帮助，如果测试中遇到了问题或是有新人加入到了团队，领导或是资深开发者就会审查他们的代码。除此之外，我们还聘请了外部专家进行 安全代码审查。

系统发布后，我们决定更加主动一些，开始了基于风险的审查：项目中有人会编写一些风险较高的代码（比如说框架与安全代码、APIs、核心业务逻辑或是之前曾经出现过问题的地方），我们会审查他们的代码。在这个过程中，代码审查体现出了它的价值，我们收获颇丰。即便如此，我们还是更进一步，让代码审查成为一个标准的实践。

这并不是一夜之间就形成的。让团队相信代码审查的价值并不是什么难事，他们已经通过基于风险的审查获得了收益。不过要想改变人们的工作方式就不是那么简单的事情了，还要确保他们有足够的时间进行代码审查，理解并对反馈作出响应。此外，设计一个高效的代码审查流程也是需要花时间的。

一开始，我们让开发者选择好搭档并安排审查，但结果却有些混乱。有时，开发者会寻找那些好说话或是比较忙的人，这样审查就比较容易通过了；此外，两个开发者还有可能事先商量好，因此审查过程就会很快结束。由于人们并不知道要花费多少时间才能完成代码审查，因此审查经常会拖得很久，常常在代码已经完成测试甚至是发布后才完成。

由于大多数人并没有太多的代码审查经验，因此他们并不确定在审查时应该看什么，如何给出有意义的反馈等信息。开发者常常会被负面的批评搞得很沮丧，有时甚至会心烦意乱。

最后，我们决定由领导来完成大部分审查工作。虽然这会增加领导的工作量，也意味着他们没有太多时间编写代码了，不过这么做却是很有效果的。通常情况下，主开发者会对需求有着更好的理解，对代码的行为有着清晰的认识，这也意味着他们更有可能发现代码中的错误。由于是同一个人完成了大部分的代码审查，因此被审查的开发者会收到一致的反馈信息。

我们如何进行代码审查

在过去的几年间，我们进行代码审查的方式几乎没有发生过什么大的变化。

无论是谁编写的，无论代码的功能是什么，重要的代码变更是一定要审查的。我们并没有一个正式的审查会议，也没发现使用诸如Code Collaborator、Crucible等工具有什么必要性，不过现在看起来使用这些工具来管理和追踪审查有助于团队更好的起步。

有时，审查是面对面完成的，不过大多数时候都是离线进行的。审查者与开发者会交换信息，也许通过邮件发送文件，因为我们觉得这种方式更加便捷，也更加方便每一个人安排自己的时间。

随着时间的流逝，审查中的变化之处是审查者该看什么，以及看到的结果。

审查正确性

很多时候，程序员们会花费很多时间争论在代码审查过程中应该看什么，但实际上他们却没有花太多时间完成真正的代码审查。

我们开始代码审查的目的是改进代码质量，寻找测试中没有发现的问题。这么做并不是教授经验不足的开发者如何编写更好的代码，或是如何在团队内分享知识。这些都应该是代码审查所带来的间接好处，不过审查的最终目的应该是确保代码能够正常工作。

相对于使用长长的检查列表，审查者在看代码时首先会问几个问题：

• 代码的行为是否与预期一致，其逻辑是否是正确无误的？
• 被审查的代码是否与其他代码拥有类似的结构和功能？

审查可理解性

随着时间的流逝，在代码审查中寻找和得到的东西会发生变化。这是因为代码本身会发生变化，完成这项工作的人——开发者与审查者也可能发生变化。

开发者需要在其IDE中装上代码检查工具，同时我们也要使用一些优秀的静态分析工具来自动检查常见的编码Bug和不好的编码实践。这意味着审查者的时间可以花在更加重要的设计错误上，比如说并发Bug、竞态条件和潜在的死锁等，因为这些问题是工具所无法检测到的。

理解，而不是批评

审查的目的是理解代码，确保代码能够正常工作，而不是批评任何人。

在代码审查过程中，请不要掺入诸如“我认为好的代码是什么，你认为好的代码是什么”这样的论战之中。代码审查应该重点关注“我需要完全理解这部分代码才能确保它能够正常工作，如果由我来修复代码中的问题，我是不会这么写的，因此希望你也不要这么来写”。

随着时间的流逝，有些东西会变好，有些则不然

随着时间的流逝，在查看代码时你会发现代码在不断变化。Michael Feathers发现大部分代码在写完后很少或是从来都不会变化；大部分变更都是发生在代码基中的很小一部分代码上的。

由于审查者会看到相同的代码在不断发生着变化，因此这也会改变他们的审查方式。

避免收益递减

类似于软件开发中的其他实践一样，你最终会发现代码审查有着收益递减的效应，特别是一直在做相同的事情的时候，以相同的方式查看相同的问题。最终会有这样一个阶段，那就是代码审查的价值几乎降到了0。不过这种情况并没有发生在我们身上，我们依然从代码审查过程中获得了很多收益。

即便改进了工具，增强了测试能力，我们依然进行代码审查，因为Bug检查工具、审查与测试会发现不同类型的问题。我们还发现代码审查会让测试更加高效，这是因为如果之前曾经发现过问题，那么开发者与测试者之间就不会出现过多的往复情况。