Do I need a custom Spliterator to avoid extra .stream() call?

Question

I have this code which works fine, but I find it ugly.

@EqualsAndHashCode
public abstract class Actions {

    @Getter
    private List<ActionsBloc> blocs;

    public Actions mergeWith(@NotNull Actions other) {

        this.blocs = Stream.of(this.blocs, other.blocs)
                                    .flatMap(Collection::stream)
                                    .collect(groupingBy(ActionsBloc::getClass, reducing(ActionsBloc::mergeWith)))
                                    .values()
                                    .stream()
                                    .filter(Optional::isPresent)
                                    .map(Optional::get)
                                    .collect(toList());

        return this;
    }
}

ActionsBloc is a super type which contains a list of Action.

public interface ActionsBloc {

    <T extends Action> List<T> actions();

    default ActionsBloc mergeWith(ActionsBloc ab) {
        this.actions().addAll(ab.actions());
        return this;
    }
}

What I want to do is merge blocs of Actions together based on the Class type. So I'm grouping by ActionsBloc::getClass and then merge by calling ActionsBloc::mergeWith.

What I find ugly is calling the values().stream() after the first stream was ended on collect.

Is there a way to operate only on one stream and get rid of values().stream(), or do I have to write a custom Spliterator? In other words have only one collect in my code.

What does the `ActionsBloc::mergeWith` look like? What is `ActionsBloc`? — Naman, Jun 20 '19 at 11:03
@Naman i've updated my post. I wont put the code of `Action` because it's superfluous for the question. — bubbles, Jun 20 '19 at 11:27
as a side note, starting from Java 9 you can replace `.filter(Optional::isPresent).map(Optional::get)` with `.flatMap(Optional::stream)` — Adrian, Jun 20 '19 at 11:53
@ElmaCherb I think it was answered once [here](https://stackoverflow.com/questions/39013437/grouping-java8-stream-without-collecting-it) - no, it's not possible to group without collecting — Adrian, Jun 20 '19 at 12:07
@Adrian related to that post, i have to write my own spliterator or use a third-party lib — bubbles, Jun 20 '19 at 12:42
Just `.collect(toMap(ActionsBloc::getClass, Function.identity(), ActionsBloc::mergeWith))`… — Holger, Jun 20 '19 at 14:54

Naman · Accepted Answer · 2019-06-20T15:47:19.017

4

You can work with a reducing identity to sort that out possibly. One way could be to update the implementation of mergeWith as :

default ActionsBloc mergeWith(ActionsBloc ab) {
    this.actions().addAll(Optional.ofNullable(ab)
            .map(ActionsBloc::actions)
            .orElse(Collections.emptyList()));
    return this;
}

and then modify the grouping and reduction to:

this.blocs = new ArrayList<>(Stream.of(this.blocs, other.blocs)
        .flatMap(Collection::stream)
        .collect(groupingBy(ActionsBloc::getClass, reducing(null, ActionsBloc::mergeWith)))
        .values());

Edit: As Holger pointed out such use cases of using groupingBy and reducing further could be more appropriately implemented using toMap as :

this.blocs = new ArrayList<>(Stream.concat(this.blocs.stream(), other.blocs.stream())
        .collect(Collectors.toMap(ActionsBloc::getClass, Function.identity(), ActionsBloc::mergeWith))
        .values());

edited Jun 20 '19 at 15:47

answered Jun 20 '19 at 11:45

Naman

27,789
26
218
353

your solution is good but i’ll have the same problem when filtering on empty lists. This way i will have 2 calls to collect too. – bubbles Jun 20 '19 at 11:55
@ElmaCherb Seems like you're looking for [`collectingAndThen`](https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/stream/Collectors.html#collectingAndThen(java.util.stream.Collector,java.util.function.Function)). Is that so, maybe I've understood the problem incorrectly? – Naman Jun 20 '19 at 11:59
I have to try it, i don’t know if i can get rid of the second stream inside collectAndThen – bubbles Jun 20 '19 at 12:33
5

Whenever you find yourself using the `reducing` collector with `groupingBy`, you should check whether `toMap` isn’t more appropriate: `.collect(toMap(ActionsBloc::getClass, Function.identity(), ActionsBloc::mergeWith))` or `.collect(collectingAndThen(toMap(ActionsBloc::getClass, Function.identity(), ActionsBloc::mergeWith), m -> new ArrayList<>(m.values())))`. Further, this looks like a job for `Stream.concat(this.blocs.stream(), other.blocs.stream())`… – Holger Jun 20 '19 at 15:00
@Holger it works like a charm with `collectingAndThen`, you can put your solution and i'll choose it as the correct one. Can you add some references about the use of `reducing` and `groupingBy` together ? it seems like a pattern – bubbles Jun 20 '19 at 15:23
@Holger Thanks for the note. Would try and practice the pattern going forward. `Stream.concat` makes sense as well. – Naman Jun 20 '19 at 15:46

Do I need a custom Spliterator to avoid extra .stream() call?

1 Answers1

Linked

Related