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

Vitaly Davidovich vitalyd at gmail.com
Wed Apr 27 13:51:52 UTC 2016 

I really don't know what to say -- "near 100% use error rate"? I don't know
that person nor whom he's representing when he says "our", but that's
appalling.  This has nothing to do with "ivory tower cranks", as he put
it.  Did you ask him why he thinks their use error rate is so high?

On Wed, Apr 27, 2016 at 9:42 AM, Brian Goetz <brian.goetz at oracle.com> wrote:

> 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>
> 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> 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
>>
>>
>
> --
> Sent from my phone
>
>
>

More information about the core-libs-dev mailing list