Closed LalitMaganti closed 7 years ago
I'd like to address the disadvantages:
Not advocating or condemning this just yet, but just want to share some thoughts.
Many operations don't make sense outside the 0..N Observable
This is the single biggest problem I found. Thankfully I used Kotlin and could do @Deprecated(level=HIDDEN)
which prevents them from being used (e.g., map
on Completable
). In Java this shows up just as a deprecated method when you use it on Completable
which I didn't care about, but obviously would be the only approach here.
yes, you can reuse operator internals from Observable but lose the optimization potential knowing that only one of the onNext, onError or onComplete is ever called
In my prototype I still retained individual observers. If you subscribe to a Single
with an Observer
though you get wrapped so that onSuccess
becomes onNext
+onComplete
.
won't work with the current architecture and would still require duplicate reactive base-class shells around: ObservableMap, SingleMap.
Non-goal. Nailing the consumer API is my only concern. This also further makes the above point not true since you can still optimize operators against singles, completables, etc.
non-final methods may lead to megamorphic dispatch
I'd be surprised if this was an actual problem in practice. Is the creation of the chain a hot path or is the execution of events down the chain the hot path? I would wager concerns of the latter outweigh the former 100x over.
no more onSuccess, has to call onNext followed by onComplete for Single and Maybe.
Same as above. You still get all the interface benefits except there's actually a lower conversion overhead.
When I first concieved Completable
I implemented it in my own repo and once I was confident it is viable and working on its own, I posted a PR against RxJava.
Whatever unification you have in mind, try implementing a minimum viable "product" (in Java) and see how far the rabbit hole goes.
Thankfully I used Kotlin
Kotlin has extension methods so you could build a native RS library just by targeting interfaces. I did a similar thing in Reactor-Core.NET where many operators implement IFlux
and IMono
(both extending IPublisher
) at the same time with the same runtime classes. From there IFlux<T> map(this IFlux<T>, Func<T, R>)
and IMono<T> map(this IMono<T>, Func<T, R>)
were just capturing the right type and returning the right type without conversion overhead.
I'd be happy to implement an MVP and see what falls out.
Extension methods also get you invokestatic!
I'm closing this for now. I can imagine this done in Kotlin where there are extension methods and suppression and thus less painful than in Java.
Here's my version from a few months ago which is incomplete: https://github.com/JakeWharton/Reagent/
I didn't use extension methods but I'll have to try.
On Sat, Nov 12, 2016, 2:23 PM David Karnok notifications@github.com wrote:
Closed #4584 https://github.com/ReactiveX/RxJava/issues/4584.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ReactiveX/RxJava/issues/4584#event-856827067, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEEEZYsjGXaHKAavbCCMfH-ixReRMJqks5q9hIhgaJpZM4KET4U .
(or alternatively make all four inherit from a common base class)
At this point in RxJava 2.0 we have four NBP classes all of which have similar semantics but are currently totally decoupled from each other. Even though @akarnokd has been doing great work keeping all the code synced across the four operators this setup is a minefield for bugs and is extremely hard to follow the changes from an external watcher perspective (I don't know how you manage to review so many changes @JakeWharton!).
My proposal is to make the class have a some base class such that effort is not duplicated across different types.
@JakeWharton from another issue:
Obviously such a drastic change has both advantages and disadvantages.
Advantages:
Disadvantages (from @akarnokd's post):