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

2.x: add ParallelTransformer interface, params-validation #5197

Merged
merged 2 commits into from
Mar 20, 2017

Conversation

akarnokd
Copy link
Member

This PR adds the ParallelTransformer interface to match the other XTransformer interfaces, adds @NonNull annotations to the ParallelFlowable operators and adds the ParallelFlowable class to the parameter validator test set.

@akarnokd akarnokd added this to the 2.1 milestone Mar 19, 2017
@codecov
Copy link

codecov bot commented Mar 19, 2017

Codecov Report

Merging #5197 into 2.x will decrease coverage by 0.06%.
The diff coverage is 100%.

@@             Coverage Diff              @@
##                2.x    #5197      +/-   ##
============================================
- Coverage     95.96%   95.89%   -0.07%     
+ Complexity     5683     5682       -1     
============================================
  Files           621      621              
  Lines         40611    40611              
  Branches       5632     5632              
============================================
- Hits          38973    38945      -28     
- Misses          657      672      +15     
- Partials        981      994      +13
Impacted Files Coverage Δ Complexity Δ
...n/java/io/reactivex/parallel/ParallelFlowable.java 100% <100%> (ø) 43 <2> (ø) ⬇️
src/main/java/io/reactivex/Observable.java 100% <100%> (ø) 506 <2> (ø) ⬇️
src/main/java/io/reactivex/Flowable.java 100% <100%> (ø) 522 <2> (ø) ⬇️
src/main/java/io/reactivex/Single.java 99.32% <100%> (ø) 131 <2> (ø) ⬇️
src/main/java/io/reactivex/Completable.java 100% <100%> (ø) 112 <2> (ø) ⬇️
src/main/java/io/reactivex/Maybe.java 100% <100%> (ø) 168 <2> (ø) ⬇️
...ternal/operators/flowable/FlowableSampleTimed.java 88.23% <0%> (-7.36%) 3% <0%> (ø)
...in/java/io/reactivex/subjects/BehaviorSubject.java 84.97% <0%> (-6.74%) 56% <0%> (-1%)
...ternal/operators/observable/ObservablePublish.java 90.35% <0%> (-5.27%) 9% <0%> (-1%)
...rnal/subscribers/SinglePostCompleteSubscriber.java 94.87% <0%> (-5.13%) 14% <0%> (-1%)
... and 49 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7df3e3c...cee64c8. Read the comment docs.

public final <U> ParallelFlowable<U> compose(Function<? super ParallelFlowable<T>, ParallelFlowable<U>> composer) {
return RxJavaPlugins.onAssembly(to(composer));
public final <U> ParallelFlowable<U> compose(@NonNull ParallelTransformer<T, U> composer) {
return RxJavaPlugins.onAssembly(composer.apply(this));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing check here for composer being null?

@@ -611,7 +611,7 @@ protected final boolean validate(Subscriber<?>[] subscribers) {
* @return the value returned by the converter function
*/
@CheckReturnValue
public final <U> U to(Function<? super ParallelFlowable<T>, U> converter) {
public final <U> U to(@NonNull Function<? super ParallelFlowable<T>, U> converter) {
try {
return converter.apply(this);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing check here for converter being null

Copy link
Member Author

Choose a reason for hiding this comment

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

Dereference will throw NPE automatically.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd still check it in order to be consistent and for the better error message knowing what exactly is null.

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated this and all other methods.

Copy link
Collaborator

@vanniktech vanniktech left a comment

Choose a reason for hiding this comment

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

Awesome 👍

@akarnokd akarnokd merged commit 354a16d into ReactiveX:2.x Mar 20, 2017
@akarnokd akarnokd deleted the ParallelTransformer branch March 20, 2017 08:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants