The use of .peek() in java 8 streams

Question

I am using .peek() in my stream but it is frowned upon, sadly I can't find a solution.

Simplified version:

static boolean fooAddTester(int size) {
    Foo foo = new foo(); //data structure

    return IntStream.range(0, size).
            .peek(i -> synchronized(r){foo.add(i)})
            .allMatch(e -> foo.isLegal());
}

what I need to do is to iterate over the IntStream and to check after every insert if the foo data structure is legal. This is logically equivalent to:

static boolean fooAddTester(int size) {
    Foo foo = new foo(); //data structure

    for(int i=0; i<size; i++){
        foo.add(i);
        if(!foo.isLegal())
            return false;
    return true;
}

However it is more complex and I'm trying to use streams to simplify and learn.

A way to do the same without using .peek() is this: which does work - but I'm just "moving" the problem to .allMatch():

return IntStream.range(0, size).
            .allMatch(i -> {
                 synchronized(r){foo.add(i)};
                 foo.isLegal();
             )};

My problem is very similar to this question, the diffrence is that I am checking every time so the solutions aren't working.

So my questions are:

Is .peek() really only to debugging or can I use it in this manner?
Is there any better solution?
Should I use my second solution?

I'm looking for a correct solution, not a working solution, all of those codes are already working.

This might help : https://softwareengineering.stackexchange.com/questions/308977/is-it-an-antipattern-to-use-peek-to-modify-a-stream-element — Emil Hotkowski, Jun 26 '17 at 12:03
you want to check a condition `after every insert`; that's relying on side-effects and it should be avoided. — Eugene, Jun 26 '17 at 12:07
the `synchronized` will not help you much btw - it will guarantee that only one element at a time is put into `foo`; but it will not guarantee *which* element is put into foo, since there is no processing order in case of a parallel stream — Eugene, Jun 26 '17 at 12:13
What is `r`? Why are you synchronizing on it? Your stream code doesn’t match the loop. You are streaming over `int` values, so you would need a `static` method `Foo.isLegal(int)` to make the first snippet accepted by the compiler. In the last snippet, you are using the method reference `Foo::isLegal` at a place where no method references are allowed. Obviously, you never tried any of these alternatives. — Holger, Jun 26 '17 at 12:55
@Holger yeah. I have just discovered the OP used the `Foo::isLegal` anywhere. sir, what I described is right in my answer? welcome to point out my mistakes. — holi-java, Jun 26 '17 at 13:09
What is the logic for `Foo::isLegal`? Looking at your `logically equivalent` example there might be a better logical solution for your problem. — SubOptimal, Jun 26 '17 at 13:37
@SubOptimal It checks the data sturcture (originaly a D-ary heap). — Adam Shem-Ur, Jun 26 '17 at 18:47
@Eugene but does garanteed that after `.peek()` `.allMatch()` will be executed? Beacuse I don't care the order of `add()`. — Adam Shem-Ur, Jun 26 '17 at 18:49
@Eugene so if side effects should by avoided should I just not uses streams? — Adam Shem-Ur, Jun 26 '17 at 18:52
@Holger I didn't because the code is a simplification of my real code, I did it for `parallel` but it isn't relevant, you are right and I didn't tried **this** code, I just didn't simplify correctly, I'll fix. I did however tried my original code (which has the exact same problem). — Adam Shem-Ur, Jun 26 '17 at 19:00
The problem is that you over-simplified. The `Foo` instance is purely local and `i` is a primitive value. So why are you synchronizing on an object referred by `r` which is out of scope and obviously unrelated? Or why do you think that `foo.add(i)` needs synchronization, but the subsequent `foo.isLegal()` could work without? It doesn’t make any sense in this simplified example, but suggests that your actual operation is much more complex and will have interference which can’t be discussed with this simplified example. — Holger, Jun 27 '17 at 10:58

holi-java · Accepted Answer · 2017-06-26T13:02:32.330

5

the documentation of Stream#peek has mentioned as below, and mainly is not absolutely:

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

@Holger has answered this question absolutely:

the most useful thing you can do with peek is to find out whether a stream element has been processed.

and some side-effects he has also pointed it out in his answer, the peek operation depends on which terminal operation was invoked. so when using peek internally you should be carefully.

so the correctly way is just using for-each loop, since Stream#collect doesn't support short-circuiting operation.

the optional way is using peek because you can control the stream by yourself. and you need to remove the synchornized block, it is unnecessary here.

return IntStream.range(0, size).peek(foo::add).allMatch(__ -> Foo.isLegal(foo));

edited Jun 26 '17 at 13:02

answered Jun 26 '17 at 12:38

holi-java

29,655
7
72
83

4

well yes `the most useful thing you can do with peek is to find out whether a stream element has been processed.`, but by logging, not relying on side-effects. As soon as you add `parallel` this will break in mysterious ways. synchronized will make sure that only one element at a time is processed, but even then you don't know which element, there is *processing order*. – Eugene Jun 26 '17 at 12:46
@Eugene but I'd like to say the OP can control how to create, consuming, operates a stream. so there is no problem here. – holi-java Jun 26 '17 at 12:48
I added `synchornized` originally to use `parallel()`. Thanks a lot for your answer! – Adam Shem-Ur Jun 26 '17 at 19:10

score 2 · Answer 2 · answered Jun 26 '17 at 12:19

I can think of only a single way to do it if you really want to use other stream operations before your logic, but I'm not really a big fan of it...

 boolean result = true;
    try {
        IntStream.range(0, 10)
                .forEachOrdered(x -> {
                    foo.add(x);
                    if (!Foo.isLegal(foo)) {
                        throw new RuntimeException("just because");
                    }
                });
    } catch (RuntimeException re) {
        result = false;
    }

    System.out.println(result);

Obviously you need to replace RuntimeException with some kind of your exception.

The use of .peek() in java 8 streams

2 Answers2