Avoid isPresent / get dance in Java-CodePudding

Say I have a piece of code that will try a few ways to find some value and, if unsuccessful, log all the ways that it tried.

Example:

public Optional<Integer> getFooOpt() {
  Optional<Integer> fooOpt = Optional.empty();
  List<String> needles = new ArrayList<>();

  Optional<Integer> barOpt = getBarOpt();

  if(barOpt.isPresent()) {
    Integer bar = barOpt.get();
    fooOpt = getFooOptByBar(bar);
    
    if(!fooOpt.isPresent()) {
      needles.add("bar "   bar);
    }
  }

  if(!fooOpt.isPresent()) {
    Optional<Integer> quxOpt = getQuxOpt();

    if(quxOpt.isPresent()) {
      Integer qux = quxOpt.get();
      fooOpt = getFooOptByQux(qux);
      
      if(!fooOpt.isPresent()) {
        needles.add("qux "   qux);
      }
    }
  }

  if(!fooOpt.isPresent()) {
    log.error("Not found by {}", needles);
  }

  return fooOpt;
}

Is there a way to restructure this code to avoid all the isPresent / get noise with Optionals here to make the code easier to read / follow?

CodePudding user response：

If you're heavily using Optional, you can try nesting Optional#orElse.

Assuming you have several methods try{something}ToComputeValue that all return Optional<Integer>:

Optional<Integer> value = tryFooToComputeValue()
  .orElse(tryBarToComputeValue()
    .orElse(tryBazToComputeValue()
      .orElseThrow(() -> throw new CannotComputeValue())));

That takes care of daisy chaining those. For logging, you could have each method log when it returns Optional.empty() (has failed to find the value). This doesn't fully match your desired behavior of logging only one statement with all methods. If you really need this, you can probably play with Optional#map or similar so that you add method names to needles when they fail to return a value.

CodePudding user response：

You can simplify this by moving some of the logic into support methods. You don't care about the barOpt and quxOpt references here - all you care about is if they can get your that sweet, sweet fooOpt goodness. I think this is a candidate for OptionalInt rather than Optional. Without the error log message construction you have something like:

OptionalInt fooOpt = getFooOptByBarOpt();
if (fooOpt.isPresent()) {
  return fooOpt;
}
fooOpt = getFooOptByQuxOpt();
if (fooOpt.isPresent()) {
  return fooOpt;
}
return OptionalInt.empty();

Looking at the error logging there doesn't seem to be a reason to compile a List of failed attempts. You only log if you didn't find a value and if you didn't find a value you know exactly what you attempted.

if (!fooOpt.isPresent()) {
  logger.error("Foo not found by bar or qux");
}
return fooOpt;