代码语言：javascript复制
Code Review是软件开发过程中非常重要的一个环节，
其主要的目的是在项目早期找到和修正错误、提升软件质量。本文主要关注的是在做Code Review的时候，我们主要在关心代码的哪些方面来进行说明。
每个人的关注点不尽相同，于我而言，我的关注点一般在下面的几个部分上：
- 基础篇 - 包括编码规范、风格、日志规范、内存泄漏等
- 进阶篇 - 包括是否有较好的抽象、数据库变更检查等
- 高阶篇 - 包括应急方案、失败性考虑等
接下来，我们逐一来看一下。
基础篇
基本编码规范
- 类、方法命名是否规范、清晰
- 方法参数是否过多（比如6个以上）、方法体是否过大
- 重复代码
- 是否有“魔数”，比如 == 1 处理什么逻辑？(建议有良好的注释说明，定义常量或枚举代替)
- ... ...
日志规范
- 日志打印是否有意义，比如像如下几种，这种日志对问题排查等没有啥意义
logger.info("方法xxx执行");
... ...
logger.error("方法xxx出错～");- 日志打印可以合并的合起来打印
#合并前s
logger.info("name={}", name);
logger.info("type={}", type);
logger.info("timeCosted={} ms", timeCosted);
#合并后
logger.info("name={},type={},timeCosted={}", name,type,timeCosted);
- 日志是否有用MDC，如有需要检查是否合理的调用了remove / clear方法，防止线程间日志打印干扰等问题。
- ... ...
经验性检查
- SimpleDateFormat是否被定义成一个全局变量，如下代码，多线程将出现问题。
private static final SimpleDateFormat DEFAULT_FOMAT
= new SimpleDateFormat("yyyy-MM-dd");
public static Date formatDefaultDate(String date) {
try {
return DEFAULT_FOMAT.parse(date);
} catch (ParseException e) {
return null;
}
}SimpleDateFormat更多的信息请参考《SimpleDateFormat线程不安全示例及其解决方法》
- 金额处理是否使用BigDecimal类型，不能使用float和doube。
另外，BigDecimal对象创建，如果没有使用好，也可能出现问题。比如下面的示例：
代码语言：javascript复制 //0.299999999999999988897769753748434595763683319091796875
BigDecimal v1 = new BigDecimal(0.3d);
System.out.println(v1);
//0.3
BigDecimal v2 = new BigDecimal("0.3");
System.out.println(v2);
//0.3
BigDecimal v3 = BigDecimal.valueOf(0.3d);
System.out.println(v3);- 文件流使用后是否正常关闭。我们需要finally关闭流，以防止内存泄漏。
- ... ...
超时问题
- 新增的服务，如Dubbo服务，需要有超时设置
- 分布式锁需要有超时处理
- 调用下游接口或者调用各种中间件需要有超时的机制
- ... ...
代码职责和调用关系
- 代码职责是否清晰，比如UserService写了很多权限资源相关的逻辑处理方法就不好。
- 层次调用是否正确，一般Controller -> Service -> DAO 。如果Controller -> DAO就不好
- Dubbo服务不要调用调用自己的服务。 这个怎么理解呢？比如有一个UserRpcService，其有update和query的接口，假设，update里面需要做用户查询的操作做判断，刚好query能够满足。
public class UserRpcServiceImpl implements UserRpcService {
public int update(xxx) {
//调用query，而query也是一个rpc接口
query(xxx);
}
public int query(xxx) {
}
}不建议在rpc接口，再调用自身的rpc接口，如本示例的update和query。
- ... ...
线程池创建线程
- 对一些多线程逻辑，使用线程池创建线程，而不是通过new Thread等方式创建。
- ... ...
进阶篇
数据库方面检查
- 对表增加字段Alter table xxx add column xxxx ... ，需要检查下当前表中的记录数，如果数据量很大这个是不能做的。需要同DBA进行沟通。或者自己通过建一个新表（比原表多一个字段），用操作新表替换旧表来完成。这里需要完成原表数据迁移等其他内容。
- 索引添加是否合适
- 是否存在危险SQL，如update / delete 语句中的变量是否在业务能够保证有必要的值，不能出现很多值没有，导致if test 都不满足，导致更新的范围扩大。
<if test="xxx !=null ">- 查看是否有循环单个插入记录的情况，改成批量插入。
- ... ...
安全渗透方面检查
- 文件上传是不是只判断了文件后缀？ 只判断后缀，攻击方可以将一个jsp等文件伪装成jpg等格式的文件，从而成功上传到服务，导致服务器信息泄漏。
- 短信验证码对一个手机号，是否调用接口就能给用户发一个新的短信验证码，从而造成短信轰炸？我们需要在短信产生的有效期内保证不重新产生短信验证码。
- 接口是否存在越权查看等风险？比如A可以通过id查看属于B的设备信息？
- ... ...
接口保护检查
- 列表查询是否有pageSize的限制（如最多一次查询100条）。如果不限制，那么假设pageSize可以为5000条，那真的是简直了，对吧？
- 如果接口调用需要有次数限制，我们还要考虑是否对方法等有限流的措施？
- ... ...
模式应用
- 如果使用了D.C.L（Double-Check Lock），那么看是否有volatile修饰
- 抽象是否充分？比如，有不同类型的服务接口调用，主要有如下几个步骤：
1、参数校验
2、检查是否有流量
3、执行业务逻辑
4、记录调用日志
5、流量扣减
6、... ...如果每个服务都自己写一遍，不是很合适，也不不容易维护和扩展。在这种情况下，不同的服务调用主要是步骤#1和步骤#3有个性化的处理，可以抽象出来。
比如写一个简单的模版，步骤#1和步骤#3使用abstract方法，由子类具体实现。
抽象类
代码语言：javascript复制
public abstract class AbstractServiceHandler<T extends ServiceRequest> {
/**
* 模版方法
*/
public void handle(T param) {
// 1、验证
this.doValidate(param);
// 2、校验是否有流量
// 3、执行业务逻辑
doBusiness(param);
// 4、记录调用日志
// 5、流量扣减
// 6、 ... ...
}
// 子类做校验
protected abstract void doValidate(T param);
// 子类做业务逻辑
protected abstract void doBusiness(T param);
}
服务请求参数
代码语言：javascript复制import java.io.Serializable;
public class ServiceRequest implements Serializable {
private static final long serialVersionUID = 4314529075012784996L;
// 属性省略
}
决策流服务处理类
代码语言：javascript复制public class DecisionFlowHandler extends AbstractServiceHandler<ServiceRequest> {
@Override
protected void doValidate(ServiceRequest param) {
// TODO Auto-generated method stub
}
@Override
protected void doBusiness(ServiceRequest param) {
// TODO Auto-generated method stub
}
}
决策工具服务处理类
代码语言：javascript复制public class DecisionToolHandler extends AbstractServiceHandler<ServiceRequest> {
@Override
protected void doValidate(ServiceRequest param) {
// TODO Auto-generated method stub
}
@Override
protected void doBusiness(ServiceRequest param) {
// TODO Auto-generated method stub
}
}
- 可以考虑一些设计原则，比如单一职责/接口隔离等。
可以参考《设计模式几大原则》
- ... ...
高阶篇
在这个篇章部分，我们要对一些“失败”的场景，方案&应急有一定的考虑。
重试和幂等
- 针对系统间的数据同步，如果失败？我们是否有重试机制？
- 针对计费等场景，失败后重试调用，我们的接口是否支持幂等？
- 文件导入任务，如中断，我们是否有重启任务的机制，继续完成？
- ... ...
方案和应急
- 缓存对象增加属性（比如User加一个type），发布上线的时候，缓存的Key是否有做版本调整。如果升级后Key不变，可能导致Redis的value是由原服务更新，导致新改的内容上线后，可能还是取的原来的值（不包含type）。
- 比如在某些场景中，Redis缓存添加是不是有开关（一般由配置中心推送设置），以防止在缓存不是很正确的场景下，用数据库来保底
- 比如涉及数据迁移或者Redis集群升级（由5.0改成6.0）, 切流的计划是否合理？
- 有时候为了减少RPC调用或者少走网络，会结合Redis（分布式） Guava（本地缓存）结合使用，本地缓存更新的策略是什么？（集群下需要通过消息广播来达到快速更新各机器本地缓存的目的）
- 缓存存放的值是否为大对象，缓存个数多大？失败策略是什么？缓存雪崩/并发等场景是否有考虑等等
- ... ...
更多缓存的一些知识，读者可以从之前的文章《啊哈！缓存》了解更多内容。
小结
本文主要从关注代码本身，对Code Review做了简易的说明，想到啥就写了点啥。
Code Review对代码的关注点，远不止这些，本文也算是一个简单的抛砖引玉，有兴趣的读者可以一起留言探讨。
