前沿

最近面试了一些人，当我问他们是否在项目中使用Code Review和UnitTest。大部分人回答是会，但是细问他们是如何Review的，他们并没有太多深刻的认识或只是当作一种行政命令。下面我想讲讲关于Code Review那些事。

1. Code Review的好处

之所以要谈Code Review，我个人认为Code Review给团队乃至个人都有很大的帮助，它不仅有助于提高产品代码质量，同时可以促进团队建设与个人成长。下面我就从代码，团队，个人三个方面简单的列举一些Code Review的优点：

• 代码质量

  1. 确保实现与设计、需求一致
  2. 保持代码风格的一致性
  3. 及时发现代码中的问题，做到及时的修正与重构
  4. 提高代码的可维护性

• 团队成长

  1. 有利于团队成员技术上的交流与分享
  2. 培养团队成员的责任感，不至于各扫门前雪
  3. 有利于团队在技术实现上达成一致
  4. 有利于新成员对项目与工作的快速了解与融入
  5. 团队工作更加透明

• 个人能力

  1. 培养代码分析和理解能力
  2. 学习他人的优秀代码和解决问题的不同思路
  3. 发现自身存在的问题，检验自己的技术水平
  4. 提高个人鉴赏代码的品味

2. Code Review的个人经验

Code Reivew不同团队、不同的项目都有自己的实施方法。我觉得方式没有对错，只要适合自己就可以，当然我还是从我的个人经验给出一些自己的建议，希望对大家有所帮助

1. 自动化检测代码风格
2. 选择合适的Review方法及工具
3. 设定参与者的角色
4. 少量多次的提交Review

1. 自动化检测代码风格

一个团队或一个产品的代码风格应该保持一致性，这样便于代码的阅读和维护。而大多数代码风格都可以利用已有的静态代码分析工具自动检测，比如C#（Resharper + StyleCop或VisualStudio自带的Code Analysis），C++（CppCoreGuidelines）。这类重复性的工作非常适合自动化，如果换成人来做，就会变得繁琐和无聊。所以在人工Review代码之前，务必将一些简单、格式化的工作交由代码分析工具自动处理，这将会使得我们的生产力得到充分释放。再次强调代码风格的Review不应该占据大量的比例，否者Review就会变得没有技术含量，最后变成一种走形式的负担。

2. Review方法及工具

Code Review一般分为：

• Pre-commit Review（提交代码之前必须Code Review）

• Post-commit Review（提交代码之后进行Code Reivew)

相比较我个人更喜欢两者的结合，我们可以为自己建立一个独立的分支（feature）进行随意的代码提交，但是一旦需要将代码提交到主干分支（master/develop）时，就必须完成Code Review以后才允许提交。这样的好处在于我们可以track我们所有的修改记录，同时又能保证主干分支代码的质量。 Git Pull Request就非常适合这种场景。因为Git创建分支的成本很低，我们可以很方便的建立一个feature branch，然后根据情况提交Review和Merge。

当然并不是所有代码版本控制软件都适合这种场景。比如SVN（Subversion）的一个branch可能就是一个阶段性的主干，同时有几个team共同维护，而每个人去建立分支的成本又很大。这个时候我建议使用Pre-commit Review去约束开发人员，保证提交到主干上的代码质量。这样可以在提交代码前及时的得到反馈，而不会等到代码提交了，别人基于你提交的代码又做了大量工作后发现之前的实现存在问题，从而导致大量不必要的返工。这里再推荐另外两个工具Review Board（开源工具）、Code Collaborator （收费工具）它们可以支持多种的代码版本控制软件。

至于Post-commit Review用它的好处就是可以及时的提交代码，好让依赖此代码的其他开发人员及时拿到代码。但是这个问题我觉得可以通过两个方法解决，一个是团队的有效沟通，另一个就是鼓励开发人员优化代码设计，减少代码之间的依赖。

3. Review参与者

Code Review一般建议设置两类成员Reviewer和Observer，其中只有Reivewer拥有代码提交的否决权。

Reviewer：必须是和你提交代码利益相关的成员，所以他有责任保证提交代码的质量。比如你和几个同事共同完成一个需求，那么你应该将他们的其中一个或几个设置为Reviewer，这样他们就会了解你做的功能，以及他们如何使用你所写的代码。而这些同事是最有可能发现你的实现是否符合设计以及可能出现的潜在问题，在Review的过程中你会得到更多的反馈与沟通。有时你提交的代码涉及到重大的架构调整或安全问题，这时你可能还要将System Architect或有经验的开发作为Reviewer，这样能及时得到正确权威的意见。

Observer：对代码是否可以提交没有决定权，他只提供自己的建议，如果他们没有时间可以不Review代码。比如一个新人，他可能希望得到有经验开发者的建议和指导，但是这些开发者并不是代码的相关人员，他们可以根据自己的时间安排决定是否Review代码。又或者作为有经验的开发人员，你可以把一些新人设置为Observer，让他们了解项目情况，以及你的实现方法。这是非常好的分享机会，有时会引出激烈的讨论和更好的解决方案。

我的建议一次可以设置一到两个Reviewer和任意多个Observer，只要你觉得有必要的人都可以添加成Observer，毕竟人多可以产生更多的反馈与思路。

4. 少量多次的提交Review

为什么要少量多次的提交代码？

1. 很明显代码量少可以给Code Reivew带来很大的帮助，Reviewer可以花更少的时间来读懂你的代码。

2. 迫使代码提交者在实现功能时有意识的对需求进行拆分和代码设计的解耦。很多时候程序员所谓的“洁癖”（直到所有功能都做好了才提交代码）其实是对实现思路的模糊不清，而且实现的每一部分都紧密耦合，缺少任何一部分就会使代码编译失败或运行错误。

3. 当第一次少量提交后，你可以得到快速的反馈，有利于和团队成员快速确认你的实现是否合理。而不是等到所有功做完以后发现需要推倒重写。

4. 养成一个快速迭代和集成的习惯，每次提交都会触发单元测试，从而快速得到代码的正确性的反馈。

这里我建议200到500有效代码行提交一次Review。

5. Code Review关注方面

• 一致性
  1. 制定项目或语言的code standard（code standard就像一个基础的checklist，但是这个是由代码提交者（author）去遵循的，不应该在这个方面浪费reviewer的时间
  2. 自动化静态代码分析（上面已经提到一些工具）
• 功能
  1. 代码是否实现了需求功能
  2. 是否有明显的bug
• 设计
  1. 是否符合设计文档
  2. 类的抽象是否正确
  3. 是否遵循SOLID原则
  4. 是否可以利用依赖倒置进行解耦
  5. 是否封装了变化点
  6. 是否可以利用已有的设计模式让代码更直观清晰
  7. 新加的代码是否在正确的位置（模块或架构层次）
• 正确性
  1. 是否异常被捕获
  2. 是否存在线程安全
  3. 是否资源/内存泄露
• 可维护性
  1. 命名（函数，类，变量等）可以清晰的表诉业务逻辑，尽可能做到代码的自注式（self-explanations）
  2. 函数，类是否过长过大，如果是，做到及时的拆分
  3. 是否一些逻辑可以重用，如果是，是否可以提取共用函数，类甚至是库
  4. 遵守KISS原则
  5. 是否有单元测试覆盖
  6. 代码是否易读
  7. 删除没有必要的代码（很多程序都有“收藏”代码的癖好，他们宁愿注释掉也不愿把没用的代码删除，理由是将来有一天需要）
• 性能
  1. 在不降低可维护性的前提下进行性能优化。
  2. 代码就是为了解决性能问题时，可以适当降低维护性，但是必须对此优化进行封装，以免污染其他代码。日后有其他更好的方法进行替换。

6. 拒绝Code Review的理由

就像我在面试的时候问应聘者，是否你们会做单元测试或Code Review，他们大多的回答是肯定的，虽然他们实际工作中并没有那么做。但是他们至少是认为Code Review是正确的事，否则他完全没有必要在我面前来掩饰自己没做这件事。现实中往往会有以下理由来支撑他们不做Code Review(这些理由同样支持他们不做Unit Test)。

• 项目太紧没有时间做
• Code Review形同虚设，只是个过程
• 代码只是临时的，Code Review以后还会被修改

下面是我对上诉理由的一些看法：

项目紧没时间：首先项目的紧急状况和有没有必要做Code Review没有直接的因果关系。当项目紧的时我们应该区分事件的紧急与重要关系（紧急重要，紧急不重要，重要不紧急，不重要不紧急）。首先我认为Code Review对于开发人员来说是重要的。其次有时程序员宁愿花大把时间完成一个不重要的功能，也不愿意为Code Review这样重要的事争取时间。项目紧，说明人员安排和需求安排出现了问题，这时需要产品经理和开发们坐下来对功能性需求和非功能性需求(Non-Functional Requirement)进行优先级定义，然后沟通调整取舍，作为开发人员如果我们都不关心非功能性需求那还有谁有这个义务。我们的目标不是做一个可以运行的程序，而是一个高质量的可维护的产品。这不仅仅为客户，还为我们自己。

Code Review只是形式过程：这里有一句话我觉很应景，心中有佛，看人即为佛，心中有屎，看人即屎。其实这里就看你是否把Code Review当作一种形式和负担。如果你的团队都是这样的人，我建议你可以离开了。作为新人如果每次你认真的Review他人的代码，或者接受他人的评审建议，那你会成长很快。同样不要纠结于简单的代码风格，它不会给你很大的进步，相信让你最痛苦的事往往让你成长的更快，强迫自己在每次review的时候都从深层次发现问题，多提交代码给那个给你建议最多的同事，只有与越优秀的人一起工作你才会变得更优秀。对于有经验的开发，不要嫌弃他人得代码，给出你的建议让代码更好，最终你也是受益的一员，毕竟你也不希望维护脏乱差的代码，我相信优秀的程序员都喜欢分享，否则就不会有今天如火如荼的开源社区。另外再优秀的程序员也可能犯错，何况人外有人天外有天。越是优秀的人越谦虚，他能包容更多的想法。

代码是临时的：请不要再自欺欺人，如果你不重视你写的每一行代码，都是临时对待，相信我你会养成习惯，你再也写不出好的代码。能写出代码的人太多，而写好代码的人却凤毛麟角。你希望成为什么样的程序员？记住一般程序员只关注功能性需求，好的程序员在关注功能性需求的同时还注重非功能性需求。

后记

很多时候我们在紧张的状态下，会放弃一些我们一直认为正确或重要的事。但是回过头来我们(也可能是别人)会为我们之前的放弃，花更大的成本和精力去弥补。