I'm trying to add two nullable list using below:

List<Employee> employees = null;
if (<some condition>) {
    employees = employeeService.getEmployees(<some criteria>);

    // Add another list of employees
    if (<some condition>) {
        List<Employee> employeesSubList = employeeService.getEmployees(<some other criteria>));
        if (!isEmpty(employeesSubList)) {
            if (!isEmpty(employees)) {
                employees.addAll(employeesSubList);
            } else {
                employees = employeesSubList;
            }
        }
    }
}

This works but the code looks horribly ugly. There is a hard condition that the parent List<Employee> employees will be null instead of empty list in case no employees are present. Is there a cleaner way to do the same?

I tried Java 8 approach but IntelliJ throws some warnings on .orElse(emptyList()).addAll(employeesSubList):

List<Employee> employees = null;
if (<some condition>) {
    employees = employeeService.getEmployees(<some criteria>);

    // Add another list of employees
    if (<some condition>) {
        List<Employee> employeesSubList = employeeService.getEmployees(<some other criteria>));
        if (!isEmpty(employeesSubList)) {
            Optional.ofNullable(employees).orElse(emptyList()).addAll(employeesSubList);
        }
    }
}

CodePudding user response：

The "problem" Intellij is probably telling you is that emptyList().addAll() is never assigned to anything, and it returns a boolean, so you need a different way to get that data

For example

    List<Employee> employeesSubList = Optional.ofNullable(employeeService.getEmployees(<some other criteria>))).orElse(emptyList());
    if (!isEmpty(employeesSubList)) {
        // assumes employees is not null 
        employees.addAll(employeesSubList);
    }

In fact, you could get rid of the if statement because adding an empty list to another list is a no-op

CodePudding user response：

Is there a cleaner way?

IMO no.

• You could argue that testing for null is cleaner than calling an isEmpty method, but I'm not convinced.

• You could argue that hiding the null tests behind an Optional is cleaner, but I'm not convinced.

But the real problem is this:

There is a hard condition that the parent List employees will be null instead of empty list.

That is what is causing you to have to deal with the null-safety issue. The clean solution is to use an empty list; i.e. a list with no elements.

Then you can rewrite your example code as:

List<Employee> employees = Collections.emptyList();
if (<some condition>) {
    employees = employeeService.getEmployees(<some criteria>);

    // Add another list of employees
    if (<some condition>) {
        List<Employee> employeesSubList = employeeService.getEmployees(<some other criteria>));
        if (!employees.isEmpty()) {
            employees.addAll(employeesSubList);
        } else {
            employees = employeesSubList;
        }
    }
}

(Note: if you use Collections.emptyList(), be aware that it returns an immutable list. So you can't add elements to it. The alternative is to use new ArrayList<>() to create a mutable list that is initially empty.)

In short, if you want "clean" you are looking the wrong part of your code-base. IMO.