Search code examples
javaintellij-ideaoption-type

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


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()));
      //Some more stuff that is depends on a or b being present
    }
  }

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?


Solution

  • 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.