4.x: Deanonymize operators #549
Conversation
|
LGTM overall, can you centralize that OnNext-ignoring observer, definitely avoid copy-and-paste, maybe make it an extension for observers? |
|
Maybe as a separate PR? |
|
Sure. |
| if (Kind == NotificationKind.OnNext) | ||
| protected override IDisposable SubscribeCore(IObserver<T> observer) | ||
| { | ||
| return scheduler.Schedule(new State { parent = parent, observer = observer }, (scheduler, state) => |
There was a problem hiding this comment.
Please use a ValueTuple e.g. (parent, observer)
| @@ -57,22 +121,69 @@ public virtual IObservable<TResult> Create<TResult>(Func<IObserver<TResult>, Tas | |||
|
|
|||
| public virtual IObservable<TResult> Create<TResult>(Func<IObserver<TResult>, CancellationToken, Task<IDisposable>> subscribeAsync) | |||
| { | |||
| return new AnonymousObservable<TResult>(observer => | |||
| return new CreateWithTaskDisposable<TResult>(subscribeAsync); | |||
There was a problem hiding this comment.
Code reuse or inheritance or something similar?
There was a problem hiding this comment.
What do you mean? I've mentioned the benefits of these changes in the PR text.
There was a problem hiding this comment.
I mean those IObserver-implementors that are duplicated. There is a good chance for code reuse I guess.
There was a problem hiding this comment.
Which other implementation are you refering to?
There was a problem hiding this comment.
Func<IObserver<TResult>, CancellationToken, Task<Action>> subscribeAsync
Func<IObserver<TResult>, CancellationToken, Task<IDisposable>> subscribeAsync
Action != IDisposable
There was a problem hiding this comment.
A common base class might be possible, but nevermind, it's good as it is.
|
Just one more thing, have you considered using ContinueWith on the Tasks instead of converting them with ToObservable? You can pass it a state and possibly save some allocations. Might as well happen in a future PR. |
|
Maybe in a later PR, the aim in this PR to avoid those anonymous instances and intermediate delegates. |
This PR factors out the use of
AnonymousObservablein standard operators for two reasons: