让我们谈谈代码审查。如果您只花几秒钟搜索有关代码审查的信息,您会看到很多关于为什么代码审查是一件好事的文章(例如, Jeff Atwood 的这篇文章 )。
您还会看到很多关于如何使用 Code Review 工具(例如我们自己的 Upsource ) 的文档。
您很少看到的是在审查其他人的代码时要查找的内容的指南。
可能没有关于要寻找什么的明确文章的原因是:有 很多 不同的事情需要考虑。而且,与任何其他需求集(功能性或非功能性)一样,各个组织对每个方面都有不同的优先级。
由于这是一个很大的话题,本文的目的是概述审阅者在执行代码审阅时可能需要注意的一些事情。确定每个方面的优先级并始终如一地检查它们是一个足够复杂的主题,足以单独成为一篇文章。
当你审查别人的代码时,你在寻找什么?
无论您是通过 Upsource 之类的工具审查代码,还是在同事遍历他们的代码期间审查代码,无论在何种情况下,有些事情比其他事情更容易评论。一些例子:
- 格式化 :空格和换行符在哪里?他们使用制表符还是空格?花括号是如何布局的?
- 样式 :变量/参数是否声明为最终的?方法变量是在靠近使用它们的代码处还是在方法开始处定义的?
- 命名 :字段/常量/变量/参数/类名称是否符合标准?名字是不是太短了?
- 测试覆盖率 :是否有针对此代码的测试?
这些都是需要检查的有效内容——您希望最大程度地减少不同代码区域之间的上下文切换并减少 认知负担 ,因此您的代码看起来越一致越好。
但是,让人工查找这些内容可能不是您组织中时间和资源的最佳利用方式,因为其中许多检查都可以自动进行。有很多工具可以确保您的代码格式一致,遵循命名标准和 final 关键字的使用,并发现由简单的编程错误引起的常见错误。例如,您可以运行
IntelliJ IDEA 从命令行进行检查
,因此您不必依赖所有团队成员在其 IDE 中运行相同的检查。
你应该找什么?
人类真正适合做什么样的事情?我们可以在代码审查中发现哪些我们无法委托给工具的内容?
事实证明,事情的数量多得惊人。这当然不是一个详尽的列表,我们也不会在这里详细介绍其中的任何一个。相反,这应该是您组织中关于您当前在代码审查中寻找哪些内容以及您或许 应该 寻找的内容的对话的开始。
设计
- 新代码如何与整体架构相契合?
- 代码是否遵循 SOLID 原则 、 领域驱动设计 和/或团队喜欢的其他设计范例?
- 新代码中使用了哪些 设计模式 ?这些合适吗?
- 如果代码库混合了标准或设计风格,那么这个新代码是否遵循当前的做法?代码是朝着正确的方向迁移,还是遵循即将被淘汰的旧代码的示例?
- 代码在正确的地方吗?比如代码跟Orders相关,是不是在Order Service里面?
- 新代码是否可以重用现有代码中的某些内容?新代码是否提供了我们可以在现有代码中重用的东西?新代码是否引入重复?如果是这样,是否应该将其重构为更可重用的模式,或者在现阶段是否可以接受?
- 代码是否过度设计?它是否为现在不需要的可重用性而构建?团队如何平衡 YAGNI 对可重用性的考虑?
可读性和可维护性
- (字段、变量、参数、方法和类的)名称是否真正反映了它们所代表的事物?
- 我能通过阅读理解代码的作用吗?
- 我能理解测试的作用吗?
- 测试是否涵盖了很好的案例子集?它们是否涵盖了幸福之路和例外情况?是否有未考虑的情况?
- 异常错误消息是否可以理解?
- 混淆的代码部分是否被记录、注释或被可理解的测试覆盖(根据团队偏好)?
功能性
- 代码真的做了它应该做的事吗?如果有自动化测试来确保代码的正确性,这些测试是否真的测试代码是否符合约定的要求?
-
代码看起来是否包含细微的错误,例如使用错误的变量进行检查,或者不小心使用了
and
而不是or
?
你有没有想过…?
- 代码是否存在潜在的安全问题?
- 是否需要满足监管要求?
- 对于自动化性能测试未涵盖的领域,新代码是否引入了可避免的性能问题,例如对数据库或远程服务的不必要调用?
- 作者是否需要创建公共文档或更改现有的帮助文件?
- 是否检查了面向用户的消息的正确性?
- 是否有明显的错误会阻止它在生产中工作?代码是否会意外指向测试数据库,或者是否有一个硬编码的存根应该换成真正的服务?
请留意此博客上更详细地涵盖这些主题的后续帖子。
什么是“好”代码是每个开发人员都有自己看法的话题。如果您有要添加到我们列表中的内容,我们很乐意在评论中听取您的意见。