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

Paul Benedict pbenedict at apache.org
Wed Apr 27 00:05:27 UTC 2016

Are you asking Brian to set up another survey monkey for the masses? I
expect a happy silent majority and a raucous minority.... just based on
past results. :-)
On Apr 26, 2016 6:38 PM, "Vitaly Davidovich" <vitalyd at gmail.com> wrote:

I've yet to hear one supporter on this thread besides yourself and Stuart.
Is there some usability survey or something that actually indicates a
significant sample of people don't like the name? Guava Optional behaves
and is named the same way, and I've not heard anyone complain about that (I
and many people I know used it for years).

I think the conversation would be *maybe* different if picking a name for a
new thing, but the deprecation churn here adds no value, IMO, and is going
to be viewed as an annoyance by a large swath of people.

On Tuesday, April 26, 2016, Brian Goetz <brian.goetz at oracle.com> wrote:

> As the person who chose the original (terrible) name, let me weigh in...
> I think calling this method "get()" was our biggest API mistake in Java
> 8.  Now, one could argue that, if this is the biggest mistake we made,
> we did pretty darn good.  Which may be true, but ... make no mistake, it
> was the wrong name (mea culpa), and experience has borne out that it is
> widely misused.  Subjectively, about half the uses of .get() I see are
> wrong -- and wrong in the obvious, avoidable way -- they don't consider
> optional might be empty. So they've just traded an unexpected NPE for an
> unexpected NSEE.
> Its problem is, essentially: it looks harmless, but isn't, but it sure
> seems like the method you obviously want when you're browsing the
> autocomplete list / javadoc.  It's a hazard both to code writers (pick the
> wrong method because the name is bad) and to code readers (deceptively
> harmless looking.)
> Methods like orElse or ifPresent look harmless, and are; methods like
> orElseThrow have potentially harmful side-effects but have names that are
> transparent about what harm they could do.  But Optional.get() is neither
> safe nor transparent -- and yet awfully tempting-looking -- and this leads
> to frequent misuse.  Naming matters, and this one was wrong, and the harm
> is observable.  I wish I'd caught it before 8 shipped.
> Stepping back a bit ... one of the most frequent complaints we get is
> "mistakes never get fixed".  So, we've decided to be more serious about
> deprecation -- Dr. Deprecator is suiting up!  But that means, for any
> item, some people are going to disagree about whether deprecation is
> suitable, and some will be inconvenienced by the deprecation -- this is
> unavoidable.
> Why prioritize this one?  In part, because it's a *new* mistake, and has
> had less time to become entrenched -- and this is the very first
> opportunity we have to fix it.  If we can't fix an API mistake at the very
> first opportunity, the logical consequence is we can't ever fix anything.
> And that doesn't seem to be the world we want to retreat to.
> Similarly, is this the worst mistake in all of Java?  Surely not. But its
> also not reasonable to say "you can't fix X until you've fixed everything
> worse than X" -- again, that's a recipe for never fixing anything.  So, in
> the context of a deprecation effort, this seems an entirely reasonable
> candidate.
> I'd like to see it fixed, and the sooner the better.
> On 4/26/2016 6:43 AM, Stephen Colebourne wrote:
>> In OpenGamma Strata we have 546 uses of Optional.get(). Renaming this
>> would be painful and add no value.
>> While I understand the pain from some developers not understanding the
>> feature, this is hardly unique in the world of Java. Developers learn
>> the right way of doing something soon enough.
>> And while
>> if (opt.isPresent()) {
>>    opt.get()
>> }
>> is sometimes not ideal, in other cases it is the only practical choice
>> (eg. where the method needs to have a return statement inside the if
>> statement).
>> Changing this to
>> if (opt.isPresent()) {
>>    opt.getWhenPresent()
>> }
>> is just noise - I can see the "present" part twice.
>> I just don't think I can support the rename (although many of the
>> webrev tidy-ups are probably good).
>> Stephen
>> 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
>>> 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
>>> 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

Sent from my phone

More information about the core-libs-dev mailing list