[Java] Optional.orElse does not return a constant value

#1

When using Optionals, whatever code is provided in the .orElse will be executed everytime the Optional is evaluated, not just when the Optional is empty and the .orElse would be hit. Sometimes people make the mistake of putting code in the .orElse that has a side effect that can affect performance (e.g. making a database query to get some kind of default value) or can result in bugs (e.g. running a database query that updates some value that should only be done when the Optional is empty). In these cases .orElseGet should be used as it defers execution until the Optional actually evaluates to empty and the .orElseGet needs to be run.

See also this popular StackOverflow thread on .orElse vs .orElseGet where the delayed execution is referenced as one of the main differences

Non-Compliant code:

Optional.ofNullable(getSpecificItemFromDB(itemId))
    .orElse(getDefaultItemFromDB())

This will call getDefaultItemFromDB() every time this optional is called, even if getSpecificItemFromDB returns a non-null value.

Compliant code:

Optional.ofNullable(getSpecificItemFromDB(itemId))
    .orElseGet(() -> getDefaultItemFromDB())

Now getDefaultItemFromDB is only called if getSpecificItemFromDB returns a null value.

To pick up on this, sonar could inspect the contents of .orElse and check if the value is constant or not

#2

Hello @matt-williams8,

Welcome to this community and thank you for this suggestion. I agree with you that this looks like a great candidate for a rule. I have therefore created the following RSPEC: https://jira.sonarsource.com/browse/RSPEC-5646.

For the time being, it will have to be reviewed internally and I cannot guarantee if/when this will be implemented.

#3

Isn’t this very similar to the existing RSPEC 4064? The only difference would be that the new spec explicitly mentions that it should only apply if the argument is not a constant, whereas the old spec only mentions that implicitly in the linked Google Groups discussion. I think you should just amend the old spec with that detail.

#4

Hello @CrushaKRool,

Indeed! I missed that one when searching whether a similar rule already existed!
Thank you for pointing it out :slight_smile: