代码审查“查”什么?
| 2015-10-05 15:03 收藏: 2
让我们来谈谈代码审查(Code Review)。如果花几秒钟去搜索有关内容,你会发现许多论述代码审查好处的文章(例如,Jeff Atwood的这篇文章)。你还会发现许多介绍如何使用代码审查工具的文档,比如我们常用的Upsource。但能够在你审查他人代码时指导查什么的内容却很少见。
或许没有明确审查条目的原因是:有太多不同的因素需要考虑。就像对任何(功能性或非功能性)需求,个体组织对各个方面的优先级都有不同的考虑。
由于文章主题覆盖面广,本文旨在概述了代码审查者在代码审查时可以关注的一些方面。各方面优先级的分配和持续检查是一个非常复杂课题,具有研究价值。
审查他人代码时,查什么?
无论是使用 Upsource 类似工具审查代码,还是在同事查他们代码期间,不管哪种情况,相比较而言评审有些内容要更容易。例如:
- 格式:在什么地方放置空格和断行符?使用制表符还是空格?大括号如何放置?
- 风格:变量、参数是否声明为final?函数变量的定义是否与调用
代码或函数开始代码太相似? - 命名:域、常量、变量、参数、类的名称是否符合标准?命名是否太短?
- 覆盖测试:这段代码是否进行了测试?
这些检查都是有意义的——你希望尽可能减少不同代码间的上下文切换并减少认知负荷,那么代码看上去越一致越好。(译者注:认知负荷(Cognitive Load)是指人在完成任务过程中进行信息加工所需要的认知资源的总量。)
但是,人工审查这些内容可能没有充分利用团队的时间和资源,因为其中大部分检查可以自动化。许多工具都能确保代码格式一致,包括命名和 final 关键字的使用规范,而且能发现简单编程错误造成的常见bug。例如,你在命令行中运行IntelliJ IDEA的检查,那么就不需要所有团队成员在他们的集成开发环境中进行相同的检查。
你应该审查什么?
哪些事情适合代码审查者做?在代码审查时,哪些事情是工具不能代劳的?
事实上,代码审查者可以做的事情很多。当然,下面给出的不是一份详尽的列表,这里也不会深入探讨其中的任何一项。相反,这应该是团队交流的起点,现在你们在代码审查中关注什么,也许正是你们需要审查的内容。
设计
- 新代码是否与整体架构匹配?
- 代码是否遵循SOLID原则、领域驱动设计以及团队喜爱的其它设计模式。
- 新代码采用哪些设计模式?这些模式合适吗?
- 如果代码库采用混合标准或设计风格,新代码是否符合当前风格?代码迁移是按正确的方向进行,还是效仿即将被淘汰的陈旧代码?
- 代码是否处于正确的位置?例如,如果代码执行与顺序有关,它是否能按顺序执行?
- 新代码能否复用部分已存在代码?新代码能否给已存在代码提供复用部分?
- 新代码是否包含冗余代码?如果包含,是应该重构成更加可复用部分,还是在这个阶段能接受这种冗余?
- 代码是否超出设计标准?这种复用构造现在是不是不需要?团队如何根据YAGNI权衡复用?
可读性与可维护性
- 命名(字段、变量、参数、函数以及类)能否反映它们代表的事物?
- 通过阅读代码,我能否理解代码的目的?
- 我能否理解测试的目的?
- 测试是否覆盖了绝大部分情况?是否覆盖常见情况和异常情况?是否存在没考虑到的情况?
- 异常错误消息是否易于理解?
- 难以理解的代码段是否进行了备注、评论或者使用易懂的测试案例进行覆盖(符合团队的偏好)?
功能
- 代码的实际工作是否符合预期?如果有自动化测试来确保代码的正确,测试能否测出代码满足约定要求?
- 代码看上去是否含有细微错误,比如使用错误变量进行检查,或者把 or 误用为 and
?
你是否考虑过……?
- 代码中是否存在潜在的安全问题?
- 是否需要满足规范要求?
- 对于没有覆盖自动化性能测试的代码段,新代码是否引入了不可避免的性能问题,比如不必要的数据调用或远程服务?
- 作者是否需要创建公共文档,或者修改现有的帮助文档。
- 展示给用户的消息是否检查无误?
- 是否存在导致产品崩溃的明显错误?代码是否会意外指向测试数据库,或者是否有应该替换成真正服务的硬编码存根代码?
如何编写“良好”代码是每个开发者都能各抒己见的主题。如果你有一些可以加入我们列表的内容,希望能在评论中收到你的回复。