Is it an antipattern to use peek() to modify a stream element?

Question

Suppose I have a stream of Things and I want to "enrich" them mid stream, I can use peek() to do this, eg:

streamOfThings.peek(this::thingMutator).forEach(this::someConsumer);

Assume that mutating the Things at this point in the code is correct behaviour - for example, the thingMutator method may set the "lastProcessed" field to the current time.

However, peek() in most contexts means "look, but don't touch".

Is using peek() to mutate stream elements an antipattern or ill-advised?

Edit:

The alternative, more conventional, approach would be to convert the consumer:

private void thingMutator(Thing thing) {
    thing.setLastProcessed(System.currentTimeMillis());
}

to a function that returns the parameter:

private Thing thingMutator(Thing thing) {
    thing.setLastProcessed(currentTimeMillis());
    return thing;
}

and use map() instead:

stream.map(this::thingMutator)...

But that introduces perfunctory code (the return) and I'm not convinced it's clearer, because you know peek() returns the same object, but with map() it's not even clear at a glance that it's the same class of object.

Further, with peek() you can have a lambda that mutates, but with map() you have to build a train wreck. Compare:

stream.peek(t -> t.setLastProcessed(currentTimeMillis())).forEach(...)
stream.map(t -> {t.setLastProcessed(currentTimeMillis()); return t;}).forEach(...)

I think the peek() version is clearer, and the lambda is clearly mutating, so there's no "mysterious" side effect. Similarly, if a method reference is used and the name of the method clearly implied mutation, that too is clear and obvious.

On a personal note, I don't shy away from using peek() to mutate - I find it very convenient.

Answer 1

You are correct, "peek" in the English sense of the word means "look, but do not touch."

However the JavaDoc states:

peek

Stream peek(Consumer action)

Returns a stream consisting of the elements of this stream, additionally performing the provided action on each element as elements are consumed from the resulting stream.

Key words: "performing ... action" and "consumed". The JavaDoc is very clear that we should expect peek to have the ability to modify the stream.

However the JavaDoc also states:

API Note:

This method exists mainly to support debugging, where you want to see the elements as they flow past a certain point in a pipeline

This indicates that it is intended more for observing, e.g. logging elements in the stream.

What I take from all of this is that we can perform actions using the elements in the stream, but should avoid mutating elements in the stream. For example, go ahead and call methods on the objects, but try to avoid mutating operations on them.

At the very least, I would add a brief comment to your code along these lines:

// Note: this peek() call will modify the Things in the stream.
streamOfThings.peek(this::thingMutator).forEach(this::someConsumer);

Opinions differ on the usefulness of such comments, but I would use such a comment in this case.

Answer 2

It could be easily misinterpreted, so I would avoid using it like this. Probably the best option is to use a lambda function to merge the two actions you need into the forEach call. You may also want to consider returning a new object rather than mutating the existing one - it may be slightly less efficient, but it is likely to result in more readable code, and reduces the potential of accidentally using the modified list for another operation that should have received the original.

Answer 3

The API note tells us that the method has been added mainly for actions such as debugging/logging/firing stats etc.

@apiNote This method exists mainly to support debugging, where you want * to see the elements as they flow past a certain point in a pipeline: *

       {@code
     *     Stream.of("one", "two", "three", "four")
     *         .filter(e -> e.length() > 3)
     *         .peek(e -> System.out.println("Filtered value: " + e))
     *         .map(String::toUpperCase)
     *         .peek(e -> System.out.println("Mapped value: " + e))
     *         .collect(Collectors.toList());
     * }