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