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.ma...@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
