1.x: Single add doOnEach #4461
1.x: Single add doOnEach #4461
Conversation
Current coverage is 84.28% (diff: 100%)@@ 1.x #4461 diff @@
==========================================
Files 271 272 +1
Lines 17607 17647 +40
Methods 0 0
Messages 0 0
Branches 2684 2687 +3
==========================================
+ Hits 14824 14874 +50
+ Misses 1923 1913 -10
Partials 860 860
|
| throw new IllegalArgumentException("onNotification is null"); | ||
| } | ||
|
|
||
| Observer<T> observer = new Observer<T>() { |
There was a problem hiding this comment.
Why not natively subscribe with a subscriber that forwards to the action?
There was a problem hiding this comment.
I just looked at doOnError & doOnSuccess. If so those should be changed too, right?
|
I'll continue this on Monday and make all suggested changes |
|
ExceptionsTest#testOnErrorExceptionIsThrownFromSingleDoOnSuccess fails although I'm not quite sure what the purpose of that test is. Also it seems a bit off to me. Can you give some insights there? |
|
Looks like removing the toObservable.toSingle no longer wraps the subscriber into a SafeSubscriber and the test just crashes with a different exception. |
|
No exception is being thrown at all. |
|
Can you single step the test? In the orginal DoOnEach, it has throwifFatal an throwOrReport you missed btw. |
|
|
||
| @Experimental |
| /** | ||
| * Modifies the source {@link Single} so that it invokes an action when it calls {@code onSuccess} or {@code onError}. | ||
| * <p> | ||
| * <img width="640" height="310" src="https://raw.githubusercontent.com/wiki/ReactiveX/RxJava/images/rx-operators/doOnEach.png" alt=""> |
There was a problem hiding this comment.
we might need a new image here since there's no onNext
|
👍 |
Really not that happy with
onNotification.call(Notification.<T>createOnNext(t));do you guys have any other way of doing this? There's no way of creating a Notification that has theonCompletedplus a value. A new one could be introduced there though. Also thedoOnEachSuccesstest feels clunky.Also why does Single when using the
domethods does the job by using an Observable? And later converting it back to a Single again. Is it due to the way Single was implemented in 1.x? With Completable there is no converting of back and forth needed.Javadoc will follow once we sorted out the few nits here.
Fixes #4457