Clean up Optional-related code. #9285
Conversation
AudricV
added
the
codequality
Isira-Seneviratne
force-pushed
the
Optional_cleanup
branch
from
fd9a8e6
to
982f6a0
Compare
Isira-Seneviratne
force-pushed
the
Optional_cleanup
branch
from
b0ac2c9
to
4e9ce88
Compare
|
Why do you think
|
Yeah, the |
| final var source = playbackListener.sourceOf(stream, streamInfo); | ||
|
|
||
| return Optional.ofNullable(source) |
There was a problem hiding this comment.
And also turn source1 into source below
| final var source = playbackListener.sourceOf(stream, streamInfo); | |
| return Optional.ofNullable(source) | |
| return Optional.ofNullable(playbackListener.sourceOf(stream, streamInfo)) |
| }, () -> { | ||
| reloadPlayQueueManager(); | ||
| setRecovery(); | ||
| }); |
There was a problem hiding this comment.
Maybe add a comment in this lambda to make it clear that it is called when we have no current stream info
Oh great! I had gradle out of sync with the ide |
Isira-Seneviratne
force-pushed
the
Optional_cleanup
branch
from
4e9ce88
to
e8216b2
Compare
| .map(item -> item.localConfiguration) | ||
| .map(localConfiguration -> localConfiguration.tag) |
There was a problem hiding this comment.
map will crash if the return value of the lambda is null. I looked it up but there does not seem to be a mapNullable (just like ofNullable exists). So I used flatMap(... -> Optional.ofNullable(...))
jshell> Optional.of(new Object()).map(null)
| Exception java.lang.NullPointerException
| at Objects.requireNonNull (Objects.java:233)
| at Optional.map (Optional.java:256)
| at (#2:1)
jshell> Optional.of(new Object()).flatMap(o -> Optional.ofNullable(null))
$4 ==> Optional.emptyThere was a problem hiding this comment.
null values returned in the lambda are converted to empty Optionals in the map method: https://developer.android.com/reference/java/util/Optional#map(java.util.function.Function%3C?%20super%20T,%20?%20extends%20U%3E)
You passed a null value for the lambda argument, not as the return type (the correct notation would be item -> null).
There was a problem hiding this comment.
You are right, I feel so stupid now... I will revert that part of my commit
| || getRoot().map(View::getParent).orElse(null) == null) { | ||
| if (!isPlayerAndPlayerServiceAvailable() || player.UIs().get(MainPlayerUi.class).isEmpty() | ||
| || getRoot().map(View::getParent).isEmpty()) { |
There was a problem hiding this comment.
Your code is perfectly equivalent, but I think the .map(View::getParent) was wrongly being used to allow getParent to return null.
|
Kudos, SonarCloud Quality Gate passed! |
What is it?
Description of the changes in your PR
SimplifyOptionalCallChains, as theOptional#isEmpty()method is now available in the Android SDK starting with version 33 and is desugared.Optionalmethod chaining.Fixes the following issue(s)
APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.
Due diligence