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

Bump to RxJava 1.1.5 #194

Merged
merged 3 commits into from
Jun 12, 2016
Merged

Bump to RxJava 1.1.5 #194

merged 3 commits into from
Jun 12, 2016

Conversation

zsxwing
Copy link
Member

@zsxwing zsxwing commented Jun 6, 2016

  • Bump to RxJava 1.1.5
  • Add missing operators

- Bump to RxJava 1.1.5
- Add missing operators
@samuelgruetter
Copy link
Collaborator

I see that more and more xxxDelayError methods are added, and the number of methods in the API becomes bigger and bigger. Currently there are 8 of them:

  def concatDelayError[U](implicit evidence: Observable[T] <:< Observable[Observable[U]]): Observable[U]
  def concatMapDelayError[R](f: T => Observable[R]): Observable[R]
  def switchMapDelayError[R](f: T => Observable[R]): Observable[R]
  def switchDelayError[U](implicit evidence: Observable[T] <:< Observable[Observable[U]]): Observable[U]
  def mergeDelayError[U >: T](that: Observable[U]): Observable[U]
  def flattenDelayError[U](implicit evidence: Observable[T] <:< Observable[Observable[U]]): Observable[U]
  def flattenDelayError[U](maxConcurrent: Int)(implicit evidence: Observable[T] <:< Observable[Observable[U]]): Observable[U]
  def combineLatestDelayError[T, R](sources: Iterable[Observable[T]])(combineFunction: Seq[T] => R): Observable[R]

Will there be even more later?
Do you think we could treat all of these in a more uniform way? Maybe by adding an ErrorDelayingObservable containing all these methods, and a toErrorDelaying method, in the same way as we have a BlockingObservable? Or maybe give these methods a boolean argument with default value: delayError: Boolean = false? Or something else? Or is this too much complication for what we gain? Also, do you know if this has been discussed in RxJava, and what was decided for RxJava?

@zsxwing
Copy link
Member Author

zsxwing commented Jun 6, 2016

Maybe by adding an ErrorDelayingObservable containing all these methods, and a toErrorDelaying method, in the same way as we have a BlockingObservable

It will become too verbose for RxJava considering Java doesn't have the extension methods.

Or maybe give these methods a boolean argument with default value: delayError: Boolean = false?

Java doesn't have the default value.

However, both them are supported in Scala.

For the default values, I remember when we use overload, default value, currying and implicit together, the compiler will be crazy and not be able to resolve the correct method. So I prefer the extension method idea.

@samuelgruetter
Copy link
Collaborator

I'm a bit confused, why you are mentioning extension methods? Do you mean extension methods as defined here? The approach used in BlockingObservable does not require extension methods at all.

@zsxwing
Copy link
Member Author

zsxwing commented Jun 8, 2016

Yep. toErrorDelaying is too verbose so I suggest to use the extension methods.

@samuelgruetter
Copy link
Collaborator

Ok, but what if we call it .delayError instead of toErrorDelaying? Then, instead of myObservable.concatMapDelayError(myFunc), we'd write myObservable.delayError.concatMap(myFunc), which has exactly the same amount of letters. What do you think about this?

@zsxwing
Copy link
Member Author

zsxwing commented Jun 8, 2016

Sounds great. I will update the PR.

* [[Observable]]s by means of the given aggregation function
* @see <a href="http://reactivex.io/documentation/operators/combinelatest.html">ReactiveX operators documentation: CombineLatest</a>
*/
def combineLatestDelayError[T, R](sources: Iterable[Observable[T]])(combineFunction: Seq[T] => R): Observable[R] = {
Copy link
Member Author

Choose a reason for hiding this comment

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

combineLatestDelayError is still in Observable object.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I guess Observable.combineLatestDelayError is better than ErrorDelayingObservable.combineLatest (unless there will be many more static xxxDelayError methods, maybe)

@zsxwing
Copy link
Member Author

zsxwing commented Jun 11, 2016

@samuelgruetter updated.

"concatDelayError(Observable[_ <: Observable[_ <: T]])" -> "concatDelayError(<:<[Observable[T], Observable[Observable[U]]])",
"concatDelayError(Iterable[_ <: Observable[_ <: T]])" -> "[use `iter.toObservable.concatDelayError`]",
"concatDelayError(Observable[_ <: Observable[_ <: T]])" -> "[use `delayError.concatDelay(<:<[Observable[T], Observable[Observable[U]]])`]",
"concatDelayError(Iterable[_ <: Observable[_ <: T]])" -> "[use `iter.toObservable.delayError.concatDelay`]",
Copy link
Collaborator

Choose a reason for hiding this comment

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

should be concat instead of concatDelay, I guess?

Copy link
Member Author

Choose a reason for hiding this comment

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

should be concat instead of concatDelay, I guess?

Fixed

@samuelgruetter
Copy link
Collaborator

LGTM

@zsxwing zsxwing merged commit 4d96d57 into ReactiveX:0.x Jun 12, 2016
@zsxwing zsxwing deleted the rxjava-1.1.5 branch June 12, 2016 23:09
@zsxwing zsxwing added this to the 0.26.2 milestone Jun 17, 2016
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

Successfully merging this pull request may close these issues.

2 participants