代码审查，第一部分见xkcd

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

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

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

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

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

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

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

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

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

代码审查习惯

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

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

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

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

关键代码审核员

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

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

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

三级代码审查

考虑到上文中提到的代码审查规则和方式，我认为可以根据代码审查各个层次的重要性和效果来区分出三个层级。

架构

第一层也是最重要的一层，以架构为中心的代码审查。在这层，我们应当考虑项目的具体情况以及之前曾达成的共识。这个层级的代码审查主要应由最有经验的程序员来完成，因为他们了解项目架构。就算是新手也可以尝试，这是一种很好的锻炼方式，但如果感到吃力也不用太过勉强。

在这个阶段，找出潜在的命名泄露和违规是至关重要的。某个规则在你看来是理所当然的，因为你对这个项目了如指掌，但对其他人来说就不一定了。这样的情况可以作为讨论的一个很好的起点，从而弄清楚代码作者的动机。

架构师和工程师在做架构层面的代码审查时，也应牢记提交的拉取请求在扩展和重复利用方面的需求，要知道这一点可以无条件地更改实现。

同样，应用安全也是需要重点关注的关键因素之一。

架构层面上的代码审查需要考虑的问题还有很多，相信你也认识到在这一阶段能够提前发现潜在问题的价值有多大了。

代码清理

在代码审查的第二层，我们可以深入到代码本身，更加注重细节。这一阶段被称作代码清理审查。

在这里，我们可以开始就内聚程度和类的组成进行争辩。将所有的疑问和困惑摊开来讨论，有助于我们更好地理解代码背后的思考过程。批判他人之前先要了解情况，也许这只是别人走出来的，我们暂时还看不懂的捷径。

说起“整洁代码”（为Bob Marting大叔和他的《代码整洁之道》一书点赞），就不得不提一句SOLID原则和测试。如果有哪里没有遵守这些规则中的任何一条，我们都应该迅速找出原因。工程师可能会出于某些原因不按规则出牌，但更有可能只是犯了个错。

编程中最艰难的部分之一大概要数命名了。如何合适地为变量、方法、函数、类和数据结构命名很是考验人。有的名字对你来说可能很顺手，但在其他人看起来可能就是云里雾里了。即便如此，我们还是要努力找到最优的“名字”，这也是二级审查应当关注的点。

打磨表层

第三阶段也就是最后一层，我们要做的是代码审查中的打磨抛光阶段。在这一阶段我们面对的是一些通常可以被linter或者编译器自动捕捉到的小错误，例如代码格式、缺失分号等其他琐碎问题。

这时我们也可以做一些细小的排序/分组更新，抱怨下代码作者使用的数据结构，到底是该用List<>还是Set<>还是别的什么的。

无论是谁提交了拉取请求并不重要。代码审查应始终一视同仁，再有经验的软件工程师也不是什么都懂，哪怕是关于他们手头上正在做的项目，他们也不一定全都会。说到底，我们只是人，人都会犯错。

原文链接：
https://medium.com/netvise-software/software-architecture-and-code-review-882d779decf

创作场景

代码审查指南：不只关注细节