On 4/25/16 4:35 PM, Jonathan Gibbons wrote:
Since the justification for this change appears to be that the IDEs might help
write people write bad code, why not look to the IDEs to generate a warning when
using Optional.get outside of the context of Optional.isPresent?
In other words, is this really such a good change?
Well I think it's more than just IDEs. The root cause is the API name "get". In
Java we're getting things all the time, and we think nothing of it. The problem
is when code tries to get something that isn't there. Historically, callers have
had to check for null, and of course it's always been a problem if that check is
forgotten.
We've introduced Optional to avoid this. The problem is that the obvious thing
to do is to get() the value out of the Optional, but this buys us right back
into the what-if-the-value-is-missing case that we had with nulls.
Consider this code from LogManager.java from the jdk webrev: [1]
2106 for (String pk : properties) {
2107 ConfigProperty cp = ConfigProperty.find(pk).get();
OK, so we find something and get something out of it. Big deal. But with the
change to getWhenPresent(), this code is now:
2106 for (String pk : properties) {
2107 ConfigProperty cp =
ConfigProperty.find(pk).getWhenPresent();
From this, it's clear that find() returns an Optional, and that this code is
asserting that a value is always present in this Optional.
Offhand I don't know if this assertion is in fact true for this code. If it is
true, then the name "getWhenPresent" is telling the reader an important fact
about the relationships among the data structures this code uses. If it's *not*
true, when this code is being reviewed (or when it's debugged after a failure),
it ought to raise questions like, "Under what circumstances is this value
absent?" Those are important points that aren't raised by a simple get().
s'marks
[1]
http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.jdk/src/java.logging/share/classes/java/util/logging/LogManager.java.sdiff.html
