17

Code Review 怎么做

 3 years ago
source link: https://xie.infoq.cn/article/c602fe972a01e7953bbcaaeb2
Go to the source link to view the article. You can view the picture content, updated content and better typesetting reading experience. If the link is broken, please click the button below to view the snapshot at that time.

Code Review现在基本成为了一种共识,再忙的团队,也会抽出时间来做Code Review。至于怎么做Code Review,每个团队的情况都可能不太一样。甚至每个人的习惯都不太一样。不得不承认的是,Code Review做得好不好和Reviewer个人的水平有很大的关系。一个好的Reviewer,反馈意见又快又好,而且反馈的意见一针见血,直至代码的最核心问题。

看到一个MR之后,我脑袋里最开始出现的几个问题是:

  1. 代码是否足够简单,是否容易阅读,是否容易理解

  2. 代码有没有陌生的或者不符合常人思维的概念

  3. 代码的排版是否符合编程规范,风格是否和现有代码一致

  4. 代码是否容易测试,是否有相应的测试代码

  5. 代码是否有明显的重复,是否拷贝了其他模块的代码

上面5个问题有了答案之后,基本上可以对这份代码有个初步的判断了。如果都没有大的问题,说明这份代码是经过深思熟虑的,是用心打磨过的,不是随意提交的代码。那么,我们可以把更多的精力放到更深层次的问题。深层次的问题,分为宏观和微观两个方面入手。

宏观上,我们主要关注以下几个方面:

  1. 新增的代码和已有的架构设计是否一致。做一个新的特性,不仅仅要看这个特性本身是否能够基于现有的架构里实现,还要看是否突破了原有设计的边界和破坏了原有的设计原则。破坏远比建设来得容易,架构能否适应业务的发展,也是需要持续演进,但是我们要知道演进的方向和原则,控制系统的边界。

  2. 上下文的切分是否合理。特性的粒度或大或小,上下文的切分都同等重要。切分上下文既是为了隔离变化,也是为了保留可拓展性。不合理的上下文切分,往往要在业务发生变化的时候要付出数倍的代价。有的时候,为了重新划分上下文,与其重构,不如重写。

  3. 依赖是否正交。切分上下文是从模块划分的角度分离业务,我们还是需要分层的视角来进一步隔离变化。对于复杂的业务,分层的思想很重要。一个复杂的问题,可以通过分层来分解成几个更容易解决的问题,同时问题也变得更容易理解。方案的理解成本更低,才可能写出更容易理解的代码。

如果一个MR,宏观上没有太大的问题,那么我们就可以集中精力好好审视和推敲微观的问题了。微观的问题一般比较琐碎,而且都是相对比较独立的问题,所有经验很重要。经验丰富的reviewer会有更出色的嗅觉,更灵敏地感觉到代码的坏味道。我一般是从以下几类常见的问题入手:

  1. 语言最佳实践。虽然现在的IDEA很强大了,但是还是会有不少最佳实践是无法通过IDEA自动识别出来的。比如Java里面的函数式的写法,try-with-resource,单例模式等。

  2. 算法的时间或者空间复杂度。大多数不是专门研究算法的工程师,对算法复杂度的敏感度是比较低的,大多数时候会忽略算法的复杂度。

  3. 设计模式的使用。对于一些有定式的问题,使用设计模式会大幅降低理解和沟通成本。如果不使用设计模式,理论上也是可以实现的。但是设计模式的好处在于,把更多的信息用代码表达出来,而且是形式化的代码。这些信息比文字、语言还要高效。

  4. 安全问题。安全问题平时关注度不高,一旦出现,代价高昂。比如常见的安全问题有:明文密码、SQL注入、shell注入、提权漏洞等等。系统性的安全问题,可以在设计阶段识别出来。代码实现的安全问题就比较隐蔽了,需要我们带着攻击者的思维去推演、挖掘。

以上就是我工作这几年总结的代码检视的思路。代码检视的质量取决于committer投入的时间和自身的水平。单个committer很难全面的发现问题,一个可以互相学习的committer群体对提高代码质量和团队水平至关重要。


About Joyk


Aggregate valuable and interesting links.
Joyk means Joy of geeK