0

I'm using an optional ID here but I don't like having to declare TWO variables to receive the data. What is a more concise way to accomplish this without having to declare an extra OptionalOrg variable?

    Optional<Organization> optionalOrg;
    Organization org;

    if (request.getOrgId() != null) {
        optionalOrg = organizationRepository.findById(request.getOrgId());
    } else {
        optionalOrg = organizationRepository.findById(DEFAULT_ORG);
    }

    if (optionalOrg.isPresent()) {
        org = optionalOrg.get();
    } else {
        throw new ClientException("Org not found);
    }
Ole V.V.
  • 81,772
  • 15
  • 137
  • 161
dataman
  • 123
  • 9
  • @OleV.V well I tend to agree Optional gains you nothing, but then why does JPA make findById return an Optional as the standard type for a repository? If I change the default method then I have tons of places through the codebase that also have to be changed? – dataman Apr 27 '23 at 03:15
  • Sorry, missed that part. Sure, get an `Optional` from there. And use its `orElseThrow` method for getting its value or throwing your exception as shown in each of the answers. Don't use `isPresent()` and `get()`. – Ole V.V. Apr 27 '23 at 05:19
  • 2
    `Organization org = organizationRepository .findById(Objects.requireNonNullElse(request.getOrgId(), DEFAULT_ORG)) .orElseThrow(() -> new ClientException("Org not found"));` – Holger Apr 29 '23 at 15:41

3 Answers3

4

orElseThrow can be used to get the contained value, or throw an exception of your choice if empty.

org = organizationRepository
  .findById(id)
  .orElseThrow(() -> new ClientException("Org not found"));
Silvio Mayolo
  • 62,821
  • 6
  • 74
  • 116
3

I would probably do that:

var orgId = Objects.requireNonNullElse(request.getOrId(), DEFAULT_ORG);
var org = organizationRepository.findById(orgId)
                                .orElseThrow(() ->  new ClientException("Org not found));

Use the appropriate type instead of var if not Java 11++ and similar function for requireNonNullElse.

NoDataFound
  • 11,381
  • 33
  • 59
  • The second line of code is good. The first line is not. Optional is *not* a general replacement for if/else (or `?`/`:`). See https://stackoverflow.com/questions/23454952/uses-for-optional for comments from Java’s designers on how Optional is intended to be used. – VGR Apr 27 '23 at 02:06
0

try this but i'm not 100% sure..

Optional<Organization> optionalOrg = Optional.ofNullable(request.getOrgId())
    .flatMap(id -> organizationRepository.findById(id))
    .or(() -> organizationRepository.findById(DEFAULT_ORG));

Organization org = optionalOrg.orElseThrow(() -> new ClientException("Org not found"));
  • 1
    Now we’re at it, an alternative is `Optional .ofNullable(request.getOrgId()) .or(() -> DEFAULT_ORG) .flatMap(id -> organizationRepository.findById(id))`. It avoids the repetition of the call to `findById()`, so I would prefer it. – Ole V.V. Apr 27 '23 at 08:48
  • 2
    @OleV.V. it’s `orElseGet(() -> DEFAULT_ORG)`, as `or` expects a supplier of an optional, but for a simple variable access, I’d use `.orElse(DEFAULT_ORG)` instead of `.orElseGet(() -> DEFAULT_ORG)`… – Holger Apr 29 '23 at 15:43