1

Given the following Function implementations...

Get User Credentials From Master Database

private Function<Long, Optional<UserCredentials>>
    getUserCredentialsFromMaster() {
        return userId -> Optional.ofNullable(userId)
            .flatMap(masterUserRepository::findById)
            .map(User::getCredentials);
}

Get User Credentials From Secondary Database

private Function<Long, Optional<UserCredentials>>
    getUserCredentialsFromSecondary() {
        return userId -> Optional.ofNullable(userId)
            .flatMap(secondaryUserRepository::findById)
            .map(User::getCredentials);
}

I need to execute either getUserCredentialsFromMaster or getUserCredentialsFromSecondary depending on where userId comes from.

Here below is my attempt:

Consider domain class UserProfile

public class UserProfile {
    Long id;
    Long internalUserId; // if internalUserId is null then externalUserId is not
    Long externalUserId; // and vice-versa
}

Attempt to obtain UserCredentials:

final UserProfile userProfile = userProfileRepository.findById(userProvileId);

final UserCredentials userCredentials =
    Optional.ofNullable(userProfile.internalUserId)
        .flatMap(getUserCredentialsFromMaster())
        .orElse(
            Optional.ofNullable(userProfile.externalUserId)
                .flatMap(getUserCredentialsFromSecondary())
                .orElseThrow(UserCredentialsNotFound::new));

internalUserId is not null but the statements above always throw UserCredentialsNotFound. I've tried to rewrite getUserCredentialsFromMaster and getUserCredentialsFromSecondary as plain Java methods invoked from an if-then-else block, and it worked as expected.

Am I missing something?

Improve this question
6
  • Both getImplementationStrategyByModelPortfolioId() and getImplementationStrategyByMandateId() are parameterless, i.e. they don't depend on internalUserId and externalUserId (you just need them to be non-null) or it's an unintentional mistake? Commented Dec 1, 2022 at 11:07
  • ... getImplementationStrategyByModelPortfolioId() and getImplementationStrategyByMandateId() were just copy-past from my actual code. In the example code they should have been getUserCredentialsFromMaster() and getUserCredentialsFromSecondary(), respectively. Sorry Commented Dec 1, 2022 at 11:12
  • By the way, either internalUserId or externalUserId must have an actual value. Commented Dec 1, 2022 at 11:14
  • One more clarification, your repository masterUserRepository::findById returns an Optional<User> or a nullable User instance? Commented Dec 1, 2022 at 12:05
  • It returns Optional<User>... but I could rework it to return a nullable object. It doesn't matter for me :-) Commented Dec 1, 2022 at 12:13

1 Answer 1

Reset to default
2

TL;DR

You're getting an exception because the argument of the Optional.orElse() is evaluated eagerly. I.e. it would be evaluated even if the Optional on which it was invoked is not empty.

But as you have said, either "internalUserId is null then externalUserId is not null and vice-versa" and orElseThrow() produces an exception.

Avoid using Optional to replace Null-checks

Firstly, note that Optional wasn't designed to perform null-checks. The design goal of the Optional is to serve as a return type, and its method ofNullable() is supposed to wrap a nullable return value, not to substitute a null-check.

You might be interested in reading:

And there's nothing wrong with implicit null-checks, it's a bit of a problem when there are a lot of them in the code, but it's rather an immediate consequence of how particular behavior in the application was implemented, then the issue related to the tools offered by the language.

The cleaner way to address this problem would be to get read of the functions and define a method producing a Supplier based on the provided id and UserRepository:

private Supplier<Optional<UserCredentials>> getUserCredentials(Long id, UserRepository repository) {
    
    return id == null ?
        Optional::empty :
        () -> repository.findById(id).map(User::getCredentials);
}

Which can be used in the following way:

UserCredentials userCredentials = Stream.of(
        getUserCredentials(userProfile.internalUserId, masterUserRepository),
        getUserCredentials(userProfile.internalUserId, secondaryUserRepository)
    )
    .map(Supplier::get)
    .flatMap(Optional::stream)
    .findFirst()
    .orElseThrow(UserCredentialsNotFound::new);

If you still want to keep using Optional

As a remedy you can use Java 9 Optional.or() which expects a Supplier of Optional which would be evaluated only if needed.

UserCredentials userCredentials = Optional.ofNullable(userProfile.internalUserId)
    .flatMap(getUserCredentialsFromMaster())
    .or(() ->
        Optional.ofNullable(userProfile.externalUserId)
            .flatMap(getUserCredentialsFromSecondary())
    )
    .orElseThrow(UserCredentialsNotFound::new);

Alternatively, you can make use Optional.orElseGet() which takes a Supplier of resulting value:

UserCredentials userCredentials = Optional.ofNullable(userProfile.internalUserId)
    .flatMap(getUserCredentialsFromMaster())
    .orElseGet(() ->
        Optional.ofNullable(userProfile.externalUserId)
            .flatMap(getUserCredentialsFromSecondary())
            .orElseThrow(UserCredentialsNotFound::new)
    );
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

This was also a solution I tried and it worked... but I'm wondering whether I could get rid of the lambda.
@j3d When you're writing something like foo(bar), argument bar should be evaluated before in order to invoke foo(). When instead of bar you have foo(baz()) then method baz() should be executed prior to the invocation of foo(). Hence in your case you have a choice between replacing orElseThrow() with orElse(defaultValue), or if you need to throw then - you need to use a Function via or(), or orElseGet().

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.