CODEWITHSUNDEEP
javajava-stream
Jordi
Jordi

Reputation: 23187

Java Streams; avoid finisher on Collectors.collectingAndThen

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?

Upvotes: 1

Views: 1335

Answers (3)

Alexander Ivanchenko
Alexander Ivanchenko

Reputation: 28968

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
            ));
}

Upvotes: 2

Alex - GlassEditor.com
Alex - GlassEditor.com

Reputation: 15497

In general to skip that part of the finisher you could pass a lambda instead of a method reference and check if the input is empty:

    .collect(
        Collectors.collectingAndThen(
            Collectors.toList(),
            r -> r.isEmpty() ? Collections.emptyList() : this.practitionerRepository.findAllById(r)
        )
    );

If your actual code is a simple as this example then you don't need to use streams or optional at all. Instead you could just check if the input of the method is null or empty in a ternary operator:

    return ids == null || ids.isEmpty() ? Collections.emptyList() :
        this.practitionerRepository.findAllById(ids);

Upvotes: 6

jcompetence
jcompetence

Reputation: 8383

If you look at the signature of the Finisher. It is just a function, so you can just write it:

public static<T,A,R,RR> Collector<T,A,RR> collectingAndThen(Collector<T,A,R> downstream, Function<R,RR> finisher) {

static interface MyRepository extends JpaRepository<Part, Long> {

}

public static void main(String[] args) {

    MyRepository myRepository = null;
    List<Long> list = null;

    Function<List<Long>, List<Part>> finisher = (ids) -> {

        return ids.isEmpty() ? Collections.emptyList() : myRepository.findAllById(ids);

    };

    Optional.ofNullable(list)
            .map(List::stream)
            .orElse(Stream.of())
            .collect(
                    Collectors.collectingAndThen(
                            Collectors.toList(),
                            finisher
                    )
            );

}

Upvotes: 2

Related Questions