-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
Implemented "First" and "FirstOrDefault" operations #357
Conversation
RxJava-pull-requests #248 SUCCESS |
* source Observable completes without emitting a single item. | ||
* @see <a href="http://msdn.microsoft.com/en-us/library/hh229177%28v=vs.103%29.aspx">MSDN: Observable.First</a> | ||
*/ | ||
public Observable<T> first() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The naming convention here is different than last
and takeLast
so wondering if we should change to match.
The last
operator is blocking so on BlockingObservable
. This first
operator seems to be similar to single
and blocking last
.
Thus I wonder if we should have takeFirst
instead of first
? This is different than Rx.Net but would clean up the same difference in convention that it has.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd expect the names to be first
and last
whether blocking or not. Also, should BlockingObservable
really extend Observable
? Seems like a potential source of confusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't extend from Observable any longer.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool. I'm behind the times :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So takeFirst
/takeLast
/ or first
/last
... or both aliased?
Until we split off BlockingObservable
last
and takeLast
meant different things, we could now have last
exist on both but with different return types.
In Scala, I'm okay with both naming schemes, with a slight preference for leaving the I was also playing around with |
I need to look at what Java 8 is calling these things, as that is the more important naming convention for the core lib to match. |
Java 8 uses |
…into operator-first-merge Conflicts: rxjava-core/src/test/java/rx/ObservableTests.java This merges pull request ReactiveX#357 Also aliased first with takeFirst to match takeLast.
…into operator-first-merge Conflicts: rxjava-core/src/test/java/rx/ObservableTests.java This merges pull request ReactiveX#357 Also aliased first with takeFirst to match takeLast.
…into operator-first-merge Conflicts: rxjava-core/src/test/java/rx/ObservableTests.java This merges pull request ReactiveX/RxJava#357 Also aliased first with takeFirst to match takeLast.
…into operator-first-merge Conflicts: rxjava-core/src/test/java/rx/ObservableTests.java This merges pull request ReactiveX/RxJava#357 Also aliased first with takeFirst to match takeLast.
…bscriber is is not permitted to subscribe, the subscription must be canceled and the ResilienceBaseSubscriber must call onSubscribe on the target Subscriber with an EmptySubscription followed by a call to onError with the supplied error. (ReactiveX#452) Issue ReactiveX#357: Fixed a bug in ResilienceBaseSubscriber. If a subscriber is is not permitted to subscribe, the subscription must be canceled and the ResilienceBaseSubscriber must call onSubscribe on the target Subscriber with an EmptySubscription followed by a call to onError with the supplied error. The bug was that the ResilienceBaseSubscriber cancelled it's own subscription as well.
This PR builds upon the
skipWhile
PR (#355) which makes implementing the twofirst
variants extremely easy.I changed
firstOrDefault
slightly from what Rx.NET does by explicitly requiring a default value as parameter of the function call. I don't know what default value I should return otherwise, exceptnull
(and imho this would be pretty useless).This PR addresses issue #44.