关于代码评审(CodeReview)那些不得不说的事儿

简介: 关于代码评审(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的流程,你代码改动也就能更快进入后续流程了。

单个变更尽可能短

  一个非常大的变更几乎没有Review,因为大的改动首先就很难Review,其实也更耗费时间,还有出问题的概率也更大,谁Review通过了代码但之后上线出问题,可能是要和你一起背锅的。所以建议大家将大的代码变更尽可能拆小,因为小的变更有以下这些好处:

  • 代码评审更快 与比起花30分钟评审一个大的变更相比,对Review代码的人来说花5分钟审查一系列小的变更更加容易.
  • Review更加彻底。 进行较大的更改后,审阅者和作者往往会因大量详细评论的来回移动而感到沮丧,有时这些评论会漏掉或遗漏重要的观点。
  • 减少导致bug的可能性。 由于您所做的更改较少,因此您和您的审阅者更容易有效地推断出CL的影响,并查看是否导致bug。
  • 减少不必要的工作 当你写了一个巨大的变更,然后评审者觉得你总体方向错了,这会导致你浪费大量的工作。
  • 更方便合并代码 因为大型的变更会导致大量的冲突,因此合代码的时候会耗费很多时间,而且可能因合并代码导致问题,我们就出现过好几次代码合并的时候冲掉别人代码的情况。
  • 有助于你作出更好的设计 优雅的设计并且完善一个小的改动比大的改动更加容易
  • 降低评审者的难度 提审部分改动,不会影响你继续编码。
  • 如果真出问题,回滚更容易 这个就不用多解释了吧。

  对于那些很难拆分的变更,也不是需要完全禁止,有时候就是有改动非常大,而且无法再去拆分了,这时候还是建议通过其他方式来保证代码质量,比如全流程测试。 然后也建议做好出问题时的快速恢复方案。

注意事项

注意自己的情绪

  代码的写作者在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想要有效果,时间投入肯定是少不了的,冰冻三尺非一日之寒,滴水石穿非一日之功。不要高估它的短期价值,也不要低估它的长期价值。

参考资料

  1. 谷歌工程实践——代码评审

  今天的分享都到这里了,大家也都知道现在外面大环境不好,这个时候更不能停下松懈的脚步。刘易斯·卡罗尔在《爱丽丝梦游仙境》中借红桃皇后之口说出了下面这句话,现在看来也是出奇的合适呢!

在我们这个地方,你必须不停地奔跑,才能留在原地。如果你要抵达另一个地方,必须以双倍于现在的速度奔跑。

——【英】刘易斯·卡罗尔《爱丽丝梦游仙境》

目录
相关文章
|
7月前
|
SQL 缓存 安全
一文浅谈CodeReview中的一些思考
CodeReview在日常的开发过程中越来越被重视,它在提高代码质量同时促进团队成员之间的知识共享和技能提升方面发挥了诸多作用,本文将主要围绕CodeReview展开,简单聊聊在CodeReview过程中的心得和思考。
89090 4
|
7月前
|
安全 Devops 测试技术
带你读《代码管理实践10讲》——六、代码评审到持续交付的最后一公里
带你读《代码管理实践10讲》——六、代码评审到持续交付的最后一公里
74 0
|
7月前
|
测试技术
如何做需求评审?
如何做需求评审?
112 0
|
SQL 安全 算法
codeReview
codeReview
130 0
|
SQL 缓存 NoSQL
代码评审的18个军规,收藏好!
大家好,我是田螺。 我们开发完需求,提测前,一般都需要代码评审。小伙伴们,你们知道代码评审,一般都有哪些军规嘛?今天田螺哥给你带来代码评审的18个军规。
208 0
|
搜索推荐 测试技术 UED
需求评审那些事
需求评审那些事
|
测试技术
关于需求评审和讲解的一些思考
上图为[产品迭代开发协作流程],上次聊了一下关于 Code Review 的一些思考。 在上面的流程中,需求评审通过后,要产出最终的需求文档、原型等交付物,还要对本次需求封版;在测试阶段的补充需求的处理方式;上线后记录或反馈的问题列入下一版的迭代。 可是,流程中都是比较理想的状态,现实工作中并没有达到设想的目标。作为一个开发人员,分别站在产品经理的角度和开发人员的角度分析一下问题的可能原因。
测试思想-文档评审 关于需求评审
测试思想-文档评审 关于需求评审
79 0
|
开发工具 git 开发者
使用commitizen实现按团队规范提交代码
使用commitizen实现按团队规范提交代码
使用commitizen实现按团队规范提交代码
|
安全 算法 Java
代码评审的三怕
代码评审的三怕