Best way to check Optional's boolean value and throw exception if true-CodePudding

I have a nullable variable id, by which I can get a Location by getLocationById (again nullable).
What I want to do is if(id!=null && (location == null || location.isDeprecated())) => throw exception, otherwise do nothing.

Following is the way I can think of (other than get the boolean out first and then throw exception after an if statement). But it will throw exception when id is null, which is not what I want.

Wondering if there's a better way? (I'm using Java17)


Optional.ofNullable(id).map(service::getLocationById)
      .filter(Predicate.not(Location::isDeprecated))
      .orElseThrow(()->new BadRequestException("Location is deprecated"))

I figured I could do following but would be great if any better ideas:


Optional.ofNullable(id).map(service::getLocationById)
      .filter(Location::isDeprecated)
      .ifPresent(l->{ throw new BadRequestException("Location is deprecated");});

CodePudding user response：

Use a try catch block. Makes things simple when you're dealing with possible exceptions like the one in your case.

CodePudding user response：

It depends on your style and the style of your colleagues. The second behaves differently when location is null, so they are not the same. To make it behave similarly you'd have to tweak it like this:

Optional.ofNullable(id).map(service::getLocationById)
  .or(()-> Optional.of(createLocationWithDeprecatedFlag()))
  .filter(Location::isDeprecated)
  .ifPresent(l->{ throw new BadRequestException("Location is deprecated");});

Not quite as nice. So in your case I would suggest to go with your first approach. It is readable and from my point of view good to understand.

Optionals are not a replacement for conditionals and should be used when they make sense and not to simply avoid conditionals at all cost. Your case could as well be a more or less simple if, but optional isn't too bad here.