在做一个代码的code review时,以下是我们需要关注的:
是否明晰
代码应该易于阅读,应该一目了然地了解每行和每个功能,否则,它太复杂了。
代码应符合模块内已使用的编码风格(缩进,间距,命名约定等)
每个函数或方法的预期意图都应简洁明了,并且一个函数不能超出一屏(几十行代码)。否则,它太复杂了。
代码是否适当地增加了注释,以供他人理解和维护?
注释是否简洁,易于理解,并且实际上为周围的代码增加了价值?
每个函数的意图是否明确,或者增加描述意图的最小注释是否会有所帮助?
API是否用Javadoc(Java)记录?
解释为什么和解释是什么一样重要,记录意图是最重要的记录形式,因为没有人知道你在想什么。
- 是否使用了不必要的和/或晦涩的语言技巧,即使仅仅30秒也可能使读者感到困惑?
- 每个函数是否桥接一个抽象级别,调用较低级别的函数?
- 是否存在跨越数十行的“函数”功能,这些功能可以轻松拆分为子函数?
- 代码被注释掉了吗?在极少数情况下,这是适当的。一般来说,注释掉的代码使阅读起来更困难,也很难理解-不要以为每个人都在使用语法突出显示。
正确性
- 有明显的错误吗?
- 有单元测试并且已经运行了吗?
- 单元测试是否足以测试代码(或代码的更改)?是否有代码覆盖工具会自动与构建一起运行?
- 是否是线程安全的?是否在类或API级别记录了线程安全性和并发性,以便调用者理解预期的语义?
设计意义
- 设计是否有意义(更改或不更改)?
- 更改是否适合现有设计(例如,新职责是否属于更改后的实体)?
- 所做的更改是否揭示了现有设计中的缺陷(例如,简单的行为更改需要复杂的代码更改)?
- 此更改是否完全解决了所有功能/问题的要求?需求的解释方式可能导致生产中的问题是否存在歧义?
重用和重复代码
- 包内的代码是否重复?如果在模块中甚至几行代码被重复复制了多次,则表明需要重构。
- 共享库中应该有局部函数吗?
- 是否存在可以完全删除或替换为现有库的代码?
配置和常量
- 是否在整个代码中散布了魔术数字或字符串常量,或更糟糕的是,重复多个地方使用这些常量?
- 是否有所有配置都设置了默认值?
- 配置文件是否增加了清晰的配置说明?
向后兼容和版本控制
- 该代码向后兼容其预先存在的API吗?
- 代码向后兼容其预先存在的配置吗?
- 是否存在并非必需的向后不兼容的更改?
- 如果存在向后不兼容的更改,将如何在软件包构建和部署方面进行管理?
依存关系
- 是否引入了新的外部依赖关系,是否会带来新的可用性或操作风险?
- 是否确实需要每个新库依赖项?是否每个现有依赖项都绝对必要?您能以更简单的方式解决问题吗?不要仅仅将最新和最出色的组件视为依赖项,因为您认为它们是浮华的。
- 是否有替代的“较轻量”依赖项可以替代使用?
故障处理
该代码可处理任何库代码调用返回的意外值?
代码对非法输入和空输入怎么处理?
该代码是否处理了边缘case?
代码是否有清晰一致的异常处理策略?
性能
- 是否有过早优化的迹象?
- 有没有明显的性能或效率问题可以解决?
- 如果大部分额外的时间都花在了代码阻塞上,代码执行时间也会更长吗?
性能指标metrics
- 该指标是否有助于运营活动?
- 是否容易知道已部署的代码在做什么?
- 从错误到调试的所有级别都有适当数量的日志记录吗?
- 代码中是否收集了性能指标?
- 每个日志消息是否都包含足够的相关信息,以便某人仅阅读日志消息就能了解系统的行为?
- 测试中是否包装了复杂的info / debug / verbose日志语句if (log level),以免影响性能?
SQL
- 用户输入是否用于构建原始SQL?(请参阅SQL注入)
- 在检查之前是否已计划好所有SQL查询的解释?
- 是否在每个SQL语句上方以注释的形式签入了解释计划结果,作为开发人员期望查询执行的快照?在比较未来的解释计划以检查假设是否发生变化时,这可能非常有用。
- 是否正确使用了绑定变量,或者生成了一个关闭的SQL语句?
- 此更改是否需要回填或直接数据库更新,并且是否已审查了回填脚本?