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

Retry in Scala adaptor is ambiguous #1044

Closed
axel22 opened this issue Apr 17, 2014 · 6 comments
Closed

Retry in Scala adaptor is ambiguous #1044

axel22 opened this issue Apr 17, 2014 · 6 comments

Comments

@axel22
Copy link

axel22 commented Apr 17, 2014

The retry overload leads to ambiguities:

object Test extends App {
  import rx.scala.lang._
  (null: Observable[String]).retry(3)
}

both method retry in trait Observable of type => rx.lang.scala.Observable[String]
and  method retry in trait Observable of type (retryCount: Int)rx.lang.scala.Observable[String]
match argument types (Int)
   (null: Observable[String]).retry(3)
zsxwing added a commit to zsxwing/RxJava that referenced this issue Apr 20, 2014
@zsxwing
Copy link
Member

zsxwing commented Apr 20, 2014

The problem is that there are two interpretations for o.retry(3): o.retry(retryCount: Int) or o.retry.apply(3).

@zsxwing
Copy link
Member

zsxwing commented Apr 20, 2014

Maybe we should remove apply method from trait Observable, considering it's likely to cause such ambiguities without careful attention.

/cc @samuelgruetter

zsxwing added a commit to zsxwing/RxJava that referenced this issue Apr 21, 2014
@samuelgruetter
Copy link
Contributor

Yes, I'd vote for removing def apply(index: Int): Observable[T], since it's probably not used that often, and elementAt is a better, more explicit name.

@benjchristensen
Copy link
Member

Thanks @samuelgruetter for weighing in. Based on the two of you agreeing, I'll merge that change.

benjchristensen added a commit that referenced this issue Apr 21, 2014
@headinthebox
Copy link
Contributor

Agree with @samuelgruetter .

@benjchristensen
Copy link
Member

Closed as I believe the fix was merged.

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

5 participants