Hi,
If Optional.get() is the method that gets most attention from beginners
and learners, then perhaps its javadoc is the place that could be
improved by making it a honey pot for advice about Optional use. The
assumption that the average programmer starts reading documentation of
some new API from the class-level javadocs is perhaps wrong. Quick help
(Ctrl-Q in IDEs like IDEA, etc.) is context-sensitive and brings up the
method-level javadoc. That's where the story should begin, I think...
Regards, Peter
On 04/26/2016 01:10 PM, Tagir F. Valeev wrote:
Hello!
While I'm not a reviewer, I agree with Stephen. While I understand the
rationale, such renaming would cause even more confusion and pain.
Also I think this is not the worst part of Java API, so if we start to
rename things, where should we stop then?
With best regards,
Tagir Valeev.
SC> In OpenGamma Strata we have 546 uses of Optional.get(). Renaming this
SC> would be painful and add no value.
SC> While I understand the pain from some developers not understanding the
SC> feature, this is hardly unique in the world of Java. Developers learn
SC> the right way of doing something soon enough.
SC> And while
SC> if (opt.isPresent()) {
SC> opt.get()
SC> }
SC> is sometimes not ideal, in other cases it is the only practical choice
SC> (eg. where the method needs to have a return statement inside the if
SC> statement).
SC> Changing this to
SC> if (opt.isPresent()) {
SC> opt.getWhenPresent()
SC> }
SC> is just noise - I can see the "present" part twice.
SC> I just don't think I can support the rename (although many of the
SC> webrev tidy-ups are probably good).
SC> Stephen
SC> On 26 April 2016 at 00:05, Stuart Marks <stuart.ma...@oracle.com> wrote:
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
