Home > Back-end >  Java mutable object to causing nullpointer exception
Java mutable object to causing nullpointer exception

Time:10-05

I have the following DTO and I pass the objects to the ArrayLists to prevent objects to be changed and fix the SonarQube error as "Message: Store a copy of allergenInfoList", etc.

public MenuItemDTO(
        PropertiesDTO propertiesDto,
        List<ModifierDTO> modifierDtoList,
        List<AllergenInfo> allergenInfoList
) {
    this.uuid = propertiesDto.getUuid();
    this.modifierDtoList = new ArrayList<>(modifierDtoList);
    this.allergenInfoList = new ArrayList<>(allergenInfoList);
  }
}

However, this approach requires null check and it makes my code ugly as shown below:

public MenuItemDTO(
        PropertiesDTO propertiesDto,
        List<ModifierDTO> modifierDtoList,
        List<AllergenInfo> allergenInfoList
) {
    this.uuid = propertiesDto.getUuid();
    if (modifierDtoList != null) {
        this.modifierDtoList = new ArrayList<>(modifierDtoList);
    }
    if (allergenInfoList != null) {
        this.allergenInfoList = new ArrayList<>(allergenInfoList);
    }
}

So, is there any better approach to fix the problem without null check?

CodePudding user response:

A sleeker solution which may potentially avoid the need to create a stream and collect the elements:

Check this for instance:

private <T> List<T> copyList(List<T> list) {
    return Optional.ofNullable(list)
        .map(ArrayList::new)
        .orElseGet(ArrayList::new);
}

This effectively check whether the passed in list is null in not, goes ahead and copies it. Otherwise, it simply creates a new empty list. On top of that it also takes care of generics and typing.

CodePudding user response:

It may be better to implement a utility/helper method to handle null checks (either directly, using Objects::isNull or Optional) and return expected result:

public class Util {
    public static List<?> copyOrNull(List<?> src) {
        return null == src ? src : new ArrayList<>(src);
    }

    public static List<?> copyOrEmpty(List<?> src) {
        return null == src ? Collections.emptyList() : new ArrayList<>(src);
    }
}

Then update the DTO code as needed:


public MenuItemDTO(
        PropertiesDTO propertiesDto,
        List<ModifierDTO> modifierDtoList,
        List<AllergenInfo> allergenInfoList
) {
    this.uuid = propertiesDto.getUuid();
    this.modifierDtoList = Util.copyOrNull(modifierDtoList);
    this.allergenInfoList = Util.copyOrEmpty(allergenInfoList);
}

CodePudding user response:

Here is a solution that deals with the possibility of having a null list as input parameter, but it needs way more code than a simple null check

Create a method like:

private <T> List<T> copyList(List<T> list) {
    return Optional.ofNullable(list)
            .map(List::stream)
            .orElseGet(Stream::empty)
            .collect(Collectors.toList());
}

OR just use a similar method and perform the if null check only once inside this method:

private <T> List<T> copyList(List<T> list) {
    if (list != null) {
        return new ArrayList<>(list);
    }
    return new ArrayList<>();
}

And use this method on your constructor, this way it looks more elegant:

public MenuItemDTO(
        PropertiesDTO propertiesDto,
        List<ModifierDTO> modifierDtoList,
        List<AllergenInfo> allergenInfoList
) {
    this.uuid = propertiesDto.getUuid();
    this.modifierDtoList = copyList(modifierDtoList);
    this.allergenInfoList = copyList(allergenInfoList);
   
}
  • Related