五种 Code Review 反模式

每个人都关心最佳实践，但最坏的做法有时可能更具启发性。

代码审查是必不可少的，但它们并不总是正确地完成。本文指出了开发人员在进行代码审查或提交pull请求时可能遇到的一些反模式。

反模式1：吹毛求疵

想象一下以下场景。代码作者花费数小时甚至数天的精力来创建他们认为最有效的解决方案。他们考虑了多种设计方案，并选择了似乎最相关的路径。他们考虑了现有应用程序的体系结构，并在适当的位置进行了更改。然后，他们以拉取请求的形式提交了解决方案，或者启动了代码审查过程，他们收到的专家反馈是：

• “你应该使用制表符，而不是空格。”
• “我不喜欢这个部分的括号。”
• “文件末尾没有空行。”
• “你的枚举是大写的，它们应该是句子大小写的。”

尽管新代码与现有代码的风格保持一致很重要，但这些几乎不需要人工审阅。人工审稿人很昂贵，可以完成计算机无法完成的事情。检查是否满足样式标准是计算机可以轻松完成的事情，并且会分散代码审查的真正目的。

如果开发人员在代码审查期间看到许多此类注释，则表明团队要么没有样式指南，要么有样式指南，但检查样式尚未自动化。解决方案是使用 checkstyle 等工具来确保遵循样式指南，或者使用 sonarqube 来识别常见的质量和安全问题。与其依靠人工审核人员来警告此类问题，不如持续集成环境可以做到这一点。

有时，如果不存在代码指南，或者内部代码样式随着时间的推移而演变并且在各个部分有所不同，则可能很难自动执行此类检查。在这些情况下，有一些方法可以应用自动检查。例如，团队可以同意执行单个提交，该提交应用商定的代码样式并且不包含其他更改。或者，团队可以同意，当文件因错误或功能而更改时，文件也将更新为新样式，并且可以将自动化工具配置为仅检查更改的文件。

如果一个团队有各种各样的代码风格，并且它没有办法自动检查风格，那么它也很容易落入下一个陷阱。

反模式2：反馈不一致

对于被邀请来审查代码的每一位开发者，至少要再邀请一位发表意见——甚至可能更多。每个人都可以同时持有多种意见。有时，代码审查可能会演变成评审员之间关于不同方法的争论，比如流（stream）或经典for循环哪种方法更好。如果团队成员对同一段代码持有相反的观点，开发者应该如何做出更改、关闭审查，并将代码推送到生产环境呢？

即使是单个评审员的想法也很容易改变，无论是在一次评审中还是在多次评审中。在一次评审中，评审员可能会敦促作者确保使用具有 O(1) 读取操作的数据结构，而在下一次评审中，评审员可能会询问为什么针对不同的使用案例存在几个数据结构，并建议通过单个结构进行线性搜索来简化代码。

当团队不清楚其“最佳实践”是什么样子并且没有弄清楚其优先级时，就会发生这种情况，例如：

• 代码是否应该转向更现代的 Java 风格？还是代码保持一致，因此继续在任何地方使用“经典”结构更重要？
• 对系统所有部分的数据结构进行 O（1） 读取操作是否重要？或者是否有可以接受 O（n） 的部分？
• 几乎所有的设计问题都可以用“视情况而定”来回答。为了更好地了解答案，开发人员需要了解其应用程序和团队的优先级。

反模式3：最后一刻的设计更改

在代码审查期间，开发人员所能收到的最令人沮丧的反馈就是审查人员从根本上不同意解决方案的设计或架构，并且强迫开发人员通过一系列审查（参见下文）逐步重写代码，或者粗暴地拒绝代码并让开发人员重新开始。

代码审查并不是检查设计的合适时机。如果团队遵循的是经典的“网关”代码审查，那么在最终由另一位开发人员审查代码之前，代码应该能够运行，并且所有测试都应该通过。此时，审查中的代码已经花费了数小时、数天或数周（尽管我真的不希望是数周；代码审查应该小而紧凑，但这是另一个话题）的努力。在代码审查期间指出基础设计是错误的，是浪费每个人的时间。

代码审查可以作为设计审查，但如果这是代码审查的目的，则审查应在实现开始时进行。然后，在开发人员过于深入地理解问题之前，他们可以使用一些桩类和方法，以及一些具有意义名称和步骤的测试，来概述他们的想法，并可能提交一些文本或图表，以便团队成员对要采取的方法提供反馈。

如果团队成员在网关审查中真正发现了令人震惊的设计问题（即代码已完成并可以工作），那么团队应该更新其流程以更早地发现这些问题。这可能意味着进行不同类型的审查，例如前一段建议的，在白板上讨论想法，结对编程，或者与技术负责人讨论拟议的解决方案。在最终代码审查中发现设计问题是对开发时间的浪费，并且极大地打击了代码作者的士气。

反模式4：Ping-Pong 评论

在理想世界中，作者会提交自己的代码供审核，审核人员会给出几个带有清晰解决方案的建议，作者会做出建议的更改并重新提交代码，审核会完成，代码会被推送。但是，如果这种情况经常发生，那么还有谁会认为代码审核过程是必要的呢？

在现实生活中，常常会发生这样的事情：

1、代码审查已启动。

2、一些评审者提出了几个建议：有的小且容易，有的空泛没有明确的解决方案，有的复杂。

3、作者做了一些修改：至少对容易修改的地方进行了修改，或者可能进行了几项修改以使评审人满意。作者可能会向评审人提问以澄清某些内容，或者作者可能会发表评论以解释为何没有做出特定的修改。

4、评审者会回来，接受一些更新，对其他的进一步评论，找出原始代码中他们不喜欢的东西，回答问题，并与其他评审者或作者在评论中争论。

5、代码作者进行更多更改，添加更多注释和问题，依此类推。

6、评审者检查更改，提出更多评论和建议，等等。

7、步骤 5 和 6 重复，也许永远重复。

• 在此过程中，从理论上讲，更改和注释应该逐渐减少到零，直到代码准备就绪。最令人沮丧的情况是，每次迭代带来的新问题至少与已关闭的旧问题一样多。在这种情况下，团队已经进入了“代码审查的无限循环”。发生这种情况的原因有很多：
• 如果评审人员吹毛求疵，给出不一致的反馈，就会发生这种情况。这些对于养成这些习惯的评审人员来说，要识别的问题和要发表的评论都是无穷无尽的。
• 当评审没有明确的目的或在审查过程中没有遵循的指导方针时，就会发生这种情况，因为每个评审者都认为必须找到所有可能的问题。
• 这种情况发生在代码审阅者的评论对代码编写者提出的要求不明确时。每条评论是否暗示必须做出更改？所有的问题是否都暗示代码缺乏自我描述性并需要改进？还是一些评论只是为了教育代码编写者，而问题只是为了帮助审阅者理解和学习？

应该将评论理解为阻止或不妨碍的因素，如果审查员决定在签署之前需要更改代码，他们需要明确指出代码作者应该采取的确切行动。

同样重要的是要明白，由谁来决定审查“已完成”。这可以通过一个检查通过的任务清单来实现，也可以由某个有权力说“已经足够好了”的人来负责。通常需要有人来打破僵局和解决分歧。这可能是高级开发人员、负责人或架构师——或者甚至是在高度信任的团队中的代码作者。但是，在某一时刻，需要有人说“审查已完成”或“当这些步骤完成时，审查已完成”。

反模式5：幽灵评论

在此，我承认我最常犯的错误模式：失踪。无论我是评审者还是代码作者，在代码评审过程中（有时就在一开始！）总有一段时间，我在评审期间不会做出回应。也许有人要求我评审一个重要的或有趣的功能，因此我决定等到“更好的时机”再“认真查看”。或者评审内容很多，我想预留充足的时间。或者我是代码作者，经过一次（或二十次）迭代后，我再也无法面对阅读和回答评论，所以我决定等到“状态合适”时再处理。

听起来很熟悉？

无论出于何种原因，有时在审查过程中会有人不作出回应。这意味着，直到这个人查看代码之前，审查都无法继续。这是资源的浪费：尽管有人投入时间去创建了资产（新代码），但只有当它被投入生产时，它才能创造价值。事实上，随着新代码与其余代码库之间的距离越来越远，它可能已经变质了。

几个因素可能导致幽灵评审。大型代码评审是其中一个因素，因为谁愿意仔细审查数十或数百个更改过的文件？不把代码评审视为实际的工作或交付成果的一部分是另一个因素。艰难或令人沮丧的代码评审体验是另一个重要因素：没有人愿意停止编码（开发人员通常喜欢的一种活动）来参加耗时且令人沮丧的活动。

以下是解决幽灵评论的建议：

• 确保代码审查的规模较小。每个团队必须制定自己的审查定义，但审查的工作时间应在数小时或数天之内，而不是数周。
• 确保代码审查的目的明确，审查者应该寻找的内容也明确。当范围是“发现代码的任何可能问题”时，很难激励自己做某事。
• 在开发过程中留出时间进行代码审查。

最后一点可能需要团队纪律，或者团队可能希望通过奖励（例如通过目标奖励或通过其他机制来确定开发人员的生产率）优秀的代码审查行为来鼓励留出时间。

团队能做些什么？

专注于创建可靠的代码审查流程。我已经在我的博客上写过它，但想在这里分享这个过程的一部分。

在进行代码审查时，有很多事情需要考虑，如果开发人员在每次代码审查中都担心所有这些问题，那么任何代码都几乎不可能通过审查过程。实现适合所有人的代码审查过程的最佳方法是考虑以下问题：

• 团队为什么要进行审核？当有明确定义的目的时，评审者的工作会更轻松，代码作者在审阅过程中遇到的令人讨厌的意外也会更少。
• 团队成员在寻找什么？当有了目的后，开发者在审查代码时可以创建一套更加聚焦的检查事项。
• 谁参与其中？谁来审查，谁负责解决意见冲突，谁最终决定代码是否好用？
• 团队何时进行评审，何时完成评审？审查可以在开发人员处理代码时或过程结束时以迭代方式进行。如果没有关于代码何时最终可以使用的明确指导，审查可能会永远持续下去。
• 团队在哪里进行审核？代码审查不需要特定的工具，因此审查可以像作者在办公桌前引导同事浏览他们的代码一样简单。

回答这些问题后，您的团队应该能够创建一个运行良好的代码审查流程。请记住，审查的目标应该是将代码投入生产，而不是证明开发人员有多聪明。

结论

通过制定明确的代码审查流程，可以消除或至少缓解代码审查反模式。许多团队认为他们应该进行代码审查，但他们没有明确的指导方针来说明他们为什么要这样做。

不同的团队需要不同类型的代码审查，就像不同的应用程序有不同的业务和性能要求一样。第一步是弄清楚为什么团队需要审查代码，然后团队可以继续工作

• 自动执行简单检查（例如，检查代码样式、识别常见错误和查找安全问题）
• 制定明确的指导方针，说明何时进行审核、寻找什么以及由谁决定何时完成审核
• 使代码审查成为开发过程的关键部分

关注代码审查的原因将有助于团队为代码审查过程创建最佳实践，从而更容易避免代码审查反模式。

 参考：https://blogs.oracle.com/javamagazine/post/five-code-review-antipatterns