说说Code Review

过年回来后开始接手管理一个技术团队，这个团队去年一年基本上都在赶项目，晚上经常加班到晚上十点以后，所以代码质量不用多说大家也能想得到。前几天跟大家开会，我提出来从下个版本开始要做code review，有个同学问到「那不会浪费很多时间吗？」，我告诉大家「会花很多时间，但不会浪费很多时间」。因为这个团队里同学之前没做过code review，所以准备下周给大家做一下分享告诉大家应该怎样做，于是今天花点时间把脑子里的想法掏出来梳理梳理。

一、目的是什么

做任何事情都要有一个目的，那么我们做code review的目的是什么呢？本来开发工期就非常紧了，特别是身处中国这个大环境下的互联网公司，老板恨不得要你二十四小时连轴转，为什么还要花那么多时间去做code review呢？我认为code review的目的在于提升代码质量。

前几天看了篇文章，里面有这么一段对我触动很大：

在这种业务需求紧张的模式下，Facebook一些开源技术方案是如何产出的，是非业务团队专门做的么？
我想说的是即使业务需求紧张，也一样把代码好好写好，另外有牛逼的tech lead和严格的code review，总的质量也不是很差。国内有一点很不好：经常没有code review；而且技术人员观念不好，把要写的代码当差事，只要能完成能用就好。所以就越来越操。
(Code reivew一直是硅谷一线互联网公司的质量控制法宝，从Apple到Google，从Facebook到现在的Airbnb和Uber。可悲的是，国内的人都太聪明，觉得这东西没用繁琐，而且减慢开发速度。有时，我们就是太过聪明。）

所以我们不要总是拿没时间来当做借口，如果对代码质量没有一定的追求，给再多时间也是没用的。业务需求紧张需要通过提高工作效率来解决，而不是不花精力提高代码质量。另外，站在一个项目的生命周期来看，写烂代码真的会比写好代码花的时间更少么？

二、好代码最重要的特征是什么？

既然做code review的目的是提高代码质量了，那么什么样的代码才能算是好的代码呢？最开始这个标题我写的是「什么样的代码才是好代码？」，后来我想了下这个问题太大，我无法对「好代码」简单的下一个定义，真正讨论起来估计得单独写一篇文章了，所以先按住这个话题，换成简单的「好代码的最重要的特征是什么？」。

我觉着好代码最重要的特征是可读性强，这样才能让和你协作的同学以及未来的你自己能够不用想太多就能看得懂，毕竟花在维护代码上的时间要远远超过写这段代码花的时间。每新增一行代码就会多增加一份维护成本，而可读性强的代码可以把维护成本降到最低。

那么我们怎样来定义这个可读性强呢？每个人都有自己的标准，怎样才能在团队里让大家都认可呢？微博的一位工程师在他写的《关于烂代码的那些事》这样写到：

在很多跟代码质量有关的书里都强调了一个观点：程序首先是给人看的，其次才是能被机器执行，我也比较认同这个观点。在评价一段代码能不能让人看懂的时候，我习惯让作者把这段代码逐字翻译成中文，试着组成句子，之后把中文句子读给另一个人没有看过这段代码的人听，如果另一个人能听懂，那么这段代码的可读性基本就合格了。
用这种判断方式的原因很简单：其他人在理解一段代码的时候就是这么做的。阅读代码的人会一个词一个词的阅读，推断这句话的意思，如果仅靠句子无法理解，那么就需要联系上下文理解这句代码，如果简单的联系上下文也理解不了，可能还要掌握更多其它部分的细节来帮助推断。大部分情况下，理解一句代码在做什么需要联系的上下文越多，意味着代码的质量越差。
逐字翻译的好处是能让作者能轻易的发现那些只有自己知道的、没有体现在代码里的假设和可读性陷阱。无法从字面意义上翻译出原本意思的代码大多都是烂代码，比如“ms代表messageService“，或者“ms.proc()是发消息“，或者“tmp代表当前的文件”。

我很认可这个说法，在这个基础上，我一直坚持认为虽然一个能把一件事情描述清楚的人写的代码不一定可读性强，但是一个无法将一件事情描述清楚的人写出来的代码可读性肯定很差。

三、那么，该怎样做呢？

说了那么多了，具体怎样落地到现实工作中呢？即使按照前文所说的把代码逐字翻译成中文讲给其他同学听，也一样可能由于认知问题导致对方听不懂，比如你认为很基础的概念可能别人并不了解。所以我认为大家要遵循一些基本原则，这样才能有效的沟通。

3.1 SOLID原则
这是面向对象的五条基本原则，我列在下面，在这里就不展开来说了

• Single responsibility principle

• Open/closed principle

• Liskov substitution principle

• Interface segregation principle

• Dependency inversion principle

3.2 Don’t Repeat Yourself
一般对这条原则的理解是对于同样的功能不要直接copy原来的代码，而是要抽象出一个公用的方法。但是实际上对同样的功能用不同的思路或者代码去实现也是一种浪费。比如常见的日志处理、异常处理逻辑。

3.3 Prefer Composition to Inheritance
这条原则跟前面提到的OOP的SOLID原则里面的Interface segregation principle有点重合之处。随着业务需求的不断迭代，小的组件逐渐会演变成大的组件，在这个过程中驾驭的难度会逐步提升，而如果在不断迭代的过程中不断抽象出小的组件，则可以在业务功能复杂的同时保持代码的简洁。比如不管是飞机还是汽车火车都是会移动的，而我在使用时只需知道这个对象是可移动的即可，至于这个对象是飞机还是汽车我并不关心。

3.4 编码规范
这个不多说了，可以采用一些行业里优秀的编码规范。但是要注意的一点是规范的作用是保持项目编码风格的统一，不要在规范上做无意义的争论。

3.5 如果不具备抽象的能力，那就重复吧
这是一个比较残酷的也比较常见的现实，看了一大摞的书废了老大的劲终于抽象出了一个组件，但是最后的结果却是加大了维护成本。所以如果你觉着无法很好的去抽象，就直接用最粗鲁的重复代码吧，毕竟这样别人还能看得懂，比抽象出来后还要再写一大堆的if else好多了。

四、技术之外的tips

在技术之外还有一些要注意的点，首先最重要的就是要有一个开放的心态，review的是代码，而不是具体的人，不要因为对方的review而感觉羞耻，当然也不要进行人身攻击。

其次，要把握review的粒度，不要一下发起一个非常大的PR，这样会给review的同学特别大的压力。比如一个PR里最好不要同时既有重构又有新特性的开发，或者憋到最后这个版本都要开发完了才一起提交一个PR。review应该是在平时的工作中持续进行的，而不是类似里程碑的总结之类的东西。

第三，code review不应该承担发现业务逻辑错误的责任，也就是平常我们所说的bug，bug应该由单元测试、功能测试、性能测试等方法来保证，不要赋予code review太多的责任。