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?
-- Jon
On 04/25/2016 04:14 PM, Stuart Marks wrote:
One of the changes in the langtools webrev
http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.langtools/
is in the compiler. Please review.
Thanks,
s'marks
-------- Forwarded Message --------
Subject: RFR(m): 8140281 deprecate Optional.get()
Date: Mon, 25 Apr 2016 16:05:13 -0700
From: Stuart Marks <stuart.ma...@oracle.com>
To: core-libs-dev <core-libs-dev@openjdk.java.net>
Hi all,
Please review these webrevs that deprecate Optional.get() and to
replace it with Optional.getWhenPresent(). The corresponding changes
are also applied to OptionalDouble.getAsDouble(),
OptionalInt.getAsInt(), and OptionalLong.getAsLong().
Unlike most deprecations, this isn't about the function or the utility
of some API, it's about the name. The solution is basically to rename
the API. The problem is that "get" shows up as the "obvious" choice in
things like IDE code completion, leading to code that mishandles empty
Optionals. Typical Stack Overflow discourse runs something like this:
Q: what do I do with this Optional thing
A: just call get()
Q: thanks, it works!
Of course, it works until it doesn't.
Examining the JDK's use of Optional.get(), I didn't see very many
cases that called get() without first checking for the presence of a
value. But I did see quite a number of cases like this:
if (opt.isPresent()) {
doSomething(opt.get());
} else {
doSomethingElse();
}
In many of these cases, the code could be refactored to use other
Optional methods such as filter(), map(), or ifPresent().
In any case this reinforces the contention that use of get() leads to
poor code.
For this changeset, in just about all cases I've simply replaced the
call to get() with a call to getWhenPresent(). In a couple cases I
replaced the stream calls
.filter(Optional::isPresent).map(Optional::get)
with
.flatMap(Optional::stream)
which I hope will become the new idiom for unwrapping a stream of
Optionals.
While many cases could be cleaned up further, I didn't change them.
The reasons are that I didn't want to spend too much time putting code
cleanup into the critical path of this changeset (I'd be happy to help
later); doing so would create potential conflicts with code coming in
from the Jigsaw forest; and there are non-obvious places where
converting from a conditional to one of the lambda-based methods could
cause performance problems at startup.
There are also a few cases where simplification is prevented because
it would end up causing the resulting lambda expressions to throw
checked exceptions. :-(
Webrevs here:
http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.langtools/
http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.jdk/
Thanks,
s'marks
