Fwd: RFR(m): 8140281 deprecate Optional.get()

[Stuart Marks]

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