I’ve this code:
private Iterable<Practitioner> pickPractitioners(List<String> ids) { return Optional.ofNullable(ids) .map(List::stream) .orElse(Stream.of()) .collect( Collectors.collectingAndThen( Collectors.toList(), this.practitionerRepository::findAllById ) ); }
Problem is that when ids
is empty, this.practitionerRepository::findAllById
is also executed.
I’d like to avoid this step if resulting collector is empty.
Any ideas?
Advertisement
Answer
Whilst the practical part of this question (how to avoid interrogating the repository with an empty list as an argument
) is already addressed in other answers I want to point out that there’s a cleaner way to build a pipeline in this method.
Firstly it’s worthy to remind that the main purpose of Optional.ofNullable()
is to create an Optional object that has to be returned from a method
.
Attempts to use Optional.ofNullable()
in order to utilize method-chaining or to avoid null-checks in the middle of the method according to Stuart Marks are considered to be anti-patterns.
Here is the quote from his talk at Devoxx:
“it’s generally a bad idea to create an Optional for the specific purpose of chaining methods from it to get a value.”
A similar idea was expressed in his answer on stackoverflow
.
What are the alternatives?
Since Java 9 Stream interface
has its own method ofNullable()
.
Returns a sequential Stream containing a single element, if non-null, otherwise returns an empty Stream.
Keeping all that in mind method pickPractitioners()
could be rewritten like this:
private Function<List<String>, Iterable<Practitioner>> getPractitioners = idList -> idList.isEmpty() ? Collections.emptyList() : this.practitionerRepository.findAllById(idList); private Iterable<Practitioner> pickPractitioners(List<String> ids) { return Stream.ofNullable(ids) .flatMap(List::stream) .collect(Collectors.collectingAndThen( Collectors.toList(), getPractitioners )); }