Nested Optional.get generates warning when checked and chained in orElse()-CodePudding

I've just stumbled upon a warning generated by IntelliJ and I'm wondering, do I miss something or is IntelliJ just ignoring the right side of the following or clause?

Example Code:

  Random random = new Random();
  public void test(){
    Optional<String> a = Optional.ofNullable(random.nextInt(10)>5?"something":null);
    Optional<String> b = Optional.ofNullable(random.nextInt(10)>5?"something":null);
    if(a.isPresent() || b.isPresent()){
      log.info(a.orElse(b.get()));
    }
  }

The warning 'Optional.get()' without 'isPresent()' check is shown on the b.get(). I get that the or is evaluated from left to right in a lazy way but I would assume that either a or b has to have a value present as it's checked explicitly in the if.

Is that an error in IntelliJ's static code analysis?

CodePudding user response：

Yes-ish. || short-circuits but orElse doesn't, so b.get still runs and raises an exception if b is absent, even if a is present. That's why Java provides orElseGet, which takes a Supplier (i.e. a zero-argument function) to be called if we actually need to run the 'else' part.

log.info(a.orElseGet(() -> b.get()));

Now, I don't have IntelliJ available on this computer right now, but I suspect it will still complain. The static analysis engine (probably) doesn't understand the interaction between orElseGet and ||. Generally, when I encounter situations like this, I factor the offending code out into a separate function.

void onFirst<T>(Optional<T> first, Optional<T> second, Consumer<T> consumer) {
  if (first.isPresent()) {
    consumer.accept(first.get());
  } else if (second.isPresent()) {
    consumer.accept(second.get());
  }
}

// Then call as
onFirst(a, b, (value) -> log.info(value));

A bit more wordy, but it passes static analysis muster. And the extra wordiness is off in a separate helper function that can be reused.

CodePudding user response：

I get that the or is evaluated from left to right in a lazy way

Yes, which means that if a.isPresent() returns true, b.isPresent() won't be evaluated and the condition will be true regardless. At which point your b.get() will throw an exception if b.isPresent() is false.

Since you're using a.orElse further down, you can simply check whether b.isPresent(), since orElse won't throw an exception if a is empty. So

if(b.isPresent()){
  log.info(a.orElse(b.get()));
}

shouldn't give a warning.

(I'm assuming this is a toy example, so I won't comment further on the abuse of Optional that is going on here)