Hi,
IMHO,boolean isEmpty() would be a good complement to the existing
empty() method.
$.02, Roger
On 4/24/2017 1:15 PM, Anthony Vanelverdinghe wrote:
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
