2.x: dedicated reduce() op implementations #4885
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Current coverage is 95.58% (diff: 83.13%)@@ 2.x #4885 diff @@
==========================================
Files 581 586 +5
Lines 37212 37373 +161
Methods 0 0
Messages 0 0
Branches 5601 5616 +15
==========================================
+ Hits 35619 35722 +103
- Misses 654 683 +29
- Partials 939 968 +29
|
jakeloo
reviewed
Nov 26, 2016
| this.actual = actual; | ||
| this.value = value; | ||
| this.reducer = reducer; | ||
| this.value = value; |
davidmoten
reviewed
Nov 27, 2016
| RxJavaPlugins.onError(e); | ||
| return; | ||
| } | ||
| value = null; |
davidmoten
reviewed
Nov 27, 2016
|
|
||
| @Override | ||
| public void onComplete() { | ||
| T v = value; |
There was a problem hiding this comment.
I guess the real problem here is you don't have those done related unit tests that I said I'd provide! I'll have a look at them soon.
davidmoten
reviewed
Nov 27, 2016
|
|
||
| @Override | ||
| public void onNext(T value) { | ||
| R v = this.value; |
There was a problem hiding this comment.
done checks in this class?
There was a problem hiding this comment.
No need for separate done flag as this.value can't be initially null and becomes null if onError is called. Updated the PR with more checks.
|
👍 with the qualifier that I'm not up on RxJava2/ReactiveStreams lifecycle in terms of subscription and disposal yet (that's for another day sorry!). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds dedicated operator implementations to
Flowable.reduce(seed, reducer)(returnsSingle),Flowable.reduceWith(seedSupplier, reducer)(returnsSingle),Observable.reduce(reducer)(returnsMaybe),Observable.reduce(seed, reducer)(returnsSingle) andObservable.reduceWith(seedSupplier, reducer)(returnsSingle)instead of using
scan().takeLast(1)(Flowable.reduce(reducer)already had a dedicated operator).Comparison (Celeron 1005M, 4GB RAM, Windows 7 x64, Java 8u112, JMH 1.16):
The new
ReducePerfbenchmark does a simple sum over a list of integer values. Unfortunately, this creates a lot of garbage for longer sequences (plus the CPU/RAM is not really suited for such benchmarks; theflowMaybelines should be roughly the same since the code didn't change but there is a significant run-to-run variance).