ReactiveX / RxJava

RxJava – Reactive Extensions for the JVM – a library for composing asynchronous and event-based programs using observable sequences for the Java VM.
Apache License 2.0
47.88k stars 7.61k forks source link

Observable.generate(Callable<S>, BiConsumer<S,Emitter<T>>) is broken #6264

Closed martinandersson closed 5 years ago

martinandersson commented 5 years ago

The method specified in the question title does not work as advertised (JavaDoc).

The method is supposed to be "stateful". The first argument is supposed to be the "initial state" and the argument passed to the BiConsumer is supposed to be the "current state".

This example prints three 1's consecutively:

Observable<Integer> numbers = Observable.generate(
        () -> 0,
        (prev, emit) -> {emit.onNext(++prev);});

numbers.take(3).subscribe(System.out::println);

I would have expect that for this guy to be stateful, whatever value I throw into the emitter is remembered and the output of the example would be "1 2 3".

However, If I dig into the source code (RxJava v2.2.3) I find that my BiConsumer is wrapped by a SimpleBiGenerator. In ObservableInternalHelper.java, line 59, I can see that my BiConsumer is called but whatever value sent to the emitter is not stored. The previous, i.e the initial state, is returned and one must deduce that this unchanged value is the one passed back to the consumer during the next iteration.

If I add a return statement in my lambda and thus use the overload that accepts a BiFunction instead of a BiConsumer, then it works and I see "1 2 3":

Observable<Integer> numbers = Observable.generate(
        () -> 0,
        (prev, emit) -> {
            int n = ++prev;
            emit.onNext(n);
            return n;
        });

numbers.take(3).subscribe(System.out::println);
akarnokd commented 5 years ago

The operator works as intended.

There are two overloads for two different purposes. The BiConsumer version can be used if you have mutable state object whereas the BiFunction can be used for working with immutable state object.

In your first example, you have an immutable Integer thus incrementing it has no effect beyond the lambda call. In the second example of yours, you create a fresh value and give it to the generate operator which will remember it for you and provide it for the next invocation of the lambda.

martinandersson commented 5 years ago

lol you shouldn't have closed this issue! The JavaDoc is clearly extremely misleading. What you just said is what should go into the docs. But the current docs says that the Observable the method returns is "stateful" - et cetera.

Besides, look at the picture/illustration in the JavaDoc. It makes a reference to "n++". I mean the docs for this method is extremely misleading and I would even say that only a great deal of creativity can make the argument they are not.

martinandersson commented 5 years ago

Plus, I would generally say that changing the SEMANTICS of an overload like that is a bad idea. However that might be, isn't a bit funny how they - like you described - work differently but have almost the exact same JavaDoc copy pasted?

I would suggest reopening the issue.

akarnokd commented 5 years ago

We don't need nor want to explain Java language fundamentals in our Javadocs. The diagrams on those methods are of an overview explanations of the operators than any literal source code example. In this regard, we didn't find enough difference between the overloads to make it any priority to draw custom diagrams for them.

This is a free and open-source library maintained by volunteers on their free time. You are more than welcome to contribute and draw better diagrams and use a more respectful language in the future.

martinandersson commented 5 years ago

I'll give you credit for the "respectful language" part. You totally beat me on that lol. I have a way of being pretty straight.

Anyways, my point is still valid. I THINK. I am by no means foreign to "Java language fundamentals" and I fail to see how this particular remark at all is connected with the JavaDocs in question. I am still learning RxJava, had I been more into the "ecosystem" I would have loved to contribute. The way I see it I contribute by bitching and nagging on Github lol.

You in your comment in this thread explained a very important detail as to why I never saw the state change. This is key and I argue must go into the JavaDoc. They are misleading for all the above stated reasons.

akarnokd commented 5 years ago

Your issue is related to the Java language fundamental that overwriting a method argument has no effect of its caller:

class Demo {
    int num;

    void accept(Integer n) {
      n++;
    }
    public static void main(String args) {
        num = 1;
        accept(num);
        System.out.println(num);
    }
}

What does this program print? Do you see the analogue to your situation?

The way I see it I contribute by bitching and nagging on Github lol.

This is not contributing, this is wasting the time of the maintainers and other contributors watching the project.