2.4 确定评审内容
在评审代码时需要从不同的角度进行考虑。首要的一点是需要评审的代码应当仅限于开发人员更改并提交的代码。这也是为什么我们推荐小步提交的原因。少量的代码更易于评审和交流。
接下来我们将逐一介绍评审人在一次完整而彻底的代码评审中应当关注的那些不同方面。
2.4.1 公司编码规范与业务需求
所有需要评审的代码必须遵守公司的编码规范并实现业务需求。新加入的代码应当遵守公司采用的最新编码标准和最佳实践。
业务需求分为不同的类型,包括业务与用户/干系人的需求和功能与实现上的需求。不论代码实现的是哪一类需求,都需要进行完整检查以确保它实现了这些需求。
例如,如果用户/干系人相关的需求指出希望能够添加客户账户,那么被评审代码是否符合当前需求列出的所有条件呢?如果公司的编码规范规定所有的编码必须包括单元测试,且测试用例需包含正常流程和异常情况,则被评审代码中是否实现了这些测试呢?如果上述几个问题中的任何一个的回答是否定的,那么就应当在评论中指出。而开发者应当在再次提交代码时处理这些问题。
2.4.2 命名规则
评审过程中应当检查不同的代码结构(例如类、接口、成员变量、局部变量、枚举和方法)的命名是否符合命名规则。没人喜欢像密码一样难以理解的名称,在大代码库中尤其如此。
以下是评审人需要关注的问题:
- 命名是否足够长,使人容易阅读和理解。
- 命名是否既体现代码意图又足够短,不会使人厌烦。
评审人必须能够读懂代码。如果代码难以阅读和理解,那么必须在合并前进行重构。
2.4.3 代码格式
正确格式化代码可以使代码易于理解。应当根据规则添加必要的命名空间、大括号和缩进。代码块的起止位置应当清晰易辨。
评审人在评审中应当关注以下问题:
- 代码缩进使用空格还是制表符。
- 空白字符的数目是否正确。
- 是否应当将过长的代码行分割为若干行。
- 使用何种换行符。
- 是否按照格式规范一行只有一条语句,只进行一个声明。
- 接续行是否正确地使用了一个制表位的缩进。
- 方法之间是否间隔一行。
- 是否用括号分隔表达式中的多个子句。
- 类和方法是否足够整洁,是否只做了它们应该执行的任务。
2.4.4 测试
测试应当易于理解并涵盖尽可能多的用例。其中既应当包含正常执行路径也需要包含异常用例。在评审测试代码时,评审人应当检查以下内容:
- 开发者是否为所有代码都编写了测试。
- 是否存在没有测试的代码。
- 所有的测试是否都有效。
- 有没有失败的测试。
- 代码是否有足够的文档,包括注释、文档注释、测试和产品文档。
- 系统中独立编译和工作的部分在集成之后是否可能产生缺陷。
- 代码是否有良好的文档记录以辅助维护和支持。
测试的评审流程如图2-10所示。
图2-10
未经测试的代码有可能在测试和产品环境中产生意料外的异常。而和未经测试的代码同样糟糕的是不正确的测试。它会导致难以调试的缺陷,降低客户体验并增加工作量。缺陷是技术债,它对业务会造成负面影响。此外,由于编写代码的人和阅读、维护与扩展代码功能的人可能并不相同,因此请尽可能为其他同事提供文档。
对于客户来说,他们依赖用户友好的文档来了解产品的功能及其使用方式。并非所有用户都精通技术,因此在编写文档时应注意那些急需帮助的非技术背景用户的需要,并注意不要自视甚高。
作为有一定技术影响力的评审人,当发现代码中存在任何可能成为问题的坏味道时务必标记、评论并拒绝此次pull request,并请开发者重新提交。
评审时还需要确保没有使用异常进行程序流程的控制,确保产生的异常均指定了有意义的信息,以方便开发者和客户进行后续处理。
2.4.5 架构规范和设计模式
请确保新代码符合项目的架构规范与公司指定的编码方式(例如SOLID、DRY、YAGNI和面向对象编程)。此外,代码可以适时合理引入设计模式。
这里说的设计模式指GoF设计模式。GoF即Erich Gamma、Richard Helm、Ralph Johnson和John Vlissides,他们是《设计模式:可复用面向对象软件的基础》这本C++相关书籍的作者。
如今,这些设计模式在几乎所有的面向对象编程语言中广泛使用。我个人推荐https://www.dofactory.com/net/design-patterns这个网站。其中包含了GoF中的所有设计模式,包括定义、UML类图、参与者、结构化代码和一些真实代码案例。
GoF的设计模式包括创建型、结构型、行为型设计模式。创建型设计模式包括抽象工厂模式(Abstract Factory)、建造者模式(Builder)、工厂方法(Factory Method)、原型模式(Prototype)和单例模式(Singleton);结构型设计模式包括适配器模式(Adapter)、桥接模式(Bridge)、组合模式(Composite)、装饰器模式(Decorator)、外观/门面模式(Façade)、享元模式(Flyweight)和代理模式(Proxy);行为型模式有职责链模式(Chain of Responsibility)、命令模式(Command)、解释器模式(Interpreter)、迭代器模式(Iterator)、中介者模式(Mediator)、备忘录模式(Memento)、观察者模式(Observer)、状态模式(State)、策略模式(Strategy)、模板方法(Template Method)和访问者模式(Visitor)。
这些代码也应当合理地组织在命名空间和模块中。在评审时也需要注意它们是否过于简单或者进行了过度设计。
2.4.6 性能和安全性
代码的性能和安全性在评审时也需要进行考量:
- 代码的性能如何。
- 代码是否存在性能瓶颈。
- 代码是否能够防范SQL注入或者拒绝服务(Denial-of-Service,DoS)攻击。
- 代码是否对数据进行了充分的验证以保证数据的清洁。是否只有验证通过的数据才能存储在数据库中。
- 是否检查了用户界面、文档和错误消息中的拼写错误。
- 代码中是否有魔法数字(magic number)或者硬编码的值。
- 配置数据是否正确。
- 是否有意外提交的密钥。
完备的代码评审应当包含上述所有的方面并满足每种规则的评审参数。接下来我们来讨论一下执行代码评审的正确时机。