关于代码评审CodeReview

在一个成熟的团队中，CodeReview 是整个研发流程中不可或缺的一步，而那些即将走向成熟的团队可能对 CodeReview 有很多的误解和问题，也不清楚 CodeReview 该如何去做，本文笔者将结合自己的经验和知识，谈谈对 CodeReview 流程的一些理解和建议。

什么是CodeReview

CodeReview 国内也称代码评审或者代码审查，也简称CR，是指在软件开发过程中，工程师对其他人所写代码做审阅（后文统称CodeReview），以达到控制代码质量的目的。通常的流程都是由代码写作者发起，请团队内其他人审阅代码，其他人对代码提出改进建议，再由代码写作者修改重新提交，直至代码通过大家的审阅为止。

为什么要做CodeReview？

其实很多人都不是很重视CodeReview，因为CodeReview的效果短期很难看到，也很难量化衡量。就如同运动一样，偶尔过量的运动不仅对身体无益，可能还会起反作用，不过长期的坚持肯定会让你更健康，但能让你健康多少很难量化，不过前一段时间github上爆火的项目《程序猿延寿指南》里给出了可参考的数据，每周3次45分钟挥拍运动可以减少全因死亡率47%，按其公式折算大概增加9年的预期寿命。

运动除了增加预期寿命外，也能显著减少很多疾病的发病率。坚持CodeReview如同坚持运动一样，趋势肯定是让整个代码库更为健康长寿。

CodeReview从长期来看，有几个明显的好处，接下来就一一讲一下。

提升代码质量

假如将一个系统比作一个生命体，一行行代码比作一个个细胞，不好的设计宛如癌细胞，会逐渐扩散，终将杀死系统。而CodeReview的过程就像是T细胞吞噬掉癌细胞，保证系统的健康成长。让系统有更长久的生命力。

没有人Review的代码，其代码水准就是写代码人的水准，而被一个团队Review过的代码，它的水准将接近甚至超过整个团队的最高水准

因为单个人可能在某些方便做的比较好，集大家之所长就能在各个方面都做的比较好。另外，随着CodeReview流程日常化，每个参与人的编码能力也会逐步提升，无限趋近于团队最高水准，因为在CodeReview的过程中，你可以看到别人做的好的地方，可以学习到经验，也可以看到别人做的不好的地方，吸取到教训。随着时间的流逝，逐渐积累为参与者的能力。

提前发现问题

在没有CodeReview流程的时候，我们都是依赖于测试，甚至是依赖于功能上线后用户暴露问题，这种发现方式已经偏晚了，尤其是让用户暴露问题的时候可能问题已经非常大了。「问题暴露的越晚，风险也就越大」。而CodeReview一般是放在代码测试之前，如果能在这个阶段就发现问题就能提前将各种风险扼杀在摇篮中。但是，CodeReview真的能提前发现问题吗？如果能的话，可以提前发现什么样的问题？

我个人觉得CodeReview可以提前发现流程或者实现上的问题，尤其是在做业务需求的时候，不同工作经验的人对同一个需求的理解肯定会有差异，代码实现上也会有很大的差异，有时候一点点的理解偏差，导致出现那种失之毫厘谬以千里的后果，写代码的人也会因为处在自己的思维定式中发现不了问题，而别人可能因为经验丰富或者需求相关背景了解更多，就能很轻易发现问题。

像这种情况一般出现在团队新人身上，他们对已有系统和业务了解不多，实现需求时很容易出现问题，这时候如果有人帮忙指出就能避免更严重的后果，另外也有助于新人了解业务和融入团队。

在CodeReview过程中另外一种问题也比较容易发现，就是那些别人之前踩过的坑。比如Java中SimpleDateFormat其实有线程安全的问题，不了解的人很容易就踩坑了。如果别人踩过这个坑，就可以在CodeReview过程中提前帮你指出来。很多开源的类库，甚至Jdk中很多包或多或少都有使用上的坑…… 这种例子就数不胜数了。

当然CodeReview也不是万能的，也有很多问题发现不了的，比如一些边缘Case或者是代码计算结果的准确性，比如你代码实现了一个很复杂公式的计算，你总不能指望有人能通过看代码来发现问题吧。这些还是老老实实去做测试吧！

经验和知识的传递

程序猿可以写出能运行的代码，但真正的工程师才能写出低bug、易扩展、易维护的高质量代码，更高级的工程师还能帮其他人成为一名合格的工程师。CodeReview的流程也是高级工程师来帮助其他人最直接的方式。

在CodeReview中，你可以看到其他人写出来的优秀代码、优秀的设计，甚至和改动相关的业务背景知识……，Review越多，学到的也越多。另外，即便是哪些不太好的代码，经过别人的Review，必然会留下很多评论或者是改动建议，甚至是别人之前踩的坑，你都能看到，从这些内容上你也可以学到很多新的知识。CodeReview不仅仅是一个提升代码质量方式，它也可以肩负起知识积累和消息集散的功能。

举个我之前遇到过的例子，我们之前有个功能需要操作机器上的文件，当然用Java的File也能实现，但是对操作多层级的文件夹时就很不方便了，需要自己写很多的代码，搞不好还容易出bug。后来我发现apache-common包中提供了IOUtils类，可以很方便操作文件夹。我把这个写到CodeReview的评论中，只要看到过的人都会知道原来有这个东西可以用。

所以，Review别人代码时请毫不吝啬地留下你的建议吧，另外，CodeReview时也不要忘记关注下别人的建议，说不定可以学到新的东西。

如何做好CodeReview

说完了CodeReview的好处，相信你肯定已经跃跃欲试了，如何能做好CodeReview？这里我根据我的知识和经验分享下我的看法。

CodeReview的步骤

了解改动的背景

CodeReview不是一上来就看代码，这样有可能你会看的云里雾里，纯粹是浪费时间。CodeReview虽然是Review代码，但是首先你的知道你要看的代码实现了什么样的功能，是在什么样的背景下去做的，清楚前因后果之后，你才能知道这个代码大概应该怎么去写，你才能更好的去Review别人的代码、去发现别人的问题。

纵观全局

知道背景之后，在你脑海中就会有一个大概的编码思路，也有个流程主线。这个时候可能有两种情况，你和写代码人的思路相同，那你就顺着你们共同的思路去帮忙Review整个流程是否正确。另一种情况就是你们思路不同，你就得看代码去了解写作者的思路，然后确认是谁的思路有问题，或者是谁的思路更好，然后同写作者一起将这个流程优化到更优。

逐层细化

确定完整个流程之后，就可以逐步深入到代码细节中了，细节可以Review的地方就很多了，可以看下下一节的内容，这里就先不展开了。

CodeReview的关注点

在CodeReview的过程中，如果有一些立足点的话可以帮助大家更好的完成CodeReview的过程，我大概总结出以下几点：

「功能性：」 代码所实现的功能是否和预期一样，是否实现了所有必须的功能？
「复杂性：」 功能实现是否过于复杂？过于复杂的代码更容易出问题，而且可维护性也会更低。
「代码风格：」 代码是否符合团队编码规范？
「文档&注释：」 如果代码功能有改动，关注下相关文档和注释有没有同步改动。错误的注释和文档可能会让未来的开发者产生理解成本。
「代码亮点：」 如果你看到变更中做得好的地方，也别吝啬你的赞美。
上面描述更偏概括性，我们来举一些更详细的例子，帮助大家理解上面几点。

代码设计良好，可读性和可维护性高。
是否有线程安全的bug。
代码没有增加不必要的复杂性。
开发者没有写一些目前没有在用的代码，这种无用的代码未来大概率也不会用，所以不要假设。
代码有适当的单元测试。
测试逻辑设计良好。
开发者使用了清晰明了的变量和函数命名。
注释清晰明了且实用，并且解释清楚了为什么这么做，而不仅仅是做了啥。
代码有相应完善的文档。
代码风格符合规范。
代码有没有更优的实现？（性能、资源占用……）

注意事项

CodeReview的礼节

首先CodeReview不是你个人炫技的舞台，看到别人代码的问题需要礼貌指出，切忌diss别人。其次，CodeReview不要为了提问题而提问题，有些代码就是没问题，你也没必要纠结必须要提个问题，直接通过即可。这里在额外说一点，大部分Review别人的代码，只关注到别人做的不好的地方，而忽视了别人做的好的地方，遇到好的代码、好的设计也别忘记点个赞。

CodeReview应当及时

别人提的CodeReview应当及时处理，在很多公司中CodeReview是开发流程中必要的一环，没有Review通过的肯定是不能上线的，如果CodeReview长时间不处理可能会延误后续的流程。另外一点，CodeReview是相互的，今天你及时帮别人Review了，明天别人也会及时帮你Review，「与人方便即与己方便」。

如何写出对CodeReview友好的代码

「程序写出来是给人看的，附带能在机器上运行。——Harold Abelson」

时刻谨记上面这句话。

提交前先做好自审

提前自己处理掉一些低级错误，可以先借助一些工具完成一些简单的检查。例如我们团队会借助checkstyle、spotbug、sonar、pmd等工具完成代码风格和一些潜在bug的检查。然后自己做好功能的自测，尽可能先消灭一部分bug，为CodeReview减少一些负担。

写清楚变更描述

这里对应上文中CodeReview步骤中的了解改动背景，作为代码的写作者，你不能一上来就让别人从代码入手吧，可能看你代码的很多其他人都缺少代码改动相关的上下文信息，完全理解代码成本很高，所以需要在变更描述中将这些信息描述清楚，包括但不仅限于改动背景、改动点、流程、具体设计…… 一句话概括就是好的变更描述应该说清楚这次是做了什么变更，以及为什么要这么做

更详实的变更描述可以让Review代码人减少理解成本，更快完成CodeReview的流程，你代码改动也就能更快进入后续流程了。

注意自己的情绪

代码的写作者在CodeReview的流程中是弱势的一方，很多人代码在收到评论被要求要修改代码的时候，都有一种不太愿意接受的心态，其实我自己也经常会有这种心态，尤其是在时间比较紧张的情况下。

其实这是很正常的心态，但是还是需要去控制自己的情绪的，从评论者的角度出发，他肯定也不是在刁难你，也是希望代码质量能更高。如果真遇到这种情况，首先可以和评论者讨论清楚他的要求是否合理，如果确定了合理性当然还是要改的。但如果在时间比较紧张的情况下，协商是否可以不修改或者是之后再修改，确实有些锦上添花的修改没必要阻塞之后的流程。

CodeReview流程应该尽早提交

日常情况下，还是建议早点提交CodeReview，并留出一定的时间来做修改，尽可能不要让这个流程变的匆忙。大部分公司的实践都是在进入测试流程的时候同时进入CodeReview流程。

关于CodeReview的几个误区

CodeReview是纯浪费时间？

如果你的团队刚开始推行CodeReview流程，这个问题肯定是会被很多人问到的。所以是不是呢? 这里我拿锻炼身体类比下，大家都知道坚持运动对健康有益，但如果只是一时兴起，偶尔过量的运动不仅对身体无益，可能还会起反作用。

实际上，坚持运动对健康的影响其实很难量化，但我们现在都知道，运动是好的。同样，CodeReview如同运动一样，它可能会耗费一定的精力和时间，但长期坚持下来必然会让整个代码库、整个系统甚至这个团队更加健康。

另外，如果一个团队CodeReview机制很成熟，写代码的人随着被Review的次数增加，其代码质量必然也会逐步提交，那么代码中的问题肯定也会逐步减少，随之而来的就是CodeReview的过程越来越轻松。

「困难的路会越走越简单，而简单的路会越走越困难。」

工期很紧，没时间做CodeReview！

这也是很多团队不做CodeReview的借口。你不做CodeReview省下的时间，会在后面测试，甚至是在后面长期维护中花费更多的时间。我们有句耳熟能详的老话叫做“磨刀不误砍柴工”，其实CodeReview和测试在软件开发过程中就是“磨刀”的工作。

「做好事前控制而不是事后弥补。因为时候弥补的代价会非常高，」

只有高级工程师才有资格Review别人代码？

虽然事实上几乎都是高级工程师在Review别人的代码，我理解造成这种现象的原因其实也比较简单，高级工程师确实经验丰富一些，也更容易Review出别人代码中的问题，所以会留下更多的Review记录，造成只有高级工程师才参与CodeReview的假象。

但这绝不意味着初级工程师没有资格Review别人的代码，所谓三人行必有我师，即便是初级工程师也有可能发现Review出别人代码中的问题。

即便你暂时确实经验不够丰富，很难找出别人的问题，但你也可以从别人Review代码的过程中学到很多东西，就像上文所说CodeReview也是团妒经验和知识的一种传递方式。参与CodeReview也是成为高级工程师必不可少的一步。

都有测试流程了，为什么还要做CodeReview？

这个问题算是已经在上文中回答过了，CodeReview需要看很多个方面的内容，而测试只能保证其结果的准确性。

有了CodeReview就不需要测试了？

问这个问题的人相比于问上个问题的人又走了另一个极端，又把CodeReview和测试放在了对立面上。虽然CodeReview如果做的比较好的话，确实能提前发现一些比较明显的问题，但是做CodeReview的是人，人是无法大规模且精准的去校验程序的执行过程，而这恰恰是自动化测试所擅长的，所以说CodeReview和测试并不是对立关系，而是一种互补关系。

只要我在团队推行了CodeReview流程，代码质量就会迅速提高？

首先可以肯定的是代码质量是会提高的，但也许没有那么快。我之前从一个没有CodeReview流程的团队加入到一个有严格CodeReview流程的团队时，有较长一段适应期。我当时最大的感受就是CodeReview太耗时间了，尤其是我刚开始还没适应新团队开发规范的时候，写代码和被Review后改代码所花费的时间基本上五五开，当然之后会好很多。

另外，每个人也需要花20%左右的时间和精力去Review别人的代码，相当于花接近三分之一的开发时间来处理CodeReview相关工作，这个比例高到可能你们老板都会怀疑，但我们当时就做到了，结果就是我们的代码质量非常高。
以我的经历来看，如果CodeReview想要有效果，时间投入肯定是少不了的，冰冻三尺非一日之寒,滴水石穿非一日之功。不要高估它的短期价值，也不要低估它的长期价值。

转自： https://segmentfault.com/a/1190000041885333