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

subscriber methods with scheduler create ambiguous overloads for Groovy and Java 8 #1116

Closed
ldaley opened this issue Apr 28, 2014 · 9 comments
Milestone

Comments

@ldaley
Copy link
Contributor

ldaley commented Apr 28, 2014

Hi,

In 0.18 there are the following methods:

public final Subscription subscribe(final Action1<? super T> onNext, final Action1<Throwable> onError)
public final Subscription subscribe(final Action1<? super T> onNext, Scheduler scheduler)

The “problem” here is that Action1 and Scheduler are both SAM types. This makes closure coercions and lambda expressions ambiguous and requires extra syntax to specify the coercion target type.

Could the Scheduler accepting methods be renamed?

There are other similar methods of course.

@benjchristensen
Copy link
Member

@headinthebox and @akarnokd What is the reason for having the Scheduler overloads on the subscribe methods? I honestly can not remember why that happened. Seems like that should just be using subscribeOn and does not need these overloads.

This is not something we want to leave as is because it breaks Groovy and Clojure.

@ldaley
Copy link
Contributor Author

ldaley commented May 16, 2014

It also makes it slightly less convenient to use a Java 8 lambda expressions because you need to add the target type to the expression.

@headinthebox
Copy link
Contributor

Never understood why, this it not in .net ... I would junk that overload. Not even sure what it is supposed to do.

@akarnokd
Copy link
Member

Looks like a convenience shortcut and we should avoid ambiguity problems with Java 8 while RxJava is in a "allowed" API flux. I vote for removal.

@benjchristensen
Copy link
Member

So do I, let's remove them.

@ldaley
Copy link
Contributor Author

ldaley commented May 17, 2014

Would it be worth some tests that reflectively check the public API for any methods that are overloaded with different SAM types (i.e. this problem) to catch this before leaking out again?

@benjchristensen
Copy link
Member

Yes, a test like that would be good. You interesting in doing it?

@benjchristensen benjchristensen added this to the 0.19 milestone May 20, 2014
@ldaley
Copy link
Contributor Author

ldaley commented May 20, 2014

Sure, I'll do it.

I'll do something for the methods on Observable, it could then later be adapted to other applicable types.

@benjchristensen
Copy link
Member

Thanks, and I'll delete the methods shortly.

benjchristensen added a commit to benjchristensen/RxJava that referenced this issue May 20, 2014
- Fixes ReactiveX#1116
- These should never have been added, the subscribeOn operator already provides this functionality
zsxwing pushed a commit to zsxwing/RxScala that referenced this issue Aug 19, 2014
- Fixes ReactiveX/RxJava#1116
- These should never have been added, the subscribeOn operator already provides this functionality
benjchristensen added a commit to ReactiveX/RxScala that referenced this issue Aug 19, 2014
- Fixes ReactiveX/RxJava#1116
- These should never have been added, the subscribeOn operator already provides this functionality
benjchristensen added a commit to ReactiveX/RxScala that referenced this issue Aug 19, 2014
- Fixes ReactiveX/RxJava#1116
- These should never have been added, the subscribeOn operator already provides this functionality
benjchristensen added a commit to ReactiveX/RxScala that referenced this issue Aug 19, 2014
- Fixes ReactiveX/RxJava#1116
- These should never have been added, the subscribeOn operator already provides this functionality
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

4 participants