code review到底在审查什么？

code review到底在审查什么？

在做一个代码的code review时，以下是我们需要关注的：

是否明晰

• 代码应该易于阅读，应该一目了然地了解每行和每个功能，否则，它太复杂了。

• 代码应符合模块内已使用的编码风格（缩进，间距，命名约定等）

• 每个函数或方法的预期意图都应简洁明了，并且一个函数不能超出一屏（几十行代码）。否则，它太复杂了。

• 代码是否适当地增加了注释，以供他人理解和维护？

注释是否简洁，易于理解，并且实际上为周围的代码增加了价值？

每个函数的意图是否明确，或者增加描述意图的最小注释是否会有所帮助？

API是否用Javadoc（Java）记录？

解释为什么和解释是什么一样重要，记录意图是最重要的记录形式，因为没有人知道你在想什么。

• 是否使用了不必要的和/或晦涩的语言技巧，即使仅仅30秒也可能使读者感到困惑？
• 每个函数是否桥接一个抽象级别，调用较低级别的函数？
• 是否存在跨越数十行的“函数”功能，这些功能可以轻松拆分为子函数？
• 代码被注释掉了吗？在极少数情况下，这是适当的。一般来说，注释掉的代码使阅读起来更困难，也很难理解-不要以为每个人都在使用语法突出显示。

正确性

• 有明显的错误吗？
• 有单元测试并且已经运行了吗？
• 单元测试是否足以测试代码（或代码的更改）？是否有代码覆盖工具会自动与构建一起运行？
• 是否是线程安全的？是否在类或API级别记录了线程安全性和并发性，以便调用者理解预期的语义？

设计意义

• 设计是否有意义（更改或不更改）？
• 更改是否适合现有设计（例如，新职责是否属于更改后的实体）？
• 所做的更改是否揭示了现有设计中的缺陷（例如，简单的行为更改需要复杂的代码更改）？
• 此更改是否完全解决了所有功能/问题的要求？需求的解释方式可能导致生产中的问题是否存在歧义？

重用和重复代码

• 包内的代码是否重复？如果在模块中甚至几行代码被重复复制了多次，则表明需要重构。
• 共享库中应该有局部函数吗？
• 是否存在可以完全删除或替换为现有库的代码？

配置和常量

• 是否在整个代码中散布了魔术数字或字符串常量，或更糟糕的是，重复多个地方使用这些常量？
• 是否有所有配置都设置了默认值？
• 配置文件是否增加了清晰的配置说明？

向后兼容和版本控制

• 该代码向后兼容其预先存在的API吗？
• 代码向后兼容其预先存在的配置吗？
• 是否存在并非必需的向后不兼容的更改？
• 如果存在向后不兼容的更改，将如何在软件包构建和部署方面进行管理？

依存关系

• 是否引入了新的外部依赖关系，是否会带来新的可用性或操作风险？
• 是否确实需要每个新库依赖项？是否每个现有依赖项都绝对必要？您能以更简单的方式解决问题吗？不要仅仅将最新和最出色的组件视为依赖项，因为您认为它们是浮华的。
• 是否有替代的“较轻量”依赖项可以替代使用？

故障处理

• 该代码可处理任何库代码调用返回的意外值？

• 代码对非法输入和空输入怎么处理？

• 该代码是否处理了边缘case？

• 代码是否有清晰一致的异常处理策略？

性能

• 是否有过早优化的迹象？
• 有没有明显的性能或效率问题可以解决？
• 如果大部分额外的时间都花在了代码阻塞上，代码执行时间也会更长吗？

性能指标metrics

• 该指标是否有助于运营活动？
• 是否容易知道已部署的代码在做什么？
• 从错误到调试的所有级别都有适当数量的日志记录吗？
• 代码中是否收集了性能指标？
• 每个日志消息是否都包含足够的相关信息，以便某人仅阅读日志消息就能了解系统的行为？
• 测试中是否包装了复杂的info / debug / verbose日志语句if (log level)，以免影响性能？

SQL

• 用户输入是否用于构建原始SQL？（请参阅SQL注入）
• 在检查之前是否已计划好所有SQL查询的解释？
• 是否在每个SQL语句上方以注释的形式签入了解释计划结果，作为开发人员期望查询执行的快照？在比较未来的解释计划以检查假设是否发生变化时，这可能非常有用。
• 是否正确使用了绑定变量，或者生成了一个关闭的SQL语句？
• 此更改是否需要回填或直接数据库更新，并且是否已审查了回填脚本？