如何做有效的Code Review?我有这些建议
代码评审(Code review)是保证代码质量的一种有效手段,做得好的话,对公司来讲是一笔收益颇高的时间投资。但实践起来往往变成了炫耀编程技能、固执己见、恶言相向、同事关系恶化的事,这该如何是好?
往往代码评审过程中,评审者(Reviewer)往往会过于关心旁枝末节,而忽视主要问题,也就是所谓的自行车棚效应。在批准价值百亿的核电站的建设提案中,专家们往往会浪费大量时间纠结于厂内自行车棚(bikeshed)的颜色;因为核电站太大、太复杂,“专家们”未必真懂,但总不能不说话啊,那就从无关痛痒的自行车棚挑毛病吧。
有效的代码评审
代码评审是开发人员编写的代码由另一个人检查以查找缺陷和改进的过程。换句话说,开发人员大部分都是独立编写代码的,当代码完成之后,他们会召集一次评审。
代码评审是提高软件质量的有效途径。在Google,所有代码都要经过同行评审。引用《 代码大全(第2版)》中的几个例子:
- IBM 的 Orbit 项目有 50 万行代码,使用了 11 级的检查。它提前交付,并且只有通常预期错误的百分之一。
- 一份对 AT&T 的一个 200 多人组织的研究报告显示,在引入评审后,生产率提高了14%,缺陷减少了90%。
- 喷气推进实验室估计,通过早期发现和修复缺陷,每次评审节省约 25,000 美元。
然而,不少团队在有效的代码评审争论中,失去了原本的效益。在功能失调的团队和组织中,对所有参与者来说它可以迅速变成一个 令人不快的经历:
- 它成为评审人员来展示技能的平台。他们在别人的代码中指出“错误”,并强加自己没有价值的“意见”。
- 在代码完全就绪前,开发人员会非常抗拒别人审查他们的代码 ——可以说这可能是一件好事,但他没有真正理解评审的意义。
- 开发人员放弃代码所有权,并开始依赖他人查找问题。
在这篇文章中,我将讨论团队和组织可以做的一些事情,让代码评审为所有参与者带来愉快的体验。
对管理层的建议:营造健康文化
有效的代码评审,需要一个重视质量和卓越的健康文化。如果团队不以提供高质量的产品为信仰,代码评审将不会给您所期望的结果。你需要一个人人参与的积极文化—立足于建设性批评,智者胜。
除了创造一个健康文化,并允许花时间和资源进行评审,管理者在代码评审中应保持低姿态。大多数人不想在上司面前暴露自己的秘密,这已是一种文化。代码评审最好由同行进行,管理层不应该询问可以用来评价人的细节。的确有一些管理人员索要检查表和成绩,以便他们可以“衡量”并评价人。
可能你已经有一个健康的文化(算你幸运)。这还不够,营造健康的文化取决于许多因素(团队和组织内部)。这是非常具有挑战性的,没有灵丹妙药。没有正确的文化,代码评审不会带来期望的收获,甚至在极端情况下可能会适得其反。
对个人的建议:换位思考
Karl Wiegers 在他的《Peer Reviews in Software: A Practical Guide》中写道:
产品的作者与评审者之间的互动至关重要。作者必须足够信任和尊重评审者才能接受他们的意见。 同样,评审者必须尊重作者的才华和辛勤工作。评审者应谨慎选择他们用来提出问题的词汇,重点关注他们对产品的观察。说『我没有看到这些变量被初始化 』可能会引发建设性的反馈, 而『你没有初始化这些变量』可能会让作者非常不爽。
关注代码很容易,但不要忘记,桌子(或计算机)的另一端有一个人。他有主见有“自我”。请记住,解决问题的方式有很多。
- 要谦虚。我既见过非常高效的评审,也见过因为吹毛求疵而非常低效的评审。不要吹毛求疵!!!
- 确保您有编码标准。编码标准是在组织中共享的人人都认同的一套准则。如果你没有编码标准,那么不要让讨论变成一个比较编码风格的比赛(大括号 { 在同一行还是下一行!)如果你遇到这样的情况,请在编码标准论坛上离线讨论。
- 学会良好地沟通。你必须能够清楚地表达想法和理由。
- 编程策略是一个仁者见仁智者见智的问题。评审者和开发人员应该寻求理解彼此的观点,而不应该成为哲学辩论
对评审者的建议:谦虚
- 开发者不是冤大头。评审的目的不是证明谁是更优秀的程序员而是查找缺陷,并确保代码是简单和可维护的。
- 问问题。不要提出可能听起来带指责的要求或言论。例如,不要说:『你没有遵循标准XYZ』。更好的方式是真正寻求理解开发者的观点:『你对标准 XYZ 有什么看法,它是否适用于这里?』,这可以引到我的下一个观点。
- 避免『你为什么』,『你为什么不』风格的问题。它会使人对立。 『为什么把它声明为全局变量?』可以更好地表达为『我不明白为什么这里用一个全局变量』。寻找方法来简化代码。代码评审的目标之一是创建“可维护”软件。
- 记住要欣赏并感谢对方。人们经常忘记一句简单的『干得好』或『它看起来很棒』的影响有多大。
有些事情,如果看起来像排练过的或以讽刺的语气说出来,就不会奏效。像正常对话一样对待代码评审。你正在聆听他人,应该真正寻求理解他们的观点。需要时提供建议和提示。如果代码很棒,不要强迫找一些消极的来说。
对开发者的建议:它不是个人的事情
- 不要感情用事。记住,别人说的不是,是代码中的缺陷或不足,而不是你。
- 意识到你和你的代码是捆绑在一起是正常的事情。如果你为自己的工作感到自豪,那是一个很好的迹象,说明你是一个关心作品的人。
- 有适当的自我。足够信任和捍卫自己的观点,但又不至于盲目拒绝对方的好建议和意见。
- 人非圣贤孰能无过。评审者作为第二双眼,可以指出你可能忽略的事情。问题与具体建议一样有价值。
- 提问题要有针对性。『将所有这些类纳入它们自己的软件包中是否更有意义』
- 感谢审稿人的时间和他们可能提供的任何反馈。
总结
同行评审是人们互相交流,而无效(ineffectiveness)则源于社会学问题。然而,管理者花费大量时间在操心使用哪些惊艳的工具。工具会有所帮助,但只有工具并不会神奇地带来结果。立足于正确的文化同时…换位思考,以人为本! :-)