同行代码审查的实战经验
2015-04-02来源:

数百万年前,猿猴从树上下来,进化出了对生拇指,最终进化为人类。

我们在强制代码审查上面看到了相似的曙光:它就像是在软件开发大草原上将人和野兽区分开来的东西。

尽管如此,我还是不时地听到团队中有这样的评论:

“在这个项目上进行代码审查是浪费时间。”

“我没有时间来做代码审查。”

“发布被延期了,因为我那卑鄙的同事还没有审查我的代码。”

“你能相信吗我的同事竟然想让我修改我的部分代码?请解释给他们听,如果我这极好的原始代码以任何方式被修改了,那么宇宙的微妙平衡就会被打破。”

我们为什么要做代码审查?

首先,让我们牢记为什么要做代码审查。所有专业的软件开发人员最重要的目标之一就是不断地提高工作的质量。即使你的团队里满满都是优秀的程序员,如果你们不能作为一个团队而协作,那么你们在一名能干的自由职业者面前也没有优势可言。代码审查是达到这一目标最重要的途径之一。特别是,代码审查:

提供了另一双眼睛去发现缺陷和更好的做事方法。

保证至少有另外一个人熟悉你的代码。

通过让新员工接触更富有经验的程序员的代码来帮助培养他们。

同时让审查者和被审查者接触他人的好想法和实践经验来促进知识共享。

激励开发人员在工作中更加认真,因为他们知道他们的代码会被另一个同事审查。

进行全面的代码审查

然而,除非在审查中投入恰当的时间和精力,否则无法达到这些目标。只是滚动浏览一遍代码,确保缩进是正确的,所有的变量都使用了小驼峰式命名法,这并不能构成一次全面的代码审查。将结对编程看作是代码审查花销的基准是有益的,结对编程是一种颇为流行的做法,它会将所有的开发时间增加100%的额外开销。 你可以花费大量的时间在代码审查上却依然比结对编程花费更少的总工程师时间。

我认为原始开发时间的25%左右的时间应该被用于代码审查。比如,如果一名开发人员花费两天实现一个功能点,审查者应该花费大约四小时对其做代码审查。

当然,只要正确的完成了审查,具体花费多长时间在审查上并不是最重要的。特别是,你必须理解你正在审查的代码。这并不仅仅表示你要了解代码所用编程语言的语法规则,它意味着你必须明白这段代码是如何融入应用程序环境的,不论是组件还是以库文件的形式。如果你没有领会每一行代码的意思,那么你的审查将不会特别有用。这就是为什么好的代码审查无法很快地完成:需要花费时间来查看各种可以触发特定函数的代码分支,确保第三方API被正确地使用(包括任何边界情况),等等。

除了查找审查代码中的缺陷和其他问题之外,你应该确保:

包含了所有必要的测试。

写了合适的设计文档。

即使是擅于写测试和文档的开发者也不总能记得修改了代码之后更新它们。在适当的时候,来自代码审查者的小小提醒对于保证测试和文档不会随时间而过时是至关重要的。

避免过度地审查代码

如果你的团队进行强制的代码审查,风险是代码审查将会积压到无法管理的地步。如果你两周不做任何代码审查,将很容易就需要花费数天来赶上进度,这意味着当你最终决定处理它们的时候,你自己的开发工作将会受到巨大的意想不到的影响。这种情况下保持代码审查的质量会尤为困难,因为恰当的代码审查需要强烈持续的脑力劳动,这很难持续不断地进行数天。

基于这个原因,开发者们应该尽量每天完成积压的审查工作。一种方法是将审查作为上午第一件事来做,在开始自己的开发工作之前完成所有审查,你可以避免审查落入失控的情况。一些人可能倾向于在午休前后或者一天结束时做审查。无论何时做审查,通过将代码审查视为你日常工作的一部分而不是一件使人分心的事,你避免了:

没有时间去处理积压的审查工作。

因为你的审查还没有完成而耽搁产品发布。

发布已经过时的审查结果,因为在此期间代码已经大大修改了。

进行粗劣的审查,因为你不得不在最后时刻仓促完成。

编写可审查的代码

审查者并不总是造成失控的审查积压工作的唯一原因。如果我的同事花费一周在一个大项目里乱糟糟地添加代码,那么他们发布的代码补丁将会非常难审查。在一个审查步骤上将会有太多工作要做,理解代码的目的和隐含的架构将会非常困难。

因此,将工作划分为可管理的小单元非常重要。我们使用Scrum方法论,所以对我们来说,用例就是合适的单元。尽量用用例来组织我们的工作,提交只属于我们当前在处理的用例的审查,这样我们写的代码将会非常容易审查。你的团队可以使用其他的方法论,但原理是一样的。

编写可审查的代码还有一些其他的先决条件。如果要做出复杂的代码架构决策,提前和审查者见面讨论一下将十分有用,这将使审查者能够比较容易理解你的代码,因为他们将明白你要达到什么目标,以及你计划如何达到它。这同时能够帮助避免当审查者建议了另一种更好的方法时你不得不重写大块代码的情况。

项目的架构应该在你的设计文档中进行详细的描述,无论如何这都是很重要的,因为它可以使新的项目成员熟悉了解新项目的详细情况并且理解已有的代码库。另外的好处是能够帮助审查者进行恰当的代码审查。单元测试同样有助于向审查者说明组件应该如何被使用。

如果你的补丁里包含了第三方代码,将它们单独提交。如果你的代码中间突然跳出9000行的jQuery代码,这将会非常难进行恰当的代码审查。

编写可审查代码的其中一个非常重要的步骤是将自己的代码审查进行注释,即你自己审查代码,并在任何你认为能够帮助审查者理解这里是怎么回事的地方添加注释。我发现给代码添加注释只花费相对较少的时间(通常只几分钟),却在代码审查的速度和质量上产生巨大差别。当然,代码的注释有许多同样的好处,并且应该被恰当使用,但通常评审注释更有意义。另外,研究表明当重读和审查自己的代码时,开发者能够发现许多缺陷。

大型代码重构

有时,有必要用一种会影响许多组件的方式来重构一个代码库。就一个大的应用程序来说,这会花费数天(或者更久),产生一个很大的补丁。在这种情况下进行标准的代码审查可能是不现实的。

最好的解决方案是增量式地重构代码。在合理范围内找出部分的代码改动,它能够使代码库正常工作,并且是朝向你想要达到的目标方向的改变。一旦这个修改完成了并且审查也通过了,进行下一步增量式的修改,如此等等直到全部重构完成。这种方法也许并不总是可行的,但通过思考和计划,通常能够避免在重构时产生巨大的整体一块的补丁。用这种方式,对于开发者来说可能会花费更多的时间来进行重构,但也提高了代码质量,也使审查变得容易多了。

如果实在无法进行增量式代码重构(可能说明原始代码的编写和组织有多好),一种解决方法大概是在进行重构时采取结对编程的方式来替代代码审查。

解决分歧

你的团队无疑是由聪明的专业人士组成的,并在几乎所有情况下,当在某个编码问题上发生意见分歧时应该可以达成一致的意见。当你的审查者倾向于另一种不同的方法时,作为一名开发者,要保持开放的态度,并准备作出妥协。不要对你的代码有种专有的态度,也不要认为审查意见是针对个人的。仅仅因为有人觉得你应该将一些重复的代码重构为可重用的函数并不意味着你就不是一个有吸引力的、风趣的、聪明的、迷人的人了。

作为一个审查者,要机智些。在建议修改之前,考虑好你的建议是否真的是更好的,还是只是编码习惯不同的问题。如果你专注于解决那些原始代码明显需要改善的部分,你会做得更好。要像这样对开发者说话:“这可能是值得考虑的……”或“有些人建议……”,而不是“我的宠物仓鼠都可以写一个比这个更高效的排序算法”。

如果你们实在无法达成一致意见,再去找一个你们两个都尊重的开发者来看一看,给出他的意见。

更多信息请查看IT技术专栏

推荐信息
Baidu
map