关于代码评审(Code Review)的文章也算是汗牛充栋了,代码评审也已经是许多组织的标准化实践。不过,在许多团队在尝试代码评审实践时,却有如下的疑问:
- “政治正确”的代码评审活动究竟有没有达到期望的实际效果?
- 给了我一大堆代码,到底该从哪里看起?哪些方面是我该评审的?哪些不是?
- 别人有没有认真评审我的代码?如何让别人更容易的评审代码?
这些问题都不是什么新问题,但是它是如此的普遍,而且经年累月的在不同的上下文中被提起,不外乎两个方面:
1, 理解代码评审的核心目标,建立关于代码评审的正确预期。
2,了解代码评审为什么可能无效,并采取有针对性的实践来提升代码评审的效果。
为什么要做代码评审
不少同学认为代码评审就是用来查错的,甚至希望用代码的缺陷数量来检验代码评审的效果。这低估了代码评审的价值。代码评审最本质的作用不是问题发现。除了代码评审,我们有更多更好的手段来发现问题。代码评审的作用更多是关于社会学的,是一种长期行为和组织文化。
CR是代码规范性的保证
编码者视角:良性的社交压力
你正在紧张的编码,交付时间迫在眉睫。你的组织对代码的单元测试有一个要求:凡是新增的代码,必须有完整的自动化单元测试。但是,这压力之下,你想给自己降低一点要求,不写这部分待的单元测试了,以后再编写吧,或者为了应付工具的覆盖率要求,先写一点不那么有用但是却能带来覆盖率的测试(例如没有断言的测试)。
但是,一旦想到你的代码发出去将会有你的同事做Review,有没有为刚才的这种想法产生一丝丝压力?这种压力是良性的,它能给你带来一种即时的反馈,阻止你去选择那些短期收益、长期损失的“投机”行为。如果没有代码评审这个环节,或许你就会真的“为所欲为”了,其实最终还是要为这种取巧行为埋单。
维护者视角:代码可读性的保证
有许多方式能实现同一个软件需求。有兴趣的读者可自行搜索“Hello World的N种写法”。 尽管条条大路通罗马,但是,不同的道路代价是不一样的。小到变量命名,大到设计结构,如果你采用的是一种不那么常见的做法,往往就是给给后来的代码维护者挖坑。这种维护活动可能发生在1个月以后,也可能发生在1年以后,甚至是更久之后。甚至那时候,作为作者的你,已经不在这个团队了,已经没有人能理解当时的软件为什么这样设计。
代码评审强制提前了这个反馈周期,代码编写完成之后,就立即有了一位或多位读者,他们是这个代码的Reviewer。所以,这段代码已经在编码完成之后,立即经历了可读性的检验。更理想地,如果组织已经有了编码规范和设计规范,还能确保这段代码遵循了这些规范。如果这时候发现这段代码没有遵循规范,那更是好事,它指向了CR的另外一个关键价值:知识传播。
CR带来了知识传播和设计共识
你可能是一个团队的Leader,正在为如何提升团队成员的编程能力发愁。你希望他们去读书,所以你介绍了诸如《整洁代码》之类的入门书籍,你还介绍了经典名著《设计模式》,还推荐了《领域驱动设计》。你也希望团队成员能理解产品的业务逻辑,所以希望团队成员周期性的分享进行业务分享。
所有这些努力都很好。但是,也有可能你会被打击。一个月过去了,似乎团队成员对命名规范都建立了概念,但是这怎么命名这件事上,大家并未形成共识。不少同学已经了解了一些设计模式,但是有人过度运用模式,搞的代码臃肿不堪,有人则只知道singleton。团队成员为什么是实体对象,什么是值对象争的不可开交,没有人说得清楚聚合是什么,应该什么场景下适用。
你真正缺乏的,是一个场景。抽象的概念如果不落到具体的事情上,就很难形成共识。有人或许知道海洋法系的“判例”,这是这法律层面形成共识的一种非常好的方法。代码评审,其实也是这形成判例:哪一类设计是合理的,哪一类设计是不合理的。通过针对具体的问题进行分析,团队就会逐渐形成设计共识,在过程中,对这些共识不那么熟悉的新同学,也可以慢慢融入。
当然,CR也能用来检验逻辑正确性
保证代码逻辑正确,是设计者的责任
为了不让CR被滥用并被寄予过高期望,我们在此首先申明一点:保证代码的逻辑正确,是设计者的责任。 代码出现来一个空指针错误,究竟是编码做的不好,是CR做的不好,还是测试做的不好?那首先肯定是代码作者制造这个问题。把这个板子打在Reviewer身上公平吗?或许,Reviewer确实有责任发现这样的问题,但是,如果代码本来就错误多多呢?如果一次性Review了1000行代码,根本看不过来呢?我能找到一大把的理由,来说明为什么漏掉这么一个空指针错误。
发现逻辑错误的其他方法
你还有许多其他的方法来发现错误,它们的成本往往并不高,例如:
- 编写自动化单元测试
- 使用代码静态检查工具
无论是否存在CR活动,上述两点都是一名专业的开发者和开发组织应该大力倡导的行为。
代码评审确实也有错误发现的价值
在上述两点的前提下,代码评审确实也应该用于发现错误-它本质上建立来一种冗余机制,通过多人来工作在同一段代码上,发现代码中可能发生的认知错误(这对于单个开发者往往是很难发现的)以及疏忽。
高效高质的代码评审
哪些因素阻碍了代码评审的效果
代码评审本身并不困难,但是,如果考虑到如下因素,可能就比较复杂了:
- 你可能对要评审对代码的设计上下文一无所知
- 你可能非常忙碌
- 你一下子收到了几千行需要被评审的代码
- ...
实际操作建议
小批量:每次Review的代码量要少
研究发现, 成功的CR活动一定是小规模的。 例如,《Modern Code Review: A Case at Google》论文介绍说, Google的CR活动中,有35%的CR仅仅修改了一个文件,90%的CR修改的文件数在10个文件以内,甚至有10%的CR仅仅修改了1行代码。
代码量少的好处显而易见:修改在哪里非常清晰,问题也会一目了然。一次推给别人1000+行代码,还想得到有价值的Review,可能性微乎其微。
当然,一次Review它代表的功能应该是有意义的,是完整的,如果不是修复缺陷,这一定程度上也对开发者迭代地开发功能的能力提出了要求。
多批次:Review要频繁发生
小批量必然导致了多批次。在微软2013年的一篇论文《iExpectations, Outcomes, and Challenges Of Modern Code Review》和前述的Google的论文中都提到了频繁Review的做法。其中,Google 的每周每Developer的代码变更中位数是3个,每周每Reviewer的Review中位数是4个。
快速响应
当每次Review的粒度不大,Review又比较频繁时,快速响应才能成为可能,也是必然的要求。在这个数据上,Google的中位数是4小时。这个指标可以成为一个较好的参照。
找对人:合适的Reviewer
谁适合Review你的代码?选一个和被Review的代码毫不相干的人肯定是不明智的。下面列出了一些潜在的候选人:
- 如果你的组织有Owner机制,Owner应该是合适人选
- 和你工作在相同上下文的同事
- 近期修改过相同代码的同事
- 比你更资深的程序员,希望得到他们的专业反馈
现在已经有一些工具,能够根据上下文推荐Reviewer,这也为选择合适的Reviewer提供了便利。
合适的工具
快速响应、高质量的Review离不开现代工具。现代的Review工具能自动集成进工作流,高亮变化,甚至能自动汇总变更。这方面已经有许多现代的工具可以使用,还没有选好工具的读者,可以自行google搜索。
考虑结对编程
当我们提到“小批量、多批次、快速反馈“的时候,如果有过结对编程经验的同学,马上就会反映过来,这就是一种试图接近结对编程的形式。
结对编程,共同编程的两位同学拥有完全相同的上下文,不存在上下文切换的烦恼,没有缺乏时间的烦恼,不需要借助额外的工具,反馈随时随地。事实上,在我的眼中,结对编程才是最好的Code Review。
综合在线Review和线下Review
在线Review应该是常态化的行为。考虑到CR的”知识传播“价值,线下Review是有益的补充。有经验的团队,会周期或者不定期的组织线下Review,这样能获得比在线Review更为广泛的知识传播面,也能引起更为热烈的讨论和辩论,有助于形成更高质量的共识。
一些可以用来数字化的CR指标
研发行为的全面数字化,带来了一些有价值的数据洞察。如果工具支持,可以通过一些指标的观测,持续推进CR活动。我们把一些建议的指标和数据总结如下:
从Author角度:
- 单次变更的代码行数 (主要指标)
- 变更的频度 (参考)
从Reviewer角度:
- 响应时间
- 评论数和拒绝率
从Reviewer的团体角度,还可以发现Author-Reviewer之间的社群关系,也是一种有价值的了解知识传播的信息。
不要做什么:
CR的本质是文化建设,强烈建议仅仅把CR的指标用作提升指引,而不要用于和绩效有关的评价。无论是前述的几种指标,还是和Review的质量甚至是缺陷相关的数据。
原文链接
本文为阿里云原创内容,未经允许不得转载。