[转载] 代码审查

转载自http://www.williamlong.info/archives/3272.html和http://www.ibm.com/developerworks/cn/rational/11-proven-practices-for-peer-review/

 

代码审查(Code Review)是软件开发中常用的手段,和QA测试相比,它更容易发现和架构以及时序相关等较难发现的问题,还可以帮助团队成员提高编程技能,统一编程风格等。

  1. 代码审查要求团队有良好的文化

  团队需要认识到代码审查是为了提高整个团队的能力,而不是针对个体设置的检查“关卡”。

  “A的代码有个bug被B发现,所以A能力不行,B能力更好”,这一类的陷阱很容易被扩散从而影响团队内部的协作,因此需要避免。

  另外,代码审查本身可以提高开发者的能力,让其从自身犯过的错误中学习,从他人的思路中学习。如果开发者对这个流程有抵触或者反感,这个目的就达不到。

  2. 谨慎的使用审查中问题的发现率作为考评标准

高效代码审查的十个经验

  在代码审查中如果发现问题,对于问题的发现者来说这是好事,应该予以鼓励。但对于被发现者,我们不主张使用这个方式予以惩罚。软件开发中bug在所难免,过度苛求本身有悖常理。更糟的是,如果造成参与者怕承担责任,不愿意在审查中指出问题,代码审查就没有任何的价值和意义。

  3. 控制每次审查的代码数量

  根据smartbear在思科所作的调查,每次审查200行-400行的代码效果最好。每次试图审查的代码过多,发现问题的能力就会下降,具体的比例关系如下图所示:

高效代码审查的十个经验

  我们在实践中发现,随着开发平台和开发语言的不同,最优的代码审查量有所不同。但是限制每次审查的数量确实非常必要,因为这个过程是高强度的脑力密集型活动。时间一长,代码在审查者眼里只是字母,无任何逻辑联系,自然不会有太多的产出。

  4. 带着问题去进行审查

  我们在每次代码审查中,要求审查者利用自身的经验先思考可能会碰到的问题,然后通过审查工作验证这些问题是否已经解决。一个窍门是,从用户可见的功能出发,假设一个比较复杂的使用场景,在代码阅读中验证这个使用场景是否能够正确工作。

  使用这个技巧,可以让审查者有代入感,真正的沉浸入代码中,提高效率。大家都知道看武侠小说不容易瞌睡,而看专业书容易瞌睡,原因就是武侠小说更容易产生代入感。

  有的研究建议每次树立目标,控制单位时间内审核的代码数量。这个方法在我们的实践中显得很机械和流程化,不如上面的方法效果好。

  5. 所有的问题和修改,必须由原作者进行确认

  如果在审查中发现问题,务必由原作者进行确认。

  这样做有两个目的:

  (1)确认问题确实存在,保证问题被解决

  (2)让原作者了解问题和不足,帮助其成长

  有些时候为了追求效率,有经验的审查者更倾向于直接修改代码乃至重构所有代码,但这样不利于提高团队效率,并且会增加因为重构引入新bug的几率,通常情况下我们不予鼓励。

  6.利用代码审查激活个体“能动性"

  即使项目进度比较紧张,无法完全的进行代码审查,至少也要进行部分代码的审查,此时随即抽取一些关键部分是个不错的办法。

  背后的逻辑是,软件开发是非常有创造性的工作,开发者都有强烈的自我驱动性和自我实现的要求。让开发者知道他写的任何代码都可能被其他人阅读和审察,可以促使开发者集中注意力,尤其是避免将质量糟糕,乃至有低级错误的代码提交给同伴审查。开源软件也很好的利用了这种心态来提高代码质量。

  7.在非正式,轻松的环境下进行代码审查

  如前所述,代码审查是一个脑力密集型的工作。参与者需要在比较轻松的环境下进行该工作。因此,我们认为像某些实践中建议的那样,以会议的形式进行代码审查效果并不好,不仅因为长时间的会议容易让效率低下,更因为会议上可能出现的争议和思考不利于进行如此复杂的工作。

  8.提交代码前自我审查,添加对代码的说明

  所有团队成员在提交代码给其他成员审查前,必须先进行一次审查。这次自我修正形式的审查除了检查代码的正确性以外,还可以完成如下的工作:

  (1)对代码添加注释,说明本次修改背后的原因,方便其他人进行审查。

  (2)修正编码风格,尤其是一些关键数据结构和方法的命名,提高代码的可读性。

  (3)从全局审视设计,是否完整的考虑了所有情景。在实现之前做的设计如果存在考虑不周的情况,这个阶段可以很好的进行补救。

  我们在实践中发现,即使只有原作者进行代码审查,仍然可以很好的提高代码质量。

  9.实现中记录笔记可以很好的提高问题发现率

  成员在编码的时候应做随手记录,包括在代码中用注释的方式表示,或者记录简单的个人文档,这样做有几个好处:

  (1)避免遗漏。在编码时将考虑到的任何问题都记录下来,在审查阶段再次检查这些问题都确认解决。

  (2)根据研究,每个人都习惯犯一些重复性的错误。这类问题在编码是记录下来,可以在审查的时候用作检查的依据。

  (3)在反复记录笔记并在审查中发现类似的问题后,该类问题出现率会显著下降

  10. 使用好的工具进行轻量级的代码审查

  “工欲善其事,必先利其器”。我们使用的是bitbucket提供的代码托管服务。

  每个团队成员独立开发功能,然后利用Pull Request的形式将代码提交给审查者。复审者可以很方便在网页上阅读代码,添加评论等,然后原作者会自动收到邮件提醒,对审阅的意见进行讨论。

  即使团队成员分布在天南海北,利用bitbucket提供的工具也能很好的进行代码审查。

1. 一次评审量要低于 200–400 行代码

Cisco 代码评审研究(见于工具栏)显示了为了得到优化的效率,开发员的评审量要低于一次 200-400 行代码(LOC)。超过这个量,搜索缺陷的能力就会降低。以这个速度,您可以找到 70-90% 的缺陷。换句话说,如果存在 10 个缺陷,那么您可以找到其中的 7 到 9 个。

关于 Cisco 案例研究

在 10 个月的监视工作之后,研究总结出了一个理论:如果适当操作的话,轻量级代码评审工作与规范的评审工作同样有效,但是前者的速度会更快(更省时)。与规范评审相比,轻量级代码评审平均要少花 6.5 个小时,并发现更多的错误(bug)。
除了确认这些理论,研究中还发现了一些新的规律,本文将这些规律都概括了出来。

图 1 中的图,描述了缺陷密度与评审代码行数量之间的关系,支持该规则。缺陷密度 就是每 1000 行代码之中所发现的错误(bug)数。评审代码行的数量超过 200 时,缺陷密度就会急剧地下降。

在这种情况下,缺陷密度就是“评审有效性”的评价手段。如果两个评审员评审相同的代码,其中一个发现了更多的错误(bug),那么我们就会认为该评审员更有效率。图 1 显示了当我们将更多的代码放到评审者面前时,她搜索缺陷效率的变化情况。这种结果很合理,因为她可能不会花费大量的时间去评审,这样就会不可避免的使得效率没有以前高。

图 1. 当代码行数量超过 200 时缺陷密度就会急剧下降,400 以后缺陷密度几乎为 0

缺陷密度与代码行数量之间的关系

 

2. 每小时低于 300–500 LOC 检查率的目标

定义

  • 检查率: 我们能够多快地评审代码呢?通常以每小时 kLOC(千代码行)来评价。
  • 缺陷率: 我们能够多快地发现缺陷呢?通常以每小时缺陷数来评价。
  • 缺陷密度: 给定量的代码之中我们能够发现多少的缺陷呢(而不是它们有多少)?通常以每 kLOC 中发现的缺陷数来评价。

您要花点时间进行代码评审。快速评审并不总是好的。我们的研究结果显示检查率低于“300-500 LOC/小时”时,可以得到优化的结果。根据所作的决定,评审者的检查率有很大的变化,就算是相似的代码开发者、评审者,文件和评审规模,也存在巨大的差异。

为了找到优化的检查率,我们将 缺陷密度 与评审者检查代码的 速度 进行了比较。得到的结果再一次落在了我们的预料之中:如果在评审您不花足够的时间,那么您就不会发现太多的缺陷。如果评审者面临大量代码的压力,那么他就不会每一行代码投入相同的注意力。他不能研究同一位置处更改的所有版本。

所以,多快算是太快呢?图 2 显示了答案:服务器端每小时超过 400 LOC 的评审速度会降低效率。而每小时 1000 LOC 的速度,您可能已经推出了,以这样的速度评审员可能根本都没有细看代码。

图 2. 评审量超过 500 行代码时检查效率就会下降了

缺陷密度与检查率之间关系图

 

3. 花足够的时间进行适当缓慢的评审,但是不要超过 60-90 分钟

永远不要对一个原型代码评审超过 60-90 分钟

我们应该讨论,为了得到更好的结果,不应该过快地评价。但是您也不应该在一个位置花太多的时间。大约 60 分钟后,评审者就会感到疲劳,于是就不会找到额外的缺陷了。这个结论得到了许多其他研究的支持。实际上,根据我们的常识,当人们从事注意力高度集中的活动时,性能状态在 60-90 分钟之后就会降低了。考虑到人体方面的限制,评审者在性能降低之前,不能评审超过 300–600 行的代码。

但反过来说,评审代码所花的时间不得低于五分钟,就算代码只有一行也是如此。通常来说,单行的代码也会影响到整个的系统,所以花上五分钟时间去检查更改可能造成的结果是值得的。

 

4. 确定在评审开始之前代码开发者已经注释源代码了

在评审开始之前代码开发者可能就消除大多数的缺陷,这一点我们将会看到。如果我们需要开发员双倍地检查他们的工作,那么评审可能更快地完成,而不用再去折中考虑代码质量的问题。就我们所知,这种特定的想法尚未得到研究,所以我们在 Cisco 研究期间对其进行了测试。

图 3. 代码开发者准备对缺陷密度的震撼性效果

Author Prep Comments 与 Defect Density 图

代码开发者准备 说的是代码开发者在评审之前要先注释他们的源代码。我们发明了这个术语,以描述研究中所评价的特定行为模式,大约 15% 的评审会阻止代码开发者这样做。注释会指导评审者进行变更,显示首先必须要查看的文件,并找到每一次代码更改的原因。这些注释不是代码之中的评论,而只是给其他评审者看的评论。

我们的理论就是,因为代码开发者需要重新考虑,并解释注释过程中所发生的更改,所以代码开发者需要在评审开始之前就找出许多缺陷,以让评审变得更加有效。如此,评审过程将会产生低密度的缺陷,因为更少的错误(bug)剩余了。很明显,与没有代码开发者准备的评审相比,代码开发者准备之后的评审有更少的缺陷存在。

我们还考虑过一个悲观的理论,以解释错误(bug)的低发现率。是不是当代码开发者在作出一项评论时,评审者会有偏见,或者自鸣得意,这样就不能尽可能多地发现错误(bug)了呢?我们随机抽查了 300 个评审者进行研究,结果证实评审者在评审代码时确实非常小心,更少的错误(bug)被发现了。

 

5. 为代码评审创建可定量化的目标,并获取制度,这样您就可以改进流程了

有了项目,您就该决定代码评审过程的目标,以及怎样评价效率问题了。当您在定义特定的目标时,您就能够决定同行评审是否真的达到了您所需要的结果。

最好从外部性的制度开始,例如“将支持访问降低 20%”或者“使开发引入的缺陷百分比减半”。这些信息使您能够更好地看清,从外部视角来看,代码能够做些什么,您还需要一个可定量化的评价手段,而不是“修复更多错误(bug)”的模糊目标。

但是,在外部制度显示结果之前需要花上一段时间。例如,支持性访问将不会得到影响,直到新的版本得到发布并交到客户手中为止。所以查看内部性流程工具,以得到发现多少缺陷,缺陷就是问题所在,以及开发员在评审上所花时间的清晰结果。代码评审的最常见内部性制度是 检查率 ,缺陷率,以及 缺陷密度

考虑一下只有自动化或者紧密控制的流程,才能给您带来可重复的代码;人类并不擅长记住启动或者终止计时器。为了得到最好的结果,您要使用能够 自动 收集代码的代码评审工具,这样用于流程改进的关键代码就是精确的了。

为了改进和提高您的流程,您可以收集代码并组合流程,以查看更改是如何影响结果的。很快,您就将会知道什么工作最适合您的团队了。

 

6. 使用检查表,因为它能极大地影响代码开发者和评审者的结果

使用检测表对于评审员非常重要,如果代码开发者忘记了某项任务,评审员也同样可能忘记。

我们极力推荐您使用检查表,来确定您可能会忘记的事情,它对代码开发者和评审者都有用。忽略是最难发现的缺陷;毕竟,评审不在那里的东西是很困难的一件事。检查表是解决这个问题的最好方式,因为它会提醒评审者和代码开发者花点时间去考虑一下可能被遗忘的事情。检查表还会提醒代码开发者和评审者确定所有的错误(bug)都得到了处理,软件功能已经通过了无效值测验,而且已经创建了单元测试。

另外一个有用的概念就是 个人检查表 。每个人一般都会犯 15-20 个错误(bug)。如果您注意到了一些典型的错误(bug),那么您就可以开发自己的个人检查表(Personal Software Process,Software Engineering Institute,以及 Capability Maturity Model Integrated 也都推荐该实践方式)。评审者将会完成决定一般性错误(bug)的工作。您所应该做的,就是拥有一个简短的检查列表,一般都是您容易遗忘的事情。

一旦您开始在检查列表中记录自己的缺陷,那么您就会少犯错了。规则将不断出现在您的脑海中,然后您的错误(bug)率就会下降了。这种情况我们将会发现它反复出现。

提示:
如果您想要得到关于检查列表的更多具体信息,那么您可以得到本文代码开发者所写书籍的免费拷贝,这本书为 Best Kept Secrets of Peer Code Review,网址为 www.CodeReviewBook.com

 

7. 确认缺陷得到了修复

是的,这种“最佳实践”看起来好像是没有脑子的。如果您遇到了评审代码以找到缺陷的所有问题,那么修复它们就变得顺理成章了!现在许多评审代码的团队没有一种好的办法,去追踪评审期间找到的缺陷,并确保评审完成之前错误(bug)确实得到了修复。确认电子邮件之中的结果,或者实时评审是非常困难的。

记住这些错误(bug)通常不是在 Rational Team Concert 日志中输入的,因为在代码发布给 QA 之前就发现了这些错误(bug)。所以,什么是代码在贴上“全部解决”标志之前确认缺陷的好办法呢?我们建议使用好的协作性评审软件,与 Rational Team Concert 相集成,以追踪评审之中所发现的缺陷。有了正确的工具,评审员就可以记录错误(bug),并在必要时与代码开发者进行讨论了。然后代码开发者会修复问题,并通知评审者,而评审者必须确认每个问题都得到了解决。工具应该追踪评审期间所发现的问题,并确保直到所有的错误(bug)被评审者 修复 才完成评审(或者作为稍后解决的单独工作项追踪)。工作项只有在评审完成时才能通过。

如果您真面临着搜索错误(bug)的烦恼,那么请确认您已经将它们全部安装了!

既然您已经学会了代码评审 流程 的最佳实践方式,那么我们接下来将会讨论一些社会效应,以及怎样管理它们以获得最佳的结果。

 

8. 培养良好的代码评审文化氛围,在这样的氛围中可以积极地评审缺陷

与其他我们能看到的大多数技术相比,代码评审对于真实团队构建能够发挥更大的作用,但是只是在管理人员能够以一种积极的,向上的,有技巧的方式进行交流时,这种优势才能发挥出来。将缺陷看做是不好的事物很容易(毕竟,它们是代码之中的错误(bug)),但是形成不好的缺陷检查态度,则会毁掉整个团队的努力,更不要说它会破坏错误(bug)检查过程了。

软件代码评审的要点在于,尽可能多的消除缺陷,不管是谁“导致”了错误(bug)。

管理人员必须建立缺陷是积极的这样的观点。毕竟,每一个缺陷的存在,都是改进代码的潜在机会,而错误(bug)评审过程的目的,就在于使代码尽可能地完美。每一个被发现并解决的缺陷,都是客户以后不会看到的缺陷,也是 QA 人员不必花费时间去解决的问题。

团队需要维持这样一种态度,就是发现缺陷,就意味着代码开发者和评审者 作为一个团队 去改进产品的质量成功了。而不是“代码开发者产生了一个缺陷,而评审者负责去发现它”。它更像是配对编程的一种有效形式。

评审员要向所有的开发者展示收集坏习惯,学习新技巧,并展开功能的机会。开发员可以从他们的错误(bug)中学习,但是只是在他们警惕错误(bug)时才会这样。如果开发员害怕发现错误(bug),那么积极的结果就会消失。

如果您是一名初级开发员,或者是一个团队的新成员,那么其他人发现缺陷时,就意味着您强有力的队友在帮助您成长为一个合格的开发员。这就比您单枪匹马地编程,没有具体的反馈时,要更快地进步。

为了维持检查缺陷是积极的这样一种理念,管理人员必须要承诺缺陷密度不会进入到性能报告之中。公开作出这种承诺是很有效的。这样开发员就会知道他们要怎样做,并抗议公开破坏这条规则的管理人员。

管理人员绝不应该将错误(bug)代码作为消极性能评审的基础。他们必须谨慎对待,并对批评造成的挫折感及消极反应保持敏感,并要一直提醒团队发现错误(bug)是一件很好的事情。

 

9. 警惕老大哥效应(Big Brother Effect)

作为一个开发员,您可以自动假设“老大哥正看着您呢”是真的,如果评审制度是由评审支持工具自动评价的,更是这样的。您是否花费了很长的时间去评审一下更改?您的同行从您的代码中是否发现了很多错误(bug)?这将如何影响您下一步的性能评价?

评估报表不应用来对付开发人员,尤其是在面对结对评审员时。这一做法会严重破坏道德观。

制度对于流程评价来说非常重要,这反过来,又为流程改进提供了一个基础。但是制度也可以被用来做坏事。如果开发员相信制度是用来对付他们的,那么他们不光是对流程有敌意,而且他们的注意力可能转到改变制度,而不是编写更好的代码,和变得更有效率上。

管理员可以做很多事情,来解决这个问题。首先也是最重要的,他们必须要警惕这一点,并且必须确定代码开发者没有面临很多的压力,而“老大哥”问题必须每次都得到详细的检查。

制度应该用来评价流程的效率,或者流程更改的效果。记住,通常来说,最困难的代码是由最有经验的开发员处理的。这些代码,反过来,最有可能出问题,因此最难检查,也有可能发现最多的缺陷。因此,大量的缺陷很有可能是由复杂性,以及代码的分块性造成的,而不是代码开发者的能力造成的。

如果制度确实能够帮助一个管理员去发现一个问题,那么将某人踢出局可能会产生更多的问题。我们推荐管理员在解决相关问题时,要将一个小组当做整体来对待。所以最好不要召开专门的会议,因为开发员在解决特定的问题可能会有压力。相反,您可以通过一个每周状态会议,或者正常的程序来解决问题。

管理员必须不断地维持这样一个年头,即搜索缺陷是好的事情,而不是糟糕的,缺陷密度与开发员的能力并不是挂钩的。记住对一个团队来说,缺陷,尤其是团队成员所引入缺陷的数量不应该被回避,也不应该用作能力的评价参数。

 

10. 评审一部分的代码,就算您不能全部完成,以从自我效能感(Ego Effect)中获益

想象一下您自己坐在编译器的前面,任务是需要修复一个小小的错误(bug)。但是您知道只要您说出了“我完成了”,您的同行 — 或者更糟,您的老板 — 就要检查您的工作了。这会改变您的开发个性吗?所以在您工作时,一般是在您声明代码评审完成之前,就会更加的谨慎了。如此您立即就会成为一个更好的开发员了,因为在您背后别人议论您时就会说,“他的员工非常谨慎,他真是一个不错的开发员”;而不是“他犯了大量愚蠢的错误(bug)。当他说工作完成时,实际上还差着远呢”。

自我效能感(Ego Effect)会促使开发员编写更好的代码,因为他们知道其他人将会查看自己编写的代码及作品。没有人想被其他人认为自己经常犯初级的错误(bug)。Ego Effect 促使开发员在向其他人交付作品时更加谨慎地进行评审。

Ego Effect 的一个良好特征,是不管评审者要对所有的代码变更负责,还是仅仅执行“点检查”,就像随机性的药物测试一样,都能正常地发挥作用。如果您的代码有三分之一的几率被评审者抽中进行评审,那么它仍然足以刺激评审者谨慎工作。如果您只有十分之一的概率被抽中检查,那么可能您就不会如此勤奋了。您知道您会说,“哈,我很少犯错”。

评审 20–33% 的代码时,从 Ego Effect 中获得花费时间方面的收益可能最大,评审 20% 的代码肯定要比不评审强很多。

 

11. 采用轻量级,工具支持的代码评审

代码评审一般有些主要的类型和无数的变数,而指南却能适用它们中的任何一个。但是,为了完全优化团队花在评审之上的时间,我们要使用工具支持的轻量级评审过程来得到最优的结果。搜索缺席时,它是有效的,实用的。

规范,或者 重量级的 检查已经流行了 30 年。它已经不是评审代码的有效形式了。重量级检查平均花费的时间是每 200 行代码 9 个小时。尽管它很有效,但是严格的过程需要三到六个参与者,并进行一系列繁琐的会议以讨论具体的细节。不幸的是,尽管需要繁琐的过程,但是大多数的公司没有条件将编程人员集成起来,进行长时间的会议。最近的几年,许多开发公司已经完全放弃了会议安排,纸质代码阅读,以及繁琐的作品收集工作,转而采用新型 轻量级 过程,以从规范的会议及老式重量级过程的重压中解放起来。

我们使用在 Cisco 中的案例研究,来确定轻量级技术与规范过程比较的特点。结果显示轻量级代码评审所需要的时间只是规范评审的五分之一(甚至更少),而且前者能够发现更多的错误(bug)。

尽管轻量级代码评审拥有很多的方法,例如实时评审和电子邮件评审,但是最有效的评审方法还是使用协作性的软件工具来促进评审,这些软件工具例如 SmartBear 的 CodeCollaborator(见于图 4)。

图 4. Cisco 研究中所使用到的轻量级代码评审工具,CodeCollaborator

CodeCollaborator diff 查看器与注释

图 4 的大图

CodeCollaborator 是与 IBM® Rational Team Concert 工作流程相集成的唯一代码评审工具。它将源代码评审与聊天形式的协作集成起来,从而使开发员从联系注释与私人代码行的繁琐活动中解放了出来。当程序员向工作项添加更改项进行评审时,在 CodeCollaborator 中将会自动创建评审,并分配适当的批准者。团队成员可以直接注释代码,与代码开发者聊天,并就每一个问题进行协作,追踪错误(bug)并修复缺陷。整个过程不需要会议,打印,或者安排日程。

有了基于 Rational Team Concert 与 CodeCollaborator 的轻量级评审过程,团队就可以进行更有效的评审,并实现代码评审的有利点。

posted on 2015-08-14 13:50  追求卓越,厚积薄发  阅读(569)  评论(0编辑  收藏  举报

导航