CODEWITHSUNDEEP
javajava-8
MarcuX
MarcuX

Reputation: 331

When or in what case should I use Optional to check null?

Should I always use the Optional to check null in Java 8?

And is it a good practice that I use it in my DAO layer? Here's a snippet code of my DAO layer:

public Optional<Tag> retrieveTagByValue(String tagValue) {
    Tag tag = null;
    try {
        tag = em.createNamedQuery("Tag.findByValue", Tag.class)
                .setParameter("tagValue", tagValue)
                .getSingleResult();
    } catch (NoResultException e) {
        System.out.println(e.getMessage());
    }
    return Optional.ofNullable(tag);
}

Because in my Service layer, I want to know whether the tag is null or not, if it is null then I create a new tag. So I am wondering should I use Optional in this case, is there any better ways to improve this method? Thanks a lot!

Upvotes: 2

Views: 1012

Answers (3)

Eugene
Eugene

Reputation: 120858

I want to know whether the tag is null or not

than this is the perfect case to use Optional. And you got your explanations about how to refactor your code, but think about the caller of this method.

You are being very explicit that this method might return a result that is not present - expressed via Optional.empty or a result that might be present, expressed via Optional.get/isPresent etc. This way you force the caller to make some decisions based on whether the results in present or not. That is simply not the case when returning a reference.

Upvotes: 1

kolobezka
kolobezka

Reputation: 91

Should I always use the Optional to check null in Java 8?

If a method would return null in Java 7 and lower then you should use Optional.empty() in Java 8. Using Optional helps you to get rid of null checks and makes code easier to read. Optional is although used in streams.

I would rewrite the code in the following manner:

public Optional<Tag> retrieveTagByValue(String tagValue) {
    try {
        Tag tag = em.createNamedQuery("Tag.findByValue", Tag.class)
            .setParameter("tagValue", tagValue)
            .getSingleResult();       
        return Optional.of(tag);
    } catch (NoResultException e) {
        return Optional.empty();
    }
}

It's not considered to be a good practice to use System.out... for logging, you should use some logging library instead.

And is it a good practice that I use it in my DAO layer?

Personally, I would replace DAO with repository (check out Spring Data) and I would use Optional in Service layer instead.

Upvotes: 2

Holger
Holger

Reputation: 298233

I’d call your example an anti-pattern unnecessarily introducing null, regardless of how your perform the subsequent null-check.

Consider:

public Optional<Tag> retrieveTagByValue(String tagValue) {
    try {
        Tag tag = em.createNamedQuery("Tag.findByValue", Tag.class)
                .setParameter("tagValue", tagValue)
                .getSingleResult();
        return Optional.of(tag);
    } catch (NoResultException e) {
        System.out.println(e.getMessage());
        return Optional.empty();
    }
}

This way, we’re not using Optional to perform a null check, we’re using it to not introduce a null in the first place.

It’s still debatable whether it is an improvement for the caller, to get an empty Optional in the erroneous case and having to loop-back and parse the standard output to find out the actual reason, compared to catching a meaningful exception.

But if not having a result is considered to be within the normal operation of this method (compare with Stream.findAny()), returning an empty Optional is fine, but then, it shouldn’t be necessary to print a message to System.out.

Upvotes: 4

Related Questions