Should i use ofNullable after checking if the value is null?

Question

i have this method for setting the auditor in my app:

public Optional<User> getCurrentAuditor() {
        Authentication authentication = SecurityContextHolder.getContext().getAuthentication();

        if (authentication==null || !authentication.isAuthenticated() || (authentication instanceof AnonymousAuthenticationToken)) {
            return Optional.empty();
        }
        User user = (User) authentication.getPrincipal();

        return Optional.ofNullable(user);
    }

from what i understood about ofNullable from this question. It expects the value to be null and will return Optional.empty() anyways, so is that check if authentication is null or not required or can i remove it.

Answer 1

I think you are confusing some things here. Or you are confusing me.

The null-check on authentication

if (authentication==null 

is necessary, since otherwise, authentication.getPrincipal() will throw an NullPointerException.

Optional.ofNullable(user) allows the user to be null without throwing an exception. This is probably what you want.

Although the question really is what do you want to happen if the authentication was successful but the principal is still null.

Do you want a NullPointerException to be thrown? Use Optional.of.

Do you want an empty Optional to be returned? Use Optional.ofNullable.

Do you want some special exception to be thrown? Implement an extra check.