staltz / rxmarbles

Interactive diagrams of Rx Observables
http://www.rxmarbles.com
BSD 3-Clause "New" or "Revised" License
4.21k stars 537 forks source link

concat marble diagram misleading #22

Open DavidMGross opened 9 years ago

DavidMGross commented 9 years ago

The diagram for concat shows the second Observable emitting items concurrently with the first one, and yet those items end up being concatenated. In reality, either those items would be lost or they wouldn't be emitted until the first Observable completes.

See: https://github.com/ReactiveX/RxJava/issues/3090#issuecomment-122973184

staltz commented 9 years ago

second Observable emitting items concurrently with the first one

Unless the observables are cold.

main marble diagram of Concat Operator is not fully correct and creates a bit of confusion (at least for my coworker, I was watching diagrams from RxJava javadocs), but probably it's limitation of Js library for marble diagrams.

The confusion arises if you are thinking of hot observables only. By default, RxMarbles displays cold observables. Rx is also cold by default.

That said, I understand that the RxJava diagram is more intuitive. To match that, I would need to build a custom renderer for concat (and maybe a couple of other related ones). It's doable, but I want to get other things solved first, such as a rendering for flatMap and other operators returning Observables-of-Observables.

For now, how about pointing out that these diagrams illustrate cold observables?

DavidMGross commented 9 years ago

There's an ambiguity, I think, about how time is represented in marble diagrams. If time proceeds from left to right and is the same for all observers (cue spooky floating E=mc² from the Twilight Zone intro), then the second (cold) Observable shouldn't begin emitting items until it is subscribed too, which won't happen until the first Observable completes, which means its emissions should appear to the right of the first Observable's onComplete notification.

If instead you interpret the left-to-right time as being relative to each Observable but not the same between Observables, there's no problem with the diagram as shown. But that approach seems to lead to misunderstandings.

I'd expect if I look over the diagrams I made for RxJava and for the reactivex.io site, I'd find that I was inconsistent in which of the above approaches I used. My original concat() diagram was like your rxmarbles version, but I later changed it after I got complaints about this very issue.

On Mon, Jul 20, 2015 at 12:24 PM, André Staltz notifications@github.com wrote:

second Observable emitting items concurrently with the first one

Not if the observables are cold.

main marble diagram of Concat Operator is not fully correct and creates a bit of confusion (at least for my coworker, I was watching diagrams from RxJava javadocs), but probably it's limitation of Js library for marble diagrams.

The confusion arises if you are thinking of hot observables only. By default, RxMarbles displays cold observables. Rx is also cold by default.

That said, I understand that the RxJava diagram is more intuitive. To match that, I would need to build a custom renderer for concat (and maybe a couple of other related ones). It's doable, but I want to get other things solved first, such as a rendering for flatMap and other operators returning Observables-of-Observables.

For now, how about pointing out that these diagrams illustrate cold observables?

— Reply to this email directly or view it on GitHub https://github.com/staltz/rxmarbles/issues/22#issuecomment-122997177.

David M. Gross PLP Consulting

pforhan commented 9 years ago

+1 to indicating cold/hot Observables. A toggle on subsequent Observables to switch them between cold and hot would be neat.