代码审查最佳实践

jopen 11年前

  代码审查是软件开发过程中的必要步骤,既可以帮助被审查者提到代码质量,又可以让审查者加深对产品的理解。软件架构师 Vlad Mihalcea 分享了自己的最佳实践。

  Vlad 认为,代码审查不是测试。代码审查是开发者与开发者之间的事情,它跟测试没有关系。代码审查要审查开发者是否最明晰的实现了任务需求。

你不必告诉审查者审查什么:正如你不必告诉测试者测试什么,你也不需要告诉你的同事审查什么。同事审查的魅力来自于你的同事从他自己的角度对当下任务设计和实施的审查。两个人的智慧总是多于一个人的智慧的。

你需要及时检查所有改变:任何地方都可能有 bug,你需要仔细寻找它们。你需要审视所有变化来获得对整体的认识。

  需求第一,Vlad 强调,需求是最重要的驱动力。

用户是为了他的需求买单的。如果当下的改变不能满足需求,你需要重新开启这个议题。如果你发现了需要重构的代码,你需要创建新的议题而不是重开启现有议题。“单一职责原则”适用于任务,也适用于代码。

  Vlad 还建议采用一对多活动:如果你不确定自己是否理解修改代码的意图,可以让别人也来进行审查,这样更安全。同时,这是一种学习方式:代码审查是一项很棒的学 习工具,特别是运行大项目时。理想状态下你要对你项目的每个部分都很熟悉,但当项目很大时这不大现实,通过代码审查你至少可以对复杂模块有所了解。

  由于存在很多的实践方式,所以开发团队经常搞不清楚它的最佳实践是什么样的,架构师 Lorinda Brandon 尝试从多个方面来分析了正确的代码审查时间应该重点考虑的几个因素。

  • 人员结构——在你的团队中有多少同事拥有足够的专业技能来担当合格的代码审查者?作者曾经和多个开发团队沟通过,这些团队都声称本身只有一个资深 的开发人员,如果让他来负责所有的代码审查工作,那么团队的核心开发就无法开展了。作者也遇到过类似的情况。在一个小规模团队里,资深的同事总是忙不过 来,因为核心的工作都在等着他做。
  • 地理位置——你的团队成员是如何分布的?他们是否是分散在世界各地还是坐在同一个敞亮的办公室里? 代码审查需要精力的专注和反馈,如果开发人员能够面对面的沟通,那么审查工作会便于实施。而物理隔离的远程团队很难达到代码审查的满意结果。
  • 所在领域——你所在的 IT 领域需要遵守怎样的规则和约束呢?如果你在一个受到严格监管的行业,那么你必须遵循有关审计和报告的规则,这意味着你必须想办法跟踪代码审查的频率和质量。如果所在的领域把安全性作为重点,那么可能在代码审查过程中要引入安全审计。
  • 复杂性——你的代码复杂性如何?在代码审查时,需要多个不同专业技能的审查者吗?例如,游戏开发可能会引入非常复杂的业务逻辑来处理文字交互和场景跟踪,同时还需要特定的动画处理和内存管理技术。在审查如此复杂的代码时,应该引入多个审查者从不同角度来检阅代码的质量。

  坚果云开发团队曾经在“月光博客”上撰文分享高效代码审查的十个经验。

  代码审查首先要求团队有良好的文化,同时谨慎的使用审查中问题的发现率作为考评标准

团队需要认识到代码审查是为了提高整个团队的能力,而不是针对个体设置的检查“关卡”。“A的代码有个 bug 被B发现,所以A能力不行,B能力更好”,这一类的陷阱很容易被扩散从而影响团队内部的协作,因此需要避免。另外,代码审查本身可以提高开发者的能力,让 其从自身犯过的错误中学习,从他人的思路中学习。如果开发者对这个流程有抵触或者反感,这个目的就达不到。

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

  代码审查需要带着问题去做,并且让原作者对发现的问题进行确认:

我们在每次代码审查中,要求审查者利用自身的经验先思考可能会碰到的问题,然后通过审查工作验证这些问题是否已经解决。一个窍门是,从用户可见的功 能出发,假设一个比较复杂的使用场景,在代码阅读中验证这个使用场景是否能够正确工作。使用这个技巧,可以让审查者有代入感,真正的沉浸入代码中,提高效 率。大家都知道看武侠小说不容易瞌睡,而看专业书容易瞌睡,原因就是武侠小说更容易产生代入感。有的研究建议每次树立目标,控制单位时间内审核的代码数 量。这个方法在我们的实践中显得很机械和流程化,不如上面的方法效果好。

如果在审查中发现问题,务必由原作者进行确认。这样做有两个目的:

  • 确认问题确实存在,保证问题被解决
  • 让原作者了解问题和不足,帮助其成长

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

来自: InfoQ