代码审查流程中的多种不同方法

在我为多家公司工作的经历中，我见到最多的代码审查方法就是专注于细节，尤其是在不知道上下文的情况下。

首先让我们考虑下这个情况：你是否还记得，有多久没见过审阅的代码完美符合“四项原则”（《设计模式——可复用的面向对象软件元素（1994）》）中的某一项模式了？你是否试着理解过，为什么会有人选择不用这些原则？你是否曾在看到某个拉取请求时被它吸引了注意力，并忍不住想留下些批评建议？有些地方明显应该套用这些模式，但你却不知道为什么开发人员没有这样做，原因就是你不清楚上下文。

搞清楚上下文还可以让你更好地理解之前工程师的命名、分组以及其他决定。而在另一方面，我们也要记住，上下文是一柄双刃剑，它可能会让我们思路闭塞。了解上下文的情况下，一切都是清楚明了的，但我们也会因此忽视一些单独去看才会发现的问题。牢记这一点，在找出最佳类和变量名之前，尽量多站在“新人”的角度上看问题。

不要误会我的意思，但仅从“整洁代码”的层面来考虑代码审查的话，我们所做的可能也只是一些表面的打磨工作。指出方法中的嵌套条件或者多参数问题的确是有意义的，我们也应当继续这样做下去，但说实话，这些并不是代码审查的最大价值所在。即使有上面列的这些错误，程序也还是会正常运行的，如果能有位经验丰富的程序员来做维护，它基本就不会出什么问题。更进一步讲，几个嵌套的“if”语句对于一般程序员来说可远比装饰器模式更容易理解。我们的关注点应当放在经过审查的代码所处的上下文中，并在那里排查潜在问题。

架构是软件中最关键的部分之一，其应当能反映业务逻辑，也是代码审查中的重点关注对象。如果我们能在这个核心中发现问题并迅速做出反应，就会为产品带来最大的价值。

我们绝不应以咄咄逼人的姿态来审查代码。立志找出代码中所有错误并在评论中毫不客气地指出它们，通常是新手程序员才会干得出来的事情，也有可能是那些在“伺机报复”的程序员的行为。不过如果最后一无所获那可就是太惨了。这种行为会让我们更多去思考一些小细节，结果失去了对所审查代码所处架构的大局观念。从具体场景中开始讨论可能的用例、问题和扩展需求，总是要比见缝插针地强塞设计模式要好。

再糟糕的代码审查也比没有强

世界上有很多的公司在合并分支之前并不会做代码审查。有时候这种公司会心血来潮，某位团队成员（可能是新来的）设置了代码审查工具，并倡导大家进行代码审查。这是新的开始，一切都很成功，但大家的热度也仅仅持续了一个月。一个月后，大家逐渐忘记要审查代码，因为没有时间，没有资源，或者因为任何你能想到的借口。借口永远不缺。代码审查变成了走程序，审查也仅仅是为了拿到批准。缺乏代码审查会导致生产的软件质量低下，而低质量软件会有怎样的后果，软件公司和工程师都很清楚。

代码审查习惯

值得一提的是，在团队内因为成员经验不同而产生的代码审查习惯的差异。这会带来一些建设性的意见或疑问，从而为代码带来价值。通过初级和资深程序员之间思维的碰撞，很有可能会产生对问题更深入的探讨。

如果是一位大拿带领并指引一队后辈，形势则会略有不同。这种情况下的代码审查更像是一种指导。

高级工程师团队中的代码审查是个蛮有趣的例子。在这种配置下的团队中，每个方案都不逊色于其他选项。方案间的不同更多是由于偏好或学习经历造成的；大多数高级工程师在编写代码时都有自己的一套习惯和模式，而想要改变这些模式并不容易；资深工程师们会据理力争，要求保留自己代码。这时能够放弃和寻求妥协的重要性就体现出来了，这也是高手们与其他程序员的区别。

软件工程师中存在的一个普遍问题是，他们对自己写的代码有很特别的“感情”，他们会认为代码下的每条评论都是针对自己的。因此，审核人在评论和提出修改建议时必须非常小心谨慎，否则很可能会引起作者的不适，进而造成审核和作者之间的隔阂。

关键代码审核员

了解项目的架构和具体细节是涉及和参与项目最多的那些工程师的责任。他们被称为关键代码审核员。他们能轻易发现低内聚、违反 SOLID 原则、层命名泄露以及其他可能在日后反咬我们一口的错误；他们可以通过拆分、提取、重用或是移动代码的某部分来提升代码可读性、带来方便维护等好处。在预知解决方案的后果，和找出可能的代码错位或重复方面，他们更是一把好手。

这些都是我们在审查代码时最应当考虑的事情。

当然，架构违规的后果最为严重，远比一个方法中存在多个参数或者类的代码行数过多要严重的多。