代码审查查什么？
2018-08-05 本文已影响0人
weihongyu12
不应该做的
- 格式：在什么地方放置空格和断行符？使用制表符还是空格？大括号如何放置？
- 风格：变量、参数是否声明为final？函数变量的定义是否与调用代码或函数开始代码太相似？
- 命名：域、常量、变量、参数、类的名称是否符合标准？命名是否太短？
- 覆盖测试：这段代码是否进行了测试？
应该做的
基本原则
设计
- 新代码是否与整体架构匹配？
- 代码是否遵循SOLID原则、领域驱动设计以及团队喜爱的其它设计模式。
- 新代码采用哪些设计模式？这些模式合适吗？
- 如果代码库采用混合标准或设计风格，新代码是否符合当前风格？代码迁移是按正确的方向进行，还是效仿即将被淘汰的陈旧代码？
- 代码是否处于正确的位置？例如，如果代码执行与顺序有关，它是否能按顺序执行？
- 新代码能否复用部分已存在代码？新代码能否给已存在代码提供复用部分？
- 新代码是否包含冗余代码？如果包含，是应该重构成更加可复用部分，还是在这个阶段能接受这种冗余？
- 代码是否超出设计标准？这种复用构造现在是不是不需要？团队如何根据YAGNI权衡复用？
可读性和可维护性
- 命名（字段、变量、参数、函数以及类）能否反映它们代表的事物？
- 通过阅读代码，我能否理解代码的目的？
- 我能否理解测试的目的？
- 测试是否覆盖了绝大部分情况？是否覆盖常见情况和异常情况？是否存在没考虑到的情况？
- 异常错误消息是否易于理解？
- 难以理解的代码段是否进行了备注、评论或者使用易懂的测试案例进行覆盖（符合团队的偏好）？
功能
- 代码的实际工作是否符合预期？如果有自动化测试来确保代码的正确，测试能否测出代码满足约定要求？
- 代码看上去是否含有细微错误，比如使用错误变量进行检查，或者把 or 误用为 and？
其他
- 代码中是否存在潜在的安全问题？
- 是否需要满足规范要求？
- 对于没有覆盖自动化性能测试的代码段，新代码是否引入了不可避免的性能问题，比如不必要的数据调用或远程服务？
- 作者是否需要创建公共文档，或者修改现有的帮助文档。
- 展示给用户的消息是否检查无误？
- 是否存在导致产品崩溃的明显错误？代码是否会意外指向测试数据库，或者是否有应该替换成真正服务的硬编码存根代码？
测试
- 新的或修改的代码有测试吗？
- 测试有覆盖到代码中令人困惑或者复杂的部分吗？
- 我能理解这些测试么？
- 这些测试需要满足什么要求？
- 我可以考虑没有被现有测试覆盖到的用例吗？
- 这些测试是否有说明代码的限制条件？
- 审查代码中的测试类型、测试级别正确吗？
- 有没有针对安全性的测试？
- 审查者也可以写测试
性能
- 这部分功能有硬性的性能需求吗？
- 如果有，有测试去检查吗？
- 功能修改或新功能对已有性能测试结果有不良影响吗？
- 代码审查中没有硬性的性能需求要怎么做？
- 数据库调用是否影响性能？
- 是否有不必要的网络调用？
- 移动程序、可穿戴程序是否过于频繁地调用后端？
- 代码通过锁来访问共享代码吗？这样会导致性能下降和死锁吗？
- 代码会造成内存泄漏吗？
- 应用程序的内存会无限增加吗？
- 代码有关闭连接或流吗？
- 资源池配置正确吗？
- 如果你检查包含反射的代码，问一下反射是不是一定需要？
- 如果超时，系统中的其他部分会受到什么影响？
- 代码中有使用多个线程来执行一个简单的操作吗？除了增加时间和复杂性外，却没有带来性能的提高？代码使用并行流机制，但是却没有从并行性中受益吗？
- 多线程环境下的代码使用了正确的数据结构吗？
- 代码容易出现竞态条件吗？
- 代码中对锁的使用正确吗？
- 代码的性能测试有价值吗？
- 如果审查的代码使用了缓存，不正确的缓存项失效是否进行处理？
代码级优化
- 代码中是否使用了不必要的同步、锁？如果代码总是执行在单线程下，加锁只会带来额外开销。
- 代码中是否使用了不必要的线程安全数据结构？例如，可以用ArrayList 替换 Vector 吗？
- 代码中是否使用了在常见操作上性能很差的数据结构？例如，使用了链表结构，却经常搜索其中的一项。
- 代码是否使用了锁或者同步机制，而实际上可以用原子变量替代？
- 代码可以得益于延迟加载吗？
- if 语句或其他逻辑语句能通过短路机制进行优化吗？比如把最快的计算放在条件的开始？
- 有很多的字符串格式吗？可以更有效率吗？
- log 语句有使用字符串格式化吗？有用检查log级别的 if 语句包起来或者使用的日志提供者可以进行延迟操作？
数据结构
- 数据结构是否被正确的使用？
- 是否使用了反模式？
SOLID原则
SOLID代表：
- S – 单一职责原则：不应该有多种情况需要修改某个类的对象
- O – 开放封闭原则：软件实体应该对扩展开放，对修改封闭
- L – 里氏替换原则：函数使用基类的引用，必须能够在不知不觉的情况下使用派生类的对象
- I – 接口分离原则：多个客户端特定的接口比使用一个通用的接口要好
- D – 依赖倒置原则：依赖抽象。不要依赖于具体实现
一些代码异味可能意味着可能已经违反了一个或多个SOLID原则：
- 很长的if/else语句
- 强制转换成一个子类型
- 很多公共方法
- 实现的方法抛出UnsupportedOperationException异常
安全
- 了解第三方库
- 检查是否需要对新路径和服务进行身份验证？
- 您的数据是否需要加密？
- 密码、加密密钥、token等秘密数据是否被正确管理？
- 代码应该记录/审计行为吗？这样做是否正确？
- 代码是否会进行任何数据更改（例如添加/更新/删除）？它是否应该记录所做的更改，由谁以及何时进行？
- 此代码是否在某些性能关键路径上？是否应该在某种性能监控系统中记录启动时间和结束时间？
- 任何记录的消息的日志记录级别是否合适？
