老司机带你走近CodeReview

背景

Code Review 很重要，但是一个团队并不是所有人都是老司机，有很多同学是没有代码 review 经验的，他们往往不知道应该重点 check哪些点。

结合业务特点和之前踩过的坑，制定一个核心点的checklist是非常必要的

• 什么写法可能导致性能低下？
• 如何保障业务一致性？
• 哪些接口要慎用？
• 什么情况下可以做重试？
• 哪些设计方式需要规避？
• 什么习惯容易引发内存泄漏？

这样可以让团队在review时，能有的放矢，过程中逐步积累起经验，共同成长。严格来说，CR应该在提测之前，包含业务逻辑实现，代码优化重构等所有代码改动过的地方，同时应有测试参与评审，评估影响面，以便完善测试用例。

代码规范检查点

检查点	建议	案例
命名规范	方法命名与代码逻辑相符	如一个getXXX、里面尽量不包含add、update等操作
最小出入参	参数不宜过多、只传必要的参数、参数需要明确；若参数为对象、则只需要包含必须的成员变量	略
条件判断	多重条件拆分为有语义的多个单个条件的组合	if((!A && !B) && C) 可以将(!A && !B) 等价拆分为 xxx.isAvaliable等
必要的日志	关键步骤打印日志、特别是对外外部依赖的调用方便排查问题	没有日志等于瞎子
初始日志需要检车输出数量和大小	超长的日志需要截取、只输出关键信息	日志过大会影响应用性能
慎重处理异常	谨慎定义业务异常、对于代码错误导致的异常需要统一拦截处理	比如统一异常拦截器、不要吃掉业务异常
注意代码溢出	进行运算操作需要考虑溢出	略
轻量级的DAO层	尽量避免连表查询、如果有尽量控制在2个表以内、尽量避免业务处理	略
魔法数字	需要定义具有业务含义的枚举	略
高内聚低耦合、圈复杂度小	一个方法尽量在100行以内、单个类的长度尽量在1000行以内	不内聚的代码容易成为屎山

业务逻辑检查点

检查点	建议	案例
空指针问题	调用任何对象之前考虑一下这个对象是否可能为空、提前做好防御	略
if判断要完整	多条件的判断尽量写成枚举、枚举的判断一定要高内聚、写在枚举类中	略
数据库字段保护	根据业务和前端定义的字段长度、服务端做参数校验、并在数据库做相应的字段长度限制	特别的是varchar类型、在落库之前进行预防
新老版本接口是否兼容	接口改动要兼容老版本、业务改动大的、建议直接使用新版本	如果客户端不做兼容、那么服务端要做向下兼容
新老版本数据是否兼容	通过数据订正保持新老版本一直	略
数据不兼容是否影响上下游	如果做了数据清洗、一定要周知到受影响的业务方	比如中台应用数据变动
安全问题	接口需要鉴权、数据需要做权限	通过统一组件拦截
敏感信息是否加密	银行卡、身份证号等敏感信息	数据泄露

技术架构检查点

检查点	建议	案例
改动范围评估	对多个模块依赖的公共接口的修改容易造成影响范围超过当前业务改动、影响其他调用该函数的模块、需要特别注意	略
加锁是否考虑全面	加锁尽量在最小力度、同时需要考虑是否会出现死锁、在集群模式下需要使用分布式锁	A代码中耗时较长没有释放锁、导致B代码加锁失败影响业务
并发写问题	根据业务情况使用分布式锁、数据库乐观锁以及悲观锁	并发写可能会导致顺序不一致
业务一致性问题	根据业务情况使用分布式事务、可靠消息、任务补偿达成一致性	略
客户端和服务端数据一致性	推拉结合	略
消息消费需要考虑乱序问题	如果业务对消息顺序有要求需要版本号机制或者状态判断	乱序消息导致业务受损
读写分离、主从延迟	根据业务情况做到数据库读写分离、合理的查主库	略
缓存和数据库一致性问题	查数据库后更新缓存、做好缓存管理链路	数据受损
是否有大事务	对于高并发、高响应的接口进行事务拆分、通过异步、补偿等完成最终一致性	大事务耗时较长并且容易打满数据库连接池
超时重试	网络抖动、设置合理的重试次数以及下游幂等处理	略
黄金链路保护	核心链路做好限流、熔断、降级等措施、非核心业务可以降级	流量激增

代码性能检查点

检查点	建议	案例
是否有可以优化的for循环	减少圈复杂度并且替换掉for循环操作数据库	略
是否有大对象	对于产生的对象大小需要评估、特别是一些list和map、考虑内存长期存活的对象	查数据库没有做限制
是否可能阻塞线程池	做好线程池隔离	非核心线程池可能会影响核心线程池
高并发查询	读多写少的场景做好缓存	略
是否有慢查询	explain以下、查看sql健康状态	一个完美的架构也可能会被慢SQL拖垮