2.x: last() to return Single #4570
Conversation
Current coverage is 78.61% (diff: 77.50%)@@ 2.x #4570 diff @@
==========================================
Files 528 530 +2
Lines 35266 35337 +71
Methods 0 0
Messages 0 0
Branches 5474 5482 +8
==========================================
+ Hits 27745 27781 +36
- Misses 5541 5576 +35
Partials 1980 1980
|
| inOrder.verify(observer, times(1)).onNext(3); | ||
| inOrder.verify(observer, times(1)).onComplete(); | ||
| inOrder.verify(observer, times(1)).onSuccess(3); | ||
| // inOrder.verify(observer, times(1)).onComplete(); |
There was a problem hiding this comment.
Why not just remove all these?
There was a problem hiding this comment.
Will come in handy when the fuse-back is implemented (by copying these and restoring them to Observer/Subscriber). I'll delete them then
| * error | ||
| * @see <a href="http://reactivex.io/documentation/operators/last.html">ReactiveX operators documentation: Last</a> | ||
| */ | ||
| @BackpressureSupport(BackpressureKind.UNBOUNDED_IN) | ||
| @SchedulerSupport(SchedulerSupport.NONE) | ||
| public final Flowable<T> last() { | ||
| return takeLast(1).single(); | ||
| public final Single<T> last() { |
There was a problem hiding this comment.
This should return a Maybe<T> because the Flowable can be empty.
There was a problem hiding this comment.
The javadoc clearly states that an empty stream results in NoSuchElementException; but the former use of single() here is also a telltale sign.
There was a problem hiding this comment.
An observable being empty is not exceptional and therefore shouldn't result in exceptions.
There was a problem hiding this comment.
For this operator it is according to the 1.x docs:
https://github.com/ReactiveX/RxJava/blob/1.x/src/main/java/rx/Observable.java#L7398
Returns an Observable that emits the last item emitted by the source Observable or notifies observers of
a{@code NoSuchElementException}if the source Observable is empty.
2.x matches the 1.x behavior.
| @@ -7659,8 +7659,8 @@ public final Disposable forEachWhile(final Predicate<? super T> onNext, Consumer | |||
| * @see <a href="http://reactivex.io/documentation/operators/last.html">ReactiveX operators documentation: Last</a> | |||
| */ | |||
| @SchedulerSupport(SchedulerSupport.NONE) | |||
| public final Observable<T> last() { | |||
| return takeLast(1).single(); | |||
| public final Single<T> last() { | |||
There was a problem hiding this comment.
This should return a Maybe<T> because the Flowable can be empty.
|
P.S. I've pushed my current branch here. https://github.com/abersnaze/RxJava/tree/interop |
I see you didn't keep the original typed code to help with I'd have to redo/reoptimize them anyways so don't really need that. |
This PR changes the return type of
last()toSingle<T>and updates the relevant locations.Originally, it was implemented as
takeLast(1).single()so to reduce impact, all other original uses now have this inlined.