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

Wed Apr 27 04:20:47 UTC 2016

Guava owner here, and yes, I've never heard such a complaint about our
Optional.get() method. This class has about a quarter of a million usages
inside Google alone (I'm not kidding), and people seem quite happy with it.

We have been planning to migrate these usages from our Optional class to
yours (sounds crazy, but this is how we roll). Threads like this one -- and
the threat of changing Optional in-place incompatibly to become a value
type as part of Valhalla -- are making us, for the first time, seriously
question whether that is such a good idea.

Please don't rename this method.

On Tue, Apr 26, 2016 at 4: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,
> then
> > 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
> the
> > 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
> given
> > 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
> 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
> >>>
> >>
> >
>
> --
> Sent from my phone
>

-- 
Kevin Bourrillion | Java Librarian | Google, Inc. | kevinb at google.com