3

I'm learning about Optional class. I'm just a bit confused about the correct use of the method Optional.ifPresent(Consumer<? super T> consumer) .

I see this example on one project:

Optional.ofNullable(user.getIdentifiantAAA())
.ifPresent(id -> identifiants.add(new Identifiant(id, IdentifiantType.AAA));

IMHO, this is less readable than:

if (user.getIdentifiantAAA() != null) {
   identifiants.add(user.getIdentifiantAAA());
}

The class Optional is:

A container object which may or may not contain a non-null value. If a value is present, isPresent() will return true and get() will return the value.

I feel that the use of Optional.ifPresent of the example breaks the principal propose of this class. An instance of Optional is created only to use the isPresent method, is it really necessary for this case?

So, my question is: should be wrap objects in Optional to ckeck non null and execute code with the method Optional.ifPresent(Consumer<? super T> consumer)?

Jesus Zavarce
  • 1,729
  • 1
  • 17
  • 27
  • Take a look at this answer: https://stackoverflow.com/questions/24228279/proper-usage-of-optional-ifpresent – Centos Nov 07 '18 at 20:37
  • 2
    It might be more reasonable if the value was already an optional, instead of a nullable value. – Louis Wasserman Nov 07 '18 at 20:41
  • 3
    I think Optional is intended to be used as return value from functions. The general guidance is to use it at the point of an public interface (so for example `getIdentifiantAAA` might return an Optional) and not absolutely everywhere. You probably wouldn't use it in the example you show, where it adds unnecessary overhead for little benefit. – Paul Rooney Nov 07 '18 at 20:48
  • 1
    Your opinion is correct. That first example is highly inappropriate. Optional is not supposed to be a fancy replacement for `if` statements. Authoritative description of the purpose of Optional: https://stackoverflow.com/a/23464794/1831987 – VGR Nov 07 '18 at 22:23

5 Answers5

4

Optional<T> is not meant as a replacement for simple if checks, the first example is definitely not the reason why Optional<T> was created.

it would only make sense to use ifPresent if you're performing some extended logic as such of:

Optional.ofNullable(user.getIdentifiantAAA())
        .map(...)
        .ifPresent(...);

or if the value you're operating on is already an Optional<T>, maybe an Optional<T> returned from a method call or some computation.

Ousmane D.
  • 54,915
  • 8
  • 91
  • 126
  • 2
    I disagree with the "extended logic" part. Even if it looks so convenient, there is no reason to wrap everything up into `Optional` – Andrew Tobilko Nov 07 '18 at 21:07
  • @AndrewTobilko sure, thing. I am not saying one "needs to" wrap it in an optional but as mentioned "it would make sense" had there been extended logic. – Ousmane D. Nov 07 '18 at 21:25
2

In Optional the mother of all bike sheds talk @Devoxx US, Stuart Marks from Oracle details how are optional useful (and some more aspects about optional)

The main use case where are useful is for returns, as @Paul Rooney mentioned too

1

I dare say that there are no requirements for a Consumer that the method ifPresent takes. Personally, I see nothing wrong with the line:

.ifPresent(id -> identifiants.add(new Identifiant(id, IdentifiantType.AAA));

The problem here is that you wrapped user.getIdentifiantAAA() in Optional just to use its methods, which is a bad practice and pertains to this thread.

Andrew Tobilko
  • 48,120
  • 14
  • 91
  • 142
0

Not sure there's a wrong way to use it, but one way I use it a lot is with Functional Interfaces.

Consider this code:

Lock lock;

try {
   lock = acquireLock();
   // do some work
}
finally {
    if(lock != null) {
        releaseLock(lock);
    }
}

Using Optional I can make this a little cleaner:

Optional<Lock> lock;

try {
   lock = acquireLock();
   // do some work
}
finally {
    lock.ifPresent(this::releaseLock);
}
Mike
  • 4,722
  • 1
  • 27
  • 40
-1

It really depends on preference. The whole point of Optional is to void dealing with null like you have in if (user.getIdentifiantAAA() != null).

tsolakp
  • 5,858
  • 1
  • 22
  • 28