janlay’s blog

悠悠人生路,翩翩少年情

千里之行始于足下,谈谈 Code Review

今天来说说 Code Review(代码审查,以下简称为 CR)。

在一支自认为还算正规的研发团队中,CR 应该是值得做而且必须要做的环节。下面是维基百科对 Code Review 的定义

是指对计算机源代码系统化地审查,常用软件同行评审的方式进行,其目的是在找出及修正在软件开发初期未发现的错误,提升软件质量及开发者的技术。

CR 的形式

CR 有多种形式,结对 CR(两人坐在一起讨论)、多人 CR(一对多);既有在线进行(借助 Review Board 之类的软件)也有离线进行(一伙人坐在一起讨论)的。

个人建议,在可能的情况下,应尽量选择多人坐在一起 CR,这里称之为「集体 CR」。借用一个时髦概念,集体 CR 可以说是 O2O CR,从线上到线下,哈哈。先看看为什么要进行 CR:

  • CR 是保证研发质量的重要手段(这是不言而喻的)
  • CR 帮助提升成员专业能力
  • CR 改善研发团队沟通交流

显然,集体 CR 形式,对后两者的促进作用尤为明显。代码开发完了,彼此相熟的团队成员一起坐下来,对大屏幕上某人的作品评头论足。Code wins arguments, 技术人员更容易因代码产生话题。讨论下来,我们在收获良好团队氛围的同时,代码质量和个人能力方面也获得提升。集体 CR 让三方受益,何乐而不为呢?

CR 如何做?

每个人、每个团队可能都会有自己的一套方法。我认为这方面没有条条框框,黑猫白猫,能发现问题的就是好猫。这里试着将我带团队以来的 CR 经验,梳理脉络,由浅入深分享一下。

0. 重在参与:认真 CR 是对别人也是对自己的尊重

无论是线上还是线下的 CR,一对一还是一对多,全情投入,才可能发现隐藏的问题,才对得起别人发起的 Review 请求。教学相长,CR 亦然。CR 过程中难免会遇到拿不准的问题,不能误人子弟啊,进行一番彻查验证再抛出观点,这样的 CR 既能帮助别人,又巩固了自己的知识领域。

1. 代码风格:好的代码首先是错落有致的代码

什么是错落有致的代码?这里不谈团队编码规范,因为它不影响代码的观赏性。流程控制 (flow control) 和逻辑块 (logic block) 总是存在于我们的代码中,所以正常的代码都会有缩进和换行。干净、有效的缩进和换行意味着清晰的流程控制和逻辑单元划分,呈现出来可能是这种效果:

alipay-home

上图是支付宝首页底部一段 JavaScript 代码,看上去还不错。如果代码量比较多,通过 Sublime Text 2 提供的 Minimap 可以快速观察出代码是否「错落有致」:

alipay-home

tips: 如果你的编辑器没有这个功能,打开代码后快速上下滚动,基本上也能判断出代码是否错落有致 :-)

反之,如果代码看上去凌乱不堪,往往意思低质量——低设计或低实现,以及不佳代码组织。

2. 正确做事:代码的基本要求

毋庸置疑,交付的代码必须能:

  1. 正确实现业务逻辑
  2. 尽量确保没有明显的性能和安全问题。

要识别代码是否正确实现了业务逻辑,CR 人员必须了解代码涉及的业务。一个简单的方法是,先让代码实现者把业务快速串讲一遍,然后结合到将要 Review 的代码分部分介绍相应的业务,这样可以帮助 CR 人员快速理解业务,才能进一步对业务逻辑实现的正确性进行评判。

在保证正确实现业务逻辑之后,还要关注实现的性能和安全性如何。不幸的是,几乎无法保证代码不存在性能或安全问题,这是人的弱点。除了 CR 人员的经验可以起到一些作用外,我们还可以借助机器(自动化工具)对来弥补人的问题。各种 Profiling 工具、自动化测试、代码扫描工具可以帮助我们发现肉眼很难观察到的潜在问题。

3. 编码原则:围绕可维护性进行 Review

完成前面几条之后,CR 过程可以打 60 分。这个分数达到了 CR 的基本要求,也许过程中大家能学习到一些东西,但没有「升华」——如何在看上去 OK 的实现中发现更多可以学习、值得改进的点,在业务中学习提高,是每个 CR 参与同学的内心的初衷。抓住这个点进行深入讨论分析,CR 过程就能获得突破。

「升华」、「学习提高」都是比较虚的字眼,如何落地实处去做呢?既然我们已经完成了代码观赏性和交付必须的正确性讨论,那么让我们把眼光稍微放长,关注一下代码的可维护性。这是更高的要求,需要 CR 同学能发现未来修改代码时可能会面对的「痛点」。通俗地说就是,代码好不好改。有几个方面可以考虑:

  1. 现有代码是否需要进一步调整优化
    • 目前运行没有问题,但是再改会比较麻烦
    • 现有代码是否有存在歧义的地方
    • 现有的组织方式是否有优化空间
  2. 调整优化是否值得去做

    修改带来风险,原因是受限于交付时间点,或修改带来的实现、自测等不确定因素。这是个平衡的问题,需要评估改和不改、以及改到何种程度与修改带来的风险,哪一个更值得去做。

  3. 未来业务逻辑会如何变化

    这是下面要讨论的问题。

4. 回归业务:考虑业务发展的代码更有生命力

从上面一条开始,我们就走上了优化改进的不归路。我把回归业务放到最后一条,也把它当作是 CR 的终极方向。

为什么代码的优化改进要回归到业务?我认为这是代码作为产品实现者的价值体现,这世界上不缺精彩绝伦的好代码,但是缺少好的产品。为什么这么说呢?因为再好的代码都是静态的,它就那里,一是一,二是二,它的逻辑永远不变,比如各种查找、排序算法。但是产品会变,业务逻辑会变,作为实现者的代码就需要变。代码的优化只有面向业务,才有可能在一段时间内轻松应对业务上的变化。

如何回归到业务呢?深入了解产品设计的背景、要解决问题、当下面临的问题,以及未来的发展思路。掌握这些现状和趋势,能帮助我们站在实现者的角度,去判断当下的代码需要如何调整以适应未来的业务变化。这里举个例子说明一下。

L 君为数据仓库某产品做了一个牛逼轰轰的可拖放的树状结点功能。CR 下来问题可圈可点,实现质量总体还算比较高。

但是跳出这个功能本身的需求,我们发现它的实现偏离了「产出一套可复用的树状结点组件,将来应用在其他的系统中」这个发展思路。比如,L 君在实现场景中的功能点时,直接操作 DOM 元素,而这些 DOM 元素是与当前场景直接相关的。这样,未来移植这个功能的时候,必然涉及到 DOM 及其相关的 CSS 迁移,预期工作量会比较大,而且会有困难。

如何满足未来的业务需要呢?可以将组件功能点以 API 的形式向外提供,DOM 元素的管理完全在组件内部实现,并依赖一个可控的 CSS. 如此实现一个自包含的、可复用的组件。

结论

CR 很有必要,也要讲求方法层层递进。好的 CR 不仅帮助改善产品质量,还能提升参与者对产品和技术的理解,更通过沟通改善团队氛围——于己于人、于业务方、于老板都是非常好的事情。

是不是有一点点动心?千里之行始于足下,现在就准备实践 Code Review 吧!

Comments