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

Brian Goetz brian.goetz at oracle.com
Wed Apr 27 13:42:38 UTC 2016


Not that you'll find an additional voice compelling, but here's some 
additional anecdotal evidence from another channel:
    https://twitter.com/jeffreymaxwell/status/725313986053427200



On 4/26/2016 8:16 PM, Vitaly Davidovich wrote:
> I'm really grasping at straws here and asking for something 
> quantitative to substantiate this deprecation plan.  As it stands, 
> there is hearsay and some stackoverflow links - surely going through 
> with this (and inflicting some level of pain on folks who don't care 
> for this) deserves something a bit better? Brian mentioned that one 
> criticism often heard is mistakes aren't corrected - I fully believe 
> that's true.  But this may end up being one of those "they want to 
> correct mistakes and THIS is what they chose??!" things.  I understand 
> the appeal in "correcting" this given it's a relatively new addition, 
> but I don't think it'll go over well.
>
> On Tuesday, April 26, 2016, Paul Benedict <pbenedict at apache.org 
> <mailto:pbenedict at apache.org>> wrote:
>
>     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
>     <javascript:_e(%7B%7D,'cvml','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
>         <javascript:_e(%7B%7D,'cvml','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
>         <javascript:_e(%7B%7D,'cvml','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/%7Esmarks/reviews/8140281/webrev.0.langtools/>
>         >>>
>         >>>
>         http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.0.jdk/
>         <http://cr.openjdk.java.net/%7Esmarks/reviews/8140281/webrev.0.jdk/>
>         >>>
>         >>> Thanks,
>         >>>
>         >>> s'marks
>         >>>
>         >>
>         >
>
>         --
>         Sent from my phone
>
>
>
> -- 
> Sent from my phone




More information about the core-libs-dev mailing list