前言
这两天为了应付公司的安全检查，我负责使用一些代码检测工具扫描代码中的漏洞。
我使用了sonar工具扫描了我们团队的代码，发现了几个非常典型的BUG，拿出来给大家分享一下，希望对你会有所帮助。
1 空指针
经过sonar扫描之后，代码中发现最多的问题是空指针异常。
我排查之后发现，有些工具类比如：CollectionUtils.isEmpty()方法判空，或者用自定义的AssertUtil工具判空，sonar根本没办法识别到，因此，存在很多误报的情况。
虽说有误报，但还真的扫出了几个空指针异常的BUG。
有位同事的代码是这样写的：
public Date calcDate(Date date) {
Date date = getDate(date);
Date now = new Date();
if(date.after(now)) {
throw new BusinessException("时间不能比当前时间早");
}
//做时间的计算
return date;
}
private Date getDate(Date date) {
if(date == null) {
return null;
}
//做一些时间的处理
}很明显，如果date为空时，在调用date.after()方法的地方，就会报空指针异常。
当时那位同事这样写的想法是，传入的date不可能为空。
但这样的代码确实不太严谨，如果有一天date真的传入空值了，必定出问题。
好的代码习惯是要对date判空。
只需要把getDate()方法这样调整一下即可：
private Date getDate(Date date) {
if(date == null) {
throw new BusinessException("date不能为空");
}
//做一些时间的处理
}判断如果date为空时，抛一个运行时的业务异常。
2 给方法参数赋值
还有一个给方法参数赋值的问题，引起了我的兴趣。
那位同事的代码大概是这样的：
public Result handle(Contract contract,Date date,Date lastDate) {
Boolean currentFlag = true;
Map<String,Data> map = getMap(currentFlag,date,lastDate);
//其他很多业务逻辑
Result result = new Result();
result.setCurrentFlag(currentFlag);
result.setData(map);
return resut;
}
private Map<String,Data> getMap(Boolean currentFlag,
Date date,Date lastDate) {
List<Data> list1 = dataMapper.query(date);
if(CollectionUtils.isEmpty(list1)) {
currentFlag = false;
list2= dataMapper.query(lastDate);
Map<String,Data> map = toMap(list2);
return map;
}
return toMap(list1);
}这段代码的问题是：currentFlag在getMap方法中是方法的参数，在Java中是指传递，也就是说在getMap方法中currentFlag即使修改了，只在getMap方法内有效，不会影响到handle方法中的currentFlag变量。
这显然是一个非常低级的错误。
为什么会出现这个问题呢？
原来刚开始是没有getMap方法的，它的代码逻辑都包含到handle方法中，但由于handle方法中的代码非常多，checkstyle过不了。
因此那位同事，把一部分逻辑抽取到了getMap方法中，但由于没有正确处理currentFlag的赋值，因此产生了这个问题。
那么，如何解决这个问题呢？
可以使用Pair类，将currentFlag和map的值封装到一个对象即可。
public Result handle(Contract contract,Date date,Date lastDate) {
Pair<Boolean,Map<String,Data>> result = getMap(currentFlag,date,lastDate);
Boolean currentFlag = result.getKey();
Map<String,Data> map = result.getValue();
//其他很多业务逻辑
Result result = new Result();
result.setCurrentFlag(currentFlag);
result.setData(map);
return resut;
}
private Pair<Boolean,Map<String,Data>> getMap(Boolean currentFlag,
Date date,Date lastDate) {
List<Data> list1 = dataMapper.query(date);
if(CollectionUtils.isEmpty(list1)) {
currentFlag = false;
list2= dataMapper.query(lastDate);
Map<String,Data> map = toMap(list2);
return new Pair(false,map);
}
return new Pair(true,toMap(list1));
}3 四舍五入错误
还有一位同事，在处理四舍五入时，是这样处理的：
public long getValue(BigDecimal value) {
BigDecimal result = value.divide(new BigDecimal(100));
result.setScale(0, ROUND_HALF_UP);
return result.longValue();
}有些小伙伴可能会说，这个方法看起来没问题呀。
如果你仔细测试一下，会发现getValue方法结果并没有四舍五入。
因为代码中并没有获取result.setScale()方法的返回值。
正确的用法是这样：
public long getValue(BigDecimal value) {
BigDecimal result = value.divide(new BigDecimal(100));
return result.setScale(0, ROUND_HALF_UP).longValue();
}我们要获取result.setScale()方法的返回值，然后获取该值的整数值。
4 多余的条件判断
此外，还发现了某位同事在做逻辑判断时，有些奇怪的写法。
例如这样的：
public void fun(List<String> userList) {
if(CollectionUtils.isEmpty(userList) ||
(Collectionutils.isNotEmpty(userList) && userList.contains("苏三"))) {
//做业务逻辑处理
}
}这个判断条件是当userList为空时，或者不为空到包含苏三时满足条件，进行业务逻辑处理。
如果你仔细想一想会发现，Collectionutils.isNotEmpty(userList) && 这个判断完全是多余的。
判断userList.contains("苏三")时，userList肯定是不为空的。
前面的CollectionUtils.isEmpty(userList)已经判断了，如果userList为空，已经满足条件了做业务逻辑处理了。
因此代码可以优化成这样的：
public void fun(List<String> userList) {
if(CollectionUtils.isEmpty(userList) ||
userList.contains("苏三")) {
//做业务逻辑处理
}
}这几个错误都是代码中非常典型的错误，希望大家后面再写代码的时候，能够尽量避免，防微杜渐。
