How to check the field of an object on NPE and condition simultaneously through Optional? And if everything is ok, do nothing, else throw exception

Question

I use Java 8 and have an issue with how to check objects through Optional. It should be like

Optional.ofNullable(person)
        .map(Person::getAge)
        .filter(age -> age > 25)
        .orElseThrow(new CustomException())

However, this code returns the value, but I don't need this. How fix it? Code should be like this

If (noNPE && person.getAge() > 25) {
       // do nothing
} else {
       throw new CustomException();
}

I don't need return value due to sonar throw warnings.

what's wrong with the second snippet? Replace `noNPE` with `person!=null` and you're done — f1sh, Nov 16 '21 at 09:32
I need one more check in this case, that person.getAge() is not null. After that code is ugly and difficult to maintain. — Grigorii Kostarev, Nov 16 '21 at 09:55
Just because `Optional` exists, doesn't mean you should use it everywhere. A simple null-check (e.g. `if (person == null || person.getAge() <= 25) throw new CustomException();`) is IMHO far simpler and more readable than using `Optional`. — Mark Rotteveel, Nov 16 '21 at 10:02

score 2 · Accepted Answer · answered Nov 16 '21 at 19:49

This is not a proper use of Optional. Optional is not a general replacement for if statements. See Uses for Optional.

It’s really not so bad if you just write:

if (person != null && person.getAge() != null && person.getAge() < 25) {
    throw new CustomException();
}

But if you don’t like that, you can refactor the check into a new method in the Person class:

public void checkAge() {
    if (getAge() != null && getAge() < 25) {
        throw new CustomException();
    }
}

// Other class:

if (person != null) {
    person.checkAge();
}

Or, you can add a method to Person which only does the age logic:

public boolean tooYoung() {
    return (getAge() != null && getAge() < 25);
}

// Other class:

if (person != null && person.tooYoung()) {
    throw new CustomException();
}

Jens Piegsa · Answer 2 · 2021-11-16T10:14:43.463

0

The only thing that needs to be fixed in your code is the API usage, otherwise it won't compile:

Optional.ofNullable(person)
        .map(Person::getAge)
        .filter(age -> age > 25)
        .orElseThrow(CustomException::new);

(orElseThrow() needs to be fed with a Supplier)

There is no problem in ignoring the returned value.

Try @SuppressWarning("unused") or //NOSONAR to silence SonarQubes false positive.

edited Nov 16 '21 at 10:14

answered Nov 16 '21 at 09:52

Jens Piegsa

7,399
5
58
106

1

"The only thing"? The actual thing that needs to be fixed here is misuse of APIs in general. `if (person == null || person.getAge() <= 25) throw new CustomException();` is a lot simpler. – rzwitserloot Nov 16 '21 at 09:59
@rzwitserloot That's what I would call a style improvement. :-) As the question targetted `Optional`, in a real-world scenario, it also could have been returned from another method, so you may simply continue using its API. – Jens Piegsa Nov 16 '21 at 10:10

score -3 · Answer 3 · answered Nov 16 '21 at 09:15

-3

You could leverage ifPresent() as the predication in the if statement:

if Optional.ofNullable(person)
        .map(Person::getAge)
        .filter(age -> age > 25)
        .orElseThrow(new CustomException())
        .isPresent() {
       // do nothing
} else {
       throw new CustomException();
}

answered Nov 16 '21 at 09:15

Gao Hongtao

1

`orElseThrow` returns type `T`, not `Optional`, so you cannot call `isPresent()`. – Mark Rotteveel Nov 16 '21 at 10:03

How to check the field of an object on NPE and condition simultaneously through Optional? And if everything is ok, do nothing, else throw exception

3 Answers3