Optional.isEmpty()

[Anthony Vanelverdinghe]

Hi Peter

I'd say no: it's merely the negation of an existing method, and given 
that the bar for adding methods to Optional is set very high (see e.g. 
[1] and [2]), I don't see how this one would meet it.

Moreover, I don't see any issues with simply writing:

     return !cf.findModule(target).isPresent();

and there are plenty of one-liners that don't use boolean negation as 
well, for example:

     return cf.findModule(target).orElse(null) == null; // [3]
     return cf.findModule(target).map(m -> false).orElse(true); // [4]
     return cf.findModule(target).stream().count() == 0;
     return cf.findModule(target).stream().allMatch(m -> false);
     return cf.findModule(target).isPresent() ^ true;
     ...
     return cf.findModule(target).equals(Optional.empty());

Adding a static import to the last one gets you very close to the 
proposed method already:
     return cf.findModule(target).equals(empty());

Of course, another option is to introduce a variable to avoid the 
combination of boolean negation and method chaining:
     boolean moduleFound = cf.findModule(target).isPresent();
     return !moduleFound;

Kind regards, Anthony

[1] https://bugs.openjdk.java.net/browse/JDK-8057557
[2] https://bugs.openjdk.java.net/browse/JDK-8058707
[3] http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-April/012273.html
[4] http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-April/012290.html


On 22/04/2017 11:40, peter.levart at gmail.com (Peter Levart) wrote:
> Hi,
>
> Seeing the following line in some JDK test that was up for review:
>
>       return cf.findModule(target).orElse(null) == null;
>
> I immediately jumped to suggest it would look better if written as:
>
>       return !cf.findModule(target).isPresent();
>
>
> But then I leaned back and asked myself: "Would it really?"
>
> The boolean negation in front of a chain of method calls tends to
> visually disappear from the view when we finally reach the offending
> method and might get missed when quickly browsing the code. In this
> respect, ".orElse(null) == null" is actually better. But the best would
> of course be something like that:
>
>       return cf.findModule(target).isEmpty();
>
> What do you think? Would this pull its weight?
>
> Regards, Peter
>
>