Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add lazy RefCount operator for IObservables. #133

Merged
merged 1 commit into from
Jun 12, 2018
Merged

Add lazy RefCount operator for IObservables. #133

merged 1 commit into from
Jun 12, 2018

Conversation

danielcweber
Copy link
Collaborator

@danielcweber danielcweber commented Sep 14, 2015

LazyRefCount connects like RefCount but may delay disconnection. This is useful whenever a lot of connect/disconnect cycles are expected within a short timespan but with a significant overhead in connecting/disconnecting.


This change is Reviewable

danielcweber added a commit to ExRam/ExRam.Extensions that referenced this pull request Sep 14, 2015
…pefully equivalent to my pull request to Rx.net (dotnet/reactive#133). Once this is merged, this implementation will be removed.
@RxDave
Copy link
Contributor

RxDave commented Jul 31, 2016

How about just adding a TimeSpan delayDisconnect overload to RefCount, rather than introducing a new operator? (Coming from a keep-the-public-surface-area-simple and cohesive point of view.)

@danielcweber
Copy link
Collaborator Author

danielcweber commented Aug 1, 2016

I'm ok with that. Having an overload instead of a new operator would reflect the sanity of the idea even more and show that this is indeed useful. Shall I change it now?

@shiftkey
Copy link
Contributor

shiftkey commented Aug 1, 2016

👍 to an overload on RefCount, especially if the delay is the only difference in behaviour here...

@RxDave
Copy link
Contributor

RxDave commented Aug 1, 2016

Would it also make sense to reuse the implementation? The classic operator would then just call the new overload passing TimeSpan.Zero. This would eliminate some code bloat.

@danielcweber danielcweber changed the title Add LazyRefCount operator for IObservables. Add lazy RefCount operator for IObservables. Aug 3, 2016
if (parent._connectableSubscription == null)
parent._connectableSubscription = parent._source.Connect();

parent._serial.Disposable = new SingleAssignmentDisposable();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd move this earlier so that the timeout task is cancelled as soon as possible.

Copy link
Collaborator

@akarnokd akarnokd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd call it still RefCount. It has a better discoverability and user's would look for this behavior under RefCount anyway.

Copy link
Contributor

@quinmars quinmars left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could generate the homoiconic IQbservable operators, instead of excluding it from the parity check.

… like RefCount but may delay disconnection. This is useful whenever a lot of connect/disconnect cycles are expected within a short timespan but with a significant overhead in connecting/disconnecting. Some unit tests have been added. Lazy RefCount has been excluded from methods that must be present for Qbservable as well. I leave it up to others to decide what Lazy RefCount means for Qbservable and whether there should be an implementation.
@danielcweber danielcweber merged commit bdb5413 into dotnet:master Jun 12, 2018
@danielcweber danielcweber deleted the LazyRefCount branch June 12, 2018 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants