I have the following DTO and I pass the objects to the ArrayList
s 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);
}