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

[Peter Levart]

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.marks at 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