1.常见问题避坑记录

1.1 为什么写这个常见问题避坑记录？

看到公司同事的代码经常写的不规范，出现比较多低级错误，所以写这个文章说明常见的各种问题，后续将持续完善该文章，不断积累记录的同时提高自身代码质量。

1.2 NPE问题

1.2.1 非正确使用对象以及属性

Person person = personMapper.selectById(123L);
if (!person.getUserName().isEmpty()) {
    // doSomething...
}

上述代码中想实现的效果是，查询id为123的person信息，然后判断如果userName不为空，就进行逻辑的处理，但是通过观察代码，可以直接发现2处错误

1. 没有处理person为null的情况
2. 没有处理person.getUserName()为null的情况

为什么会这么说呢，来依次验证一下，首先是数据库里是没有准备id为123的person信息，所以这里肯定是查不到信息的，开始测试：

1. 验证情况1，不处理person为null的情况，直接进行使用

Person person = personMapper.selectById(123L);
String userName = person.getUserName();

运行结果

原因分析

可以看到上述代码在查询不到person信息的时候，直接调用person.getUserName()确实出现了NPE，因为person查不到就代表person == null,null.getUserName()肯定就NPE了

1. 验证情况2，查询到person信息之后，不正确的使用person上的属性，例如person.getUserName() 我们把上面的代码稍微改改，对person进行处理之后再用它的属性,这里我提前准备好了一条id为1685291564680609793的数据，这条数据的userName为null，开始测试：

Person person = personMapper.selectById(1685291564680609793L);
if (Objects.nonNull(person) && !person.getUserName().isEmpty()) {
    // doSomething...
}

运行结果

原因分析

上述代码的本意是在userName不为空的时候进行逻辑处理，但是判断的方式却出现了错误。如果person.getUserName()为null，那么person.getUserName().isEmpty()就等价于null.isEmpty()，因此就会出现NPE

那么我们应该如何正确判断字符串为空呢，我们可以使用别人造好的轮子,直接引入这个依赖

<dependency>
    <groupId>org.apache.commons</groupId>
    <artifactId>commons-lang3</artifactId>
    <version>3.9</version>
</dependency>

然后我们就可以使用StringUtils.isNotEmpty()方法来帮我们正确的判断字符串的为空 稍微修改一下上面的代码：

Person person = personMapper.selectById(1685291564680609793L);
if (Objects.nonNull(person) && StringUtils.isNotEmpty(person.getUserName())) {
    // doSomething...
}

这样我们就可以正确的判断字符串为空了

补充说明

StringUtils.isEmpty()和StringUtils.isBlank()的作用不一样，前者不允许字符串中有空格，只判断null和length，后者允许存在空格

1.2.2 非正确使用集合进行查询

有的时候我们需要通过服务调用或者方法传递过来的参数进行查询，假设下面的nameList的来源是外部，不能确定传进来的是什么，可能是null、可能是[]、也可能是正常的list，那么我们直接调用下面的方法就有可能出现问题，如果nameList为null的时候就会出现NPE

List<String> nameList = null;  
List<Person> people = personMapper.selectList(Wrappers.<Person>lambdaQuery().in(Person::getUserName, nameList));

运行结果

org.mybatis.spring.MyBatisSystemException: nested exception is org.apache.ibatis.builder.BuilderException: Error evaluating expression 'ew.sqlSegment != null and ew.sqlSegment != '' and ew.nonEmptyOfNormal'. Cause: org.apache.ibatis.ognl.OgnlException: sqlSegment [java.lang.NullPointerException]

原因分析

nameList为null，条件为in的时候传给mybatis一个null会导致NPE。那么应该如何解决呢？需要结合具体的场景，可以在service层就处理nameList，保证调用到这里的时候nameList不为null，或者如果一定要在这里调用的话，可以在in前面加上条件，修改之后的代码如下，这样就不会NPE

List<String> nameList = null;
List<Person> people = personMapper.selectList(Wrappers.<Person>lambdaQuery()
        .in(!CollectionUtils.isEmpty(nameList), Person::getUserName, nameList));

1.2.3 equals的不正确使用

在方法中，直接用入参调用equals,这样如果key为null的时候就会出现NPE

public void testMethod(Integer key) {
    if (key.equals(1)) {
        // doSomething...
    }
}

改进如下：

1. 把已知常量作为equals的调用方

if (1.equals(key)) {
    // doSomething...
}

1. 使用Objects.equals方法

if (Objects.equals(1, key)) {
    // doSomething...
}

1.3 "多此一举"问题

1.3.1 mybatis查询集合之后做多余的判断

List<Person> list = personMapper.selectList(Wrappers.<Person>lambdaQuery().eq(Person::getId, 123L));  
System.out.println("list = " + list);  
if (list != null && list.isEmpty()) {  
// doSomething ...  
}

查看控制台输出的list

JDBC Connection [HikariProxyConnection@1703458581 wrapping com.mysql.cj.jdbc.ConnectionImpl@15a3b42] will not be managed by Spring
==>  Preparing: SELECT id,user_name,sex,age,deleted,version,create_time,update_time FROM person WHERE deleted=0 AND (id = ?)
==> Parameters: 123(Long)
<==      Total: 0
Closing non transactional SqlSession [org.apache.ibatis.session.defaults.DefaultSqlSession@688a2c09]
list = []

可以看到，就算查询不到数据，mybatis给我们返回的也是个空数组，不会出现结果为null的情况，因此可以删除list != null这个判断条件

补充说明

如果某些情况需要对查询到的list进行判空，然后安全的进行处理，大可不必直接写list != null || list.isEmpty(),spring给我们提供了很多个工具类，我们可以直接用，简化开发，不要重复造轮子

1. CollectionUtils.isEmpty()

List<String> list;  
if (!CollectionUtils.isEmpty(list)) {  
    // doSomething...  
}

具体实现：

public static boolean isEmpty(@Nullable Collection<?> collection) {
    return collection == null || collection.isEmpty();
}

2.ObjectUtils.isEmpty()

List<String> list = new ArrayList<>();
if (!ObjectUtils.isEmpty(list)) {
    // doSomething...
}

具体实现:

public static boolean isEmpty(@Nullable Object obj) {
    if (obj == null) {
        return true;
    } else if (obj instanceof Optional) {
        return !((Optional)obj).isPresent();
    } else if (obj instanceof CharSequence) {
        return ((CharSequence)obj).length() == 0;
    } else if (obj.getClass().isArray()) {
        return Array.getLength(obj) == 0;
    } else if (obj instanceof Collection) {
        return ((Collection)obj).isEmpty();
    } else {
        return obj instanceof Map ? ((Map)obj).isEmpty() : false;
    }
}

1.4 魔法值问题

Person person = new Person().setSex(0);  
personMapper.insert(person);

如上述代码所示，新添加一个Person的信息，设置sex为0，可能自己开发的时候知道0代表的是男还是女，但是其他同事来阅读代码会出现不知道这个0是什么意思，代表的是男还是女，得翻来覆去的找，浪费时间。因此我们在写代码的时候应该注意，应该根据具体场景采用枚举或者常量来代表魔法值，如果是常量需要判断该常量需要作为当前类的常量还是需要作为全局常量来使用，优化代码：

1. 新建枚举

@Getter
@AllArgsConstructor
public enum PersonEnum {

    /**
     * 性别:男
     */
    SEX_MALE("男", 0),

    /**
     * 性别:女
     */
    SEX_FEMALE("女", 1);

    private final String key;

    private final Integer value;

}

1. 修改原代码

Person person = new Person().setSex(PersonEnum.SEX_MALE.getValue());  
personMapper.insert(person);

可以看到修改之后可读性变强了。

1.5 代码逻辑问题

List<Person> personList;
if (!personList.isEmpty() || peopleList != null) {
    // doSomething...
}

假设上面的personList为null，那这个if判断是不是又NPE了，怎么可以先把isEmpty()放到前面先判断呢，而且peopleList != null放到 || 的后面，就算能执行到这里，肯定永远是true，所以像这种错误是可以避免的

1.6 方法命名不清晰

现有个方法，想实现根据userName(用户姓名)查询最近20条操作记录。

错误示例：public List<OperationRecord> selectTwenty(String userName)

为什么说上面的方法命名不清晰呢？

首先我第一眼看到这个方法，我不知道查询20什么，然后方法里又有条件，会让人产生疑惑，看不懂这个代码，还需要进行推理才能知道要表达的意思，因此才说这个方法命名不清晰。

如何改进？

一个方法的命名无非就是为了见名知意，让其他人以最小的代价就能知道你这个方法做的事情。无非就是为了告诉别人，你这个方法是通过xxx查询xxx，如果改进成下面这个名字，是不是更加清晰呢？ public List<OperationRecord> selectLatestTwentyOperationRecordListByUserName(String userName) 是不是相对上面来说就清晰得多了。 如果看到这里觉得比较长，没关系，还可以进一步优化，如果所处的上下文都是只查询操作记录表的信息，那么我们可以优化为public List<OperationRecord> selectLatestTwentyListByUserName(String userName)，提高代码可读性的同时减少了方法长度

1.7 方法返回值不统一

private List<Person> selectAllPersonList() {
    List<Person> list;
    if ("xxx".equals("xxx")) {
        return null;
    }
    return list;
}

上述代码的问题在于如果正常返回的时候是一个List<Person>，但是if代码块里又有返回null的，这个给到前端处理起来可能会比较麻烦，应该改成统一的格式，如果没查询到数据，应该也是返回空数组，所以应该修改一下，保持返回数据格式的统一

if ("xxx".equals("xxx")) {
    return Collections.emptyList();
}

1.8 常量位置存放错误

这个主要是发现前端有一些常量提取到了api/xxx/xxx.ts下，在项目结构中，这里是存专门用于请求的api，不是存常量，把常量也放到这里这样会弄得很混乱，要按照分类把文件放到它应该在的地方......

1.9 方法修改之后没有同步注释

/**
 * 根据用户工号查询用户信息
 *
 * @param userName 用户姓名
 * @return 用户信息集合
 */
public List<Person> selectPersonListByName(String userName) {

}

上述代码的问题在于，注释写的是根据用户工号查询用户信息，但是参数又是姓名，这样会产生歧义，如果修改方法之后注释与方法的作用不一致，需要把方法的注释进行同步

2 总结

上述代码问题都是比较常见的，并且可以避免掉的，不然写出来的代码肯定存在很多问题。当然了，上面只是很小的一部分，还有其他的常见的问题需要在实际开发过程中多思考，不断总结改进。