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

Stops Integer boxing via null checks in Observable.concatEager #6012

Closed
wants to merge 1 commit into from
Closed

Stops Integer boxing via null checks in Observable.concatEager #6012

wants to merge 1 commit into from

Conversation

jnlopar
Copy link
Contributor

@jnlopar jnlopar commented May 22, 2018

Replaces with verifyPositive as in other similar methods.

This seems to be a trivial change and makes this more consistent with other areas of the code. Let me know if it needs its own issue or any specific tests. Given the methods only accept primitive ints, and passing a null Integer would throw a runtime exception, I'm not sure there's anything needed outside of the already present testing.

…er boxing from primitives. Replaces with verifyPositive as in other similar methods.
@jnlopar jnlopar changed the title Stops Integer unboxing via null checks in Observable.concatEager Stops Integer boxing via null checks in Observable.concatEager May 22, 2018
@codecov
Copy link

codecov bot commented May 22, 2018

Codecov Report

Merging #6012 into 2.x will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #6012      +/-   ##
============================================
- Coverage     98.28%   98.26%   -0.03%     
+ Complexity     6161     6159       -2     
============================================
  Files           659      659              
  Lines         44522    44522              
  Branches       6201     6201              
============================================
- Hits          43757    43748       -9     
+ Misses          233      231       -2     
- Partials        532      543      +11
Impacted Files Coverage Δ Complexity Δ
src/main/java/io/reactivex/Observable.java 100% <100%> (ø) 539 <2> (ø) ⬇️
...l/operators/observable/ObservableFlatMapMaybe.java 84.96% <0%> (-5.89%) 2% <0%> (ø)
.../io/reactivex/internal/schedulers/IoScheduler.java 89.24% <0%> (-3.23%) 9% <0%> (ø)
...rnal/operators/flowable/FlowableFlatMapSingle.java 92.93% <0%> (-2.72%) 2% <0%> (ø)
...tivex/internal/schedulers/TrampolineScheduler.java 96.1% <0%> (-2.6%) 6% <0%> (ø)
...ternal/operators/completable/CompletableMerge.java 96.42% <0%> (-2.39%) 2% <0%> (ø)
...a/io/reactivex/internal/util/QueueDrainHelper.java 97.91% <0%> (-2.09%) 56% <0%> (-2%)
.../internal/disposables/ListCompositeDisposable.java 98% <0%> (-2%) 34% <0%> (-1%)
.../io/reactivex/disposables/CompositeDisposable.java 98.14% <0%> (-1.86%) 39% <0%> (-1%)
.../operators/observable/ObservableFlatMapSingle.java 90.29% <0%> (-1.5%) 2% <0%> (ø)
... and 23 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 5f1ce20...ed02bbf. Read the comment docs.

Copy link
Contributor

@artem-zinnatullin artem-zinnatullin left a comment

Choose a reason for hiding this comment

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

Noice

also means that there were no unit tests that'd check boundaries of the parameters but it's not that critical tbh

@akarnokd
Copy link
Member

There is already a reflection based range check for the base classes so these null checks can be simply removed; the delegated calls should perform the verifyPositive calls already too.

@akarnokd
Copy link
Member

Thanks for contribution. I've posted a broader solution to this problem in #6014.

@akarnokd akarnokd closed this May 23, 2018
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.

3 participants