五个代码审查反模式 - Trisha Gee

20-05-08 banq

本文指出了所有开发人员在审查其代码或提交拉取请求时可能遇到的特定反模式,并对此进行了谴责。

代码作者花了数小时甚至数天的时间来创建他们认为最有效的解决方案。他们考虑了多种设计方案,并采取了最相关的道路。他们考虑了现有应用程序架构,并在适当的位置进行了更改。然后他们将其解决方案作为请求请求提交,或者开始了代码审查过程,并且收到的专家反馈是

  • “您应该使用制表符,而不是空格。”
  • “我不喜欢本节中的括号。”
  • “文件末尾没有空白行。”
  • “您的枚举是大写的,应该用句子大写。”

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

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

反模式:反馈不一致

对于每位受邀审查代码的开发人员,至少要征询一个意见,甚至更多。每个人可以同时持有多个意见。有时,代码审查可能会成为审查者之间关于不同方法的争论。

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

  • 代码是否应该朝着更现代的Java风格发展?还是更重要的是,代码是一致的,并因此继续在各处使用“经典”构造?
  • 在系统所有部分中对数据结构进行O(1)读取操作是否重要?还是有O(n)可接受的部分?

几乎所有设计问题都可以用“取决于”来回答。为了更好地了解答案,开发人员需要了解其应用程序和团队的优先级。

反模式:最后一刻的设计变更

开发人员在代码审查期间获得的最令人沮丧的反馈是,当审查者从根本上不同意解决方案的设计和架构并强制完全重写代码时。

代码审查不是审查设计的正确时机。如果团队遵循经典的“网关”代码审查,则该代码应该可以正常工作,并且所有测试都应通过,然后再由另一个开发人员查看该代码。在这一点上,需要花费数小时,数天甚至数周的时间(尽管我真的希望不要数周;代码审查应该很小,但这是另一个主题)。在代码审查期间指出底层设计是错误的,这是浪费每个人的时间。

反模式:打乒乓球式循环评论

在理想的情况下,作者将提交其代码以供审阅,审阅者将提出一些具有明确解决方案的评论,作者将提出建议的更改并重新提交代码,审阅将完成,代码将被推送。但是,如果这总是想打乒乓球一样来回循环定期发生,谁能证明代码审查过程的合理性呢?

在现实生活中,经常发生以下情况:

  1. 代码检查开始。
  2. 许多评论者提出了一些建议:一些小而轻松;一些蓬松而没有明显的解决方案;还有一些复杂。
  3. 作者进行了一些更改:至少是简单的修复,或者可能进行了一些更改,以使审阅者满意。作者可能会问审稿人一些问题以澄清问题,或者作者可能会发表评论以解释为什么未进行特定更改。
  4. 审阅者返回,接受一些更新,对其他评论进行进一步评论,找到其他他们对原始代码不满意的事物,回答问题,并与其他审阅者或作者就审阅中的评论进行辩论。
  5. 代码作者进行了更多更改,添加了更多注释和问题,等等。
  6. 审阅者检查更改,提出更多评论和建议,等等。
  7. 重复步骤5和6,也许永远重复一次。

从理论上讲,在此过程中,更改和注释应降至零,直到代码准备就绪为止。最令人沮丧的情况是,每次迭代都带来至少与已关闭的旧问题一样多的新问题。在这种情况下,团队已进入“代码审查的无限循环”。发生这种情况的原因有很多:

  • 如果审稿人挑剔并且反馈不一致,就会发生这种情况。对于习惯于这些习惯的审阅者来说,有无数的问题可以识别,也有无数的评论可以发表。
  • 当审查没有明确的目的或审查期间没有遵循准则时,就会发生这种情况,因为每个审查者都认为必须找到所有可能的问题。
  • 当不清楚代码编写者对审阅者的评论有何要求时,就会发生这种情况。每个评论是否都暗示必须进行更改?所有问题是否都暗示该代码不是自记录文档,需要改进?还是只是为了下一次教育代码作者而发表一些评论,而提出的问题仅仅是为了帮助审阅者理解和学习吗?

注释应该理解为显示最高者或不是显示最高者,并且如果审阅者决定代码需要更改才能签名,则他们必须明确说明代码作者应采取的行动。

同样重要的是要了解由谁来负责决定审核是否“完成”。这可以通过已检查并通过的项目的任务列表来实现,也可以由有权说“足够好”的个人来完成。通常需要有人可以打破僵局并解决分歧。它可能是高级开发人员,主管或架构师,甚至可能是团队中高度信任的代码编写者。但是在某些时候,有人需要说“审查完成”或“这些步骤完成后,审查就完成了”。

反模式:Ghost Reviewer

在审阅期间根本不回应。也许有人要求我审查一个重要或有趣的功能,所以我决定将其保留,直到“更好的时间”,当我可以“正确地看待它”。

以下是解决方法的建议:

  • 确保代码审查很小。每个团队都必须确定其定义,但是要审查的时间是数小时或数天,而不是数周。
  • 确保代码审查的目的明确,并且审查者应寻找的内容也明确。当范围“找到代码中的任何可能的问题”时,很难激励自己去做某事。
  • 在开发过程中留出时间进行代码审查。

最后一点可能需要团队纪律,或者团队可能希望通过(例如)通过目标或用于确定开发人员生产率的任何机制来奖励良好的代码审查行为来鼓励时间。

 

                   

猜你喜欢