谈谈 Code Review

谈谈 Code Review

说明

​ 这篇来讲讲 Code Review。Code Review 是软件开发过程中非常重要的一个环节，不过相对于单元测试，大家可能接触更少，同时，想要做好 Code Review 往往也更困难。在这篇文章里，我会先普及 Code Review 的常识，然后讲一些自己在实践中积累的经验。PS，这也是一篇会保持更新的文章 : )

什么是 Code Review？

​ Code Review 翻译成中文是代码评审，具体的定义可以看 wiki。这篇 wiki 介绍说 Code Review 在帮助团队找到代码缺陷这件事上作用巨大：“代码审查一般可以找到及移除约65%的错误，最高可以到85%”。实际上， Code Review 的好处远不止这一条，它至少能在以下三个方面帮到我们：

1. 传播知识。相信很多人第一次提交 Code Review 都有类似的经历：短短几百行代码，却被提了密密麻麻几十条 comments，更新了十多次代码，才最终被 accept 。其实当代码被 accept，提交代码的工程师通过这次 review 就学习到了代码规范和很多好的实践。同时，通过 review 更资深工程师的代码，年轻的工程师也更直观地学习架构和编码；另外，工程师之间也可以通过 review 代码来共享项目知识，看代码实现在绝大多数时候是了解项目的最好方式。
2. 增进代码质量。这点也很容易理解，有经验的工程师可以在架构设计、代码细节等各个方面帮助到初学者。不同工程师也会有知识盲点，互相 review 进步也很快。另外，被 review 的代码质量更高还有一个很多人注意不到的心理因素：在状态不佳的时候，工程师难免会匆忙写些“潦草”的代码，但是当你知道自己的代码会被review 的工程师提交 comment 打回来，自然会更仔细些 : -)
3. 找出潜在的 bug。这是大部分团队进行 Code Review 的目的。就像上面提到的，Code Review 在这方面效果不错。其实我认为大部分代码 bug 应该由单元测试，功能测试，性能测试和回归测试来保障。不过由于静态分析不理解业务，另外有些 bug 在测试中并不容易复现，这两种情况下，经验丰富的工程师来 review 代码就尤其重要了。

​ 接下来就不对 Code Review 的作用再长篇大论了，事实上，网上这类文章非常多。我接下来想谈的是做 Code Review 的根本原因是什么。

​ 毕竟 Code Review 是在编码过程中加了一道流程，又需要很多交流沟通，review 双方甚至可能由于编程理念不一样而产生争执，各种原因导致做好 Code Review 并不容易。现实情况确实如此，比如 v2ex 上有个帖子：发起个讨论，你们公司有 code review 吗？ ，一百多位工程师回答了这个问题，从他们的答案中可以看出，做好 Code Review 的公司寥寥无几。即使在一线互联网公司，也有不少团队反对 Code Review ，陈皓就写过一篇文章：《从Code Review 谈如何做技术》 ，提到了在阿里实施 Code Review 遇到的阻力，反对的原因是工期紧、需求变更快。如果不想清楚为什么要做 Code Review ，遇到障碍会非常容易妥协，慢慢 Code Review 就会走样，最终流于形式。反之，在我们遇到障碍，review 代码不顺利时就会以积极的心态来解决问题。

​ 在我看来，我们在完成编码工作的同时，也需要时时分享架构设计，交流各自的代码，Code Review 正是时时分享架构设计，交流代码最好的方式。这是我们需要做 Code Review 的根本原因。交流代码的频率非常重要，在你构思好设计，有了骨架代码，写完一个功能后都应该及时提交 review，以便其他工程师能够及时了解你的思路，和你沟通实现细节。而那些认为代码只要能运行就好，或者认为没人应该对自己代码指手画脚的人显然和你我不同，他们不会是这篇文章的读者 : )

​ 那么我们怎样做好 Code Review 呢？两个方面：一是减负，Code Review 只做它应该做的事情。二是提高 Code Review 的效率。

Code Review 应该讨论的是功能实现、架构设计、代码质量，不应该做以下两件事情。

1. 检查代码风格和编程规范。除了新人，其他工程师提交的代码不应该通过 review 来确保代码风格。这里所说的代码风格包括但不限于：命名规范、代码缩进、注释和文档等等。你可以利用 IDE 或者其他工具来保证编程规范和代码风格的统一。如果你在制定团队的代码规范，可以 follow Google Java Style 或者 Facebook Coding Standards 。在这里我推荐好好看看 Facebook 的规范，因为 Google 的代码规范告诉你应该怎么做，而 Facebook 的规范解释了为什么要这样做，以及在什么情况下需要权衡。
2. 检查常规的 bad smell 和代码 bug 。《重构》 和 《Clean Code》 对代码 bad smell 都有非常详细的描述。团队的工程师应该熟读这两本书，避免这些 bad smell，比如：重复代码、过长函数、过大类、过于亲密的两个 classes 等，你可以利用 IDE 和静态代码检查工具 checkstyle、 findbugs 和 pmd 来帮你检查出这些问题。同样，代码中的大多数 bug 也不应该在 Code Review 阶段来发现，你应该通过静态工具、单元测试、集成测试和性能测试来发现它们。

再说说如何提高 Code Review 的效率。

• 选用合适的工具。我用过 Phabricator、Gerrit、Gitlab 来 review 代码。这里推荐使用 Phabricator，就不过多介绍了，可以看 这里 的讨论。
• 每次只 Review 少量代码。新人经常犯的一个错误，花了两周甚至更多的时间写代码，然后又花了一些时间做测试，等他们自己觉得“大功告成”才敢自信地提交 review，殊不知，这个时候提交的 review 几乎没有什么意义。一方面，对于提交 review 的工程师来说，辛辛苦苦开发测试了两三周，就等 review 完成后 release 了，这时候如果收到各种需要修改代码的意见，心里难免会有抵触情绪，尤其是 review 的结果是架构需要改动，代码需要大面积重写，内心一定是崩溃的。另一方面，代码审查者如果面对数千甚至上万行代码，需要理清项目架构都需要花费大量时间，强行 review 这种代码，争论、修改、测试代码，很可能 一周甚至更长时间都完成不了 review，结果就是双方都痛苦不堪。在实践过程中，我们认为，频繁 review 代码并且每次只 review 少量代码非常重要，我自己认为 reivew 不超过 500 行代码比较好。
• 明确职责。 Review 过程中经常遇到的一个问题是 review 响应不及时。比如 assgin 给了工程师，工程师没有及时 Review，或者提交 review 的工程师没有及时响应修改意见。造成这种现象的原因大致有这么几种：工程师没有及时处理 review 的习惯；review 工程师需要项目的领域知识等。解决方法也很简单，Review 是项目开发的一部分，进度由开发工程师来负责，这样，Code Review 如果不顺利，应该由提交代码的工程师负责推动，如果需要讲解代码，提交代码的工程师应该主动走到 reviewer 工位上，说说背景知识和代码逻辑。也就是说，如果沟通受阻，工程师应该更积极的面对 reviewer ，毕竟大家是在花自己的时间来帮助他。
• 整理 checklist。如果你回顾犯过的编程错误就会发现，在某个阶段自己容易犯类似的错误。其实上，团队里的工程师也有这种情况。刚入门的工程师会出现 API 误用；刚加入团队的工程师对编码规范需要一段时间来适应；有些工程师在代码命名上总会犯同样的错误；也有一些工程师搞不清楚并发编程；还有工程师甚至常常写面条式的代码而不自知。如果每位工程师有自己的 checklist 来记录这些问题，定期回顾自己是不是还在犯类似的错误，他们的水平就会很快提高，至少不容易重复已经犯过的错误。同样，团队也需要积累 Code Review 的 checklist，刚开始，这个 list 可能非常初级，会有一些常见的 bad small，甚至代码规范的内容。不过没关系，相信随着团队成员能力的提升，这个 list 慢慢会集中在设计方面，比如编写代码的工程师有没有考虑到代码可维护性、扩展性和性能等等。
• **完善CI/CD设施。**很多团队不愿意做 Code Review，其实和不愿意做单元测试、集成测试原因一样，这些团队的CI/CD 工具不成熟，每增加一个不直接产出“功能代码”的步骤就会增加工作负担。其实根本原因是工程效率工具的缺失。
• **培养工程师的能力。**还有一个比较常见问题是，有些新人面对被 review 代码往往提不出问题。这个时候就需要工程师提升自己的能力。如果平时有积累，面对等待 review 的代码，即使不能面面俱到，也能提供不少有价值的意见，比如整理学习 Restful API 知识，在评审 Http 接口代码时就会是专家；掌握了 Spring 框架，Guava 等工具类，也能在很多时候提出很好的建议。慢慢地积累更多经验后，这些工程师就能提出更多业务、设计方面的意见。