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

Subscription Utilities and Default Implementations #173

Closed
benjchristensen opened this issue Mar 7, 2013 · 4 comments
Closed

Subscription Utilities and Default Implementations #173

benjchristensen opened this issue Mar 7, 2013 · 4 comments
Assignees
Milestone

Comments

@benjchristensen
Copy link
Member

Provide a createSubscription() utility that returns a Subscription implementation that is thread-safe that provides an isUnsubscribed() getter to be used by most Observables via Observable.create() since most of the time they just need a thread-safe volatile/atomic boolean to check in a loop.

Also, where should it go along with these:

    public static Subscription createSubscription(final Action0 unsubscribe)
    public static Subscription createSubscription(final Object unsubscribe)
    public static Subscription noOpSubscription()

Should this be on Observable or should a new Subscriptions utility class exist?

@ghost ghost assigned benjchristensen Mar 7, 2013
@benjchristensen
Copy link
Member Author

The type of things to model in Rx are here: http://msdn.microsoft.com/en-us/library/system.reactive.disposables(v=vs.103).aspx

Disposable == Subscription in RxJava.

The 'Disposable' name means something special in C# similar to AutoCloseable (http://docs.oracle.com/javase/7/docs/api/java/lang/AutoCloseable.html) in Java7 (which is why Subscription doesn't implement AutoCloseable yet since we are sticking to Java6).

We want an rx.subscriptions package.

Subscription is currently an interface (which I like in this case since it can be a FunctionalInterface in Java8 and implemented with lambdas) but it means we can't exactly model the Subscription.create() method signature of Rx.Net.

I'm torn ... as Subscription.create() would be a very nice method signature.

We could have `rx.subscriptions.Subscriptions' as the utility method but it's slightly contrived.

For example:

    rx.subscriptions.Subscriptions.create(final Action0 unsubscribe)
    rx.subscriptions.Subscriptions.create(final Object unsubscribe)
    rx.subscriptions.Subscriptions.empty()

Then we'd have a class like `rx.subscriptions.BooleanSubscription' which is what would provide the functionality most Observables want.

@benjchristensen
Copy link
Member Author

Anyone have thoughts on what decisions to make with this API?

This was never tackled internally at Netflix as we just had custom Subscription objects that implement the interface - but that's not appropriate for a standalone library, particularly when trying to comply with the Rx.Net design and API.

@mairbek
Copy link
Contributor

mairbek commented Mar 7, 2013

I like the idea of creating Subscriptions class and moving it alongside with a Subscription interface to a separate package.

It seems like .NET naming convention dictates that interfaces should start with I and static class should be named as a singular word (IDisposable -> Disposable). Since we are dealing with Java, having Subscriptions as a utility class seems to be a natural choice because this naming style is widely used within a JDK itself, e.g. Collection -> Collections.

Besides that I see there are several wrappers in a System.Reactive.Disposables, some of them like CompositeDisposable seems to be particularly useful. It would be nice to see whole namespace migrated to RxJava.

@benjchristensen
Copy link
Member Author

Done ... interested in people's review however. Will be part of 0.6.0 as it involves some small breaking changes in the pursuit of trying to match Rx.Net (such as this https://rx.codeplex.com/SourceControl/changeset/view/6bf8a7952bdd#Rx/NET/Source/System.Reactive.Core/Reactive/Disposables/Disposable.cs)

rickbw pushed a commit to rickbw/RxJava that referenced this issue Jan 9, 2014
jihoonson pushed a commit to jihoonson/RxJava that referenced this issue Mar 6, 2020
* Fixing blinking test in AsyncRetryEventPublisherTest

* Removing race condition from 'RateLimiterChainSpec.test events' test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants