Creating Observable#create overloads for SyncOnSubscribe and AsyncOnSubscribe

[stealthcode]

This is to facilitate the discovery of methods for creating observables that respect back pressure semantics. Currently the Observable#create(OnSubscribe) static method is the easiest method to discover for creating an observable which does not provide clear facilities for managing back pressure.

[benjchristensen]

I like this addition. It makes it far more discoverable to do 'create' in a safer way.

+1

[abersnaze]

I'm worried that having the same name will cause method resolution problems with other not so strict JVM languages.

[davidmoten]

Looks good. @abersnaze what's an example?

[akarnokd]

I guess you couldn't call create(OnSubscribe) because of overload problems.

[stealthcode]

You can still call to the OnSubscribe overload and it would work just fine. The purpose behind the 2 new overloads is discoverability.

[akarnokd]

Sure these two methods point to the classes, but I'd prefer those createStateful and such methods added to Observable instead.

[stealthcode]

@akarnokd so you would prefer to have the static create methods explicitly listed out? This is how that would look when in an IDE (i.e. in Eclipse with Observable.create then hitting CTRL+Space).

One static method per concrete option:

create(OnSubscribe<T> f)
createAsyncSingleState(Func0<? extends S> generator, Action3<? super S, Long, ? super Observer<Observable<? extends T>>> next)
createAsyncSingleState(Func0<? extends S> generator, Action3<? super S, Long, ? super Observer<Observable<? extends T>>> next, final Action1<? super S> onUnsubscribe)
createAsyncStateful(Func0<? extends S> generator, Func3<? super S, Long, ? super Observer<Observable<? extends T>>, ? extends S> next, Action1<? super S> onUnsubscribe)
createAsyncStateless(Action2<Long, ? super Observer<Observable<? extends T>>> next)
createAsyncStateless(Action2<Long, ? super Observer<Observable<? extends T>>> next, Action0 onUnsubscribe)
createSyncSingleState(Func0<? extends S> generator, Action2<? super S, ? super Observer<? super T>> next)
createSyncSingleState(Func0<? extends S> generator, Action2<? super S, ? super Observer<? super T>> next, Action1<? super S> onUnsubscribe)
createSyncStateful(Func0<? extends S> generator, Func2<? super S, ? super Observer<? super T>, ? extends S> next, Action1<? super S> onUnsubscribe)
createSyncStateful(Func0<? extends S> generator, Func2<? super S, ? super Observer<? super T>, ? extends S> next)
createSyncStateless(Action1<? super Observer<? super T>> next)
createSyncStateless(Action1<? super Observer<? super T>> next, Action0 onUnsubscribe)

One overload per highlevel category (Sync, Async)

create(OnSubscribe<T> f)
create(SyncOnSubscribe<S, T> syncOnSubscribe)
create(AsyncOnSubscribe<S, T> asyncOnSubscribe)

This isn't exactly a direct apples-to-apples representation as you'd have to use the SyncOnSubscribe static methods. e.g.

create(SyncOnSubscribe.createStateless(Action1<? super Observer<? super T>> next))

I think I prefer the latter because the surface area of the Observable is constrained to two (three-ish) high level categories of creation options.

[stealthcode]

I've added more explanation to the javadoc.

[stevegury]

I prefer current version (with overload per high-level category).
👍

[stealthcode]

The test seems to be a flaky unit test.

rx.ObservableTests > testErrorThrownIssue1685 FAILED
    java.lang.AssertionError: UncaughtExceptionHandler didn't get anything.
        at org.junit.Assert.fail(Assert.java:93)
        at org.junit.Assert.assertTrue(Assert.java:43)
        at org.junit.Assert.assertNotNull(Assert.java:526)
        at rx.ObservableTests.testErrorThrownIssue1685(ObservableTests.java:1113)

I have added @Experimental to these overloads (to mirror the status of the underlying Sync/Async O.S.). Is this otherwise an acceptable API addition or are there concerns? @akarnokd @abersnaze

[akarnokd]

Travis sometimes seems to slow down drastically; that 1 second wait in that test is like 1000x more than usually needed to verify the behavior. Just rerun the test in this case.

We can start with your two methods and add the rest later.

[stealthcode]

I would prefer not to have both styles personally as redundancy makes for a cluttered and confusing api. Is it a personal preference for listing all variants?

[akarnokd]

Only the sync ones, the async ones can be hidden behind that create overload.

[stealthcode]

I'm fine discussing that when the time comes.

[abersnaze]

@davidmoten I can't remember the exact instances of the issue but I'm not as familiar with scala, clojure, groovy, or kotlin. I did a simple test in groovy, that worked, just because I had a REPL handy. it would be nice have a lowest common denominator for method signatures so we don't make it unnecessarily difficult in any of those languages.

[JakeWharton]

Kotlin handles the overloads fine 👍

On Thu, Mar 3, 2016 at 7:26 PM George Campbell [email protected]
wrote:

@davidmoten https://github.com/davidmoten I can't remember the exact
instances of the issue but I'm not as familiar with scala, clojure, groovy,
or kotlin. I did a simple test in groovy, that worked, just because I had a
REPL handy. it would be nice have a lowest common denominator for method
signatures so we don't make it unnecessarily difficult in any of those
languages.

—
Reply to this email directly or view it on GitHub
#3738 (comment).