Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[Stuart Marks]

On 12/18/17 2:31 AM, Tagir Valeev wrote:

I think that both methods could co-exist with slightly different
semantics (even though they implementation is identical):
...
Think of get() as
an assertion-like method: if get() throws, then it's a bug in the code
(most likely right in this method).
...
If orElseThrow() throws, then
it's an acceptable situation (e.g. we are inside the library method
and it documents that NoSuchElementException could be thrown if some
precondition is not met).


Right, so the semantic difference is between an assertion check and a 
precondition check. I agree that there is a difference, but get() doesn't really 
mean "assertion check" to me. The orElseThrow() method could be used for both, 
without much loss. I think it's because we experts are used to get(), and it's 
all we had for both cases in JDK 8 and 9, that there is discomfort with the 
prospect of reducing the status of get().



However I don't like issuing a warning if
get() is used like in the code above. And if get() will be deprecated,
then we would need to issue warning on every get() usage. Thus I'm
against the deprecation.


Yes, better warnings control would clearly be necessary before we would proceed 
with deprecation.



I believe that for any kind of API you can find bad code which abuses
it regardless on how many years passed since the API was released.
That's not always the fault of API creators.


It's certainly possible to write bad code in any language, using any API, 
certainly right after it's been introduced. As time goes on, people learn more, 
articles are written and conference talks presented, etc., and good usage 
patterns tend to emerge.


The situation with get() seems qualitatively different. I consistently and 
repeatedly see it misused, and it doesn't seem to be getting any better. When 
this occurs, it indicates that there's something wrong with the API.


s'marks

Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[Tagir Valeev]

Hello!

I think that both methods could co-exist with slightly different
semantics (even though they implementation is identical):

The get() method should be called when it's evident from the narrow
context that Optional is present at the point and using functional
alternatives (like ifPresent()) is inconvenient at this place, e.g.:

  for(X x : list) {
Y y = calculateYPossiblyThrowingCheckedException(x);
Optional optional = getOptional(x, y);
if(optional.isPresent()) {
  return optional.get(); // get() will always succeed
}
  }
  return null; // or whatever else default value

Such loop is difficult to convert to Stream/Optional chain and not
very easy to get rid of isPresent/get. One possibility is to change to
  T t = getOptional(x, y).orElse(null);
  if(t != null) return t;
But it's really questionable (after all using optionals we want to get
rid of nulls). So I think having get() here is fine. Think of get() as
an assertion-like method: if get() throws, then it's a bug in the code
(most likely right in this method).

The orElseThrow() method should be called when we are not sure that
Optional is present (e.g. it's received from the method call), but we
are fine with default NoSuchElementException. If orElseThrow() throws,
then
it's an acceptable situation (e.g. we are inside the library method
and it documents that NoSuchElementException could be thrown if some
precondition is not met).

In IntelliJ IDEA we warn by default if get() is called and we cannot
statically determine that Optional is always present at this point
(e.g. isPresent() was checked). In future Java we may suggest to
replace with
orElseThrow() in such cases. However I don't like issuing a warning if
get() is used like in the code above. And if get() will be deprecated,
then we would need to issue warning on every get() usage. Thus I'm
against the deprecation.

> I know that you like get() (or at least you're OK with it). But even three
> years after the release of Java 8, I still see bad code written with get(),
> so I don't think this problem is going to go away.

I believe that for any kind of API you can find bad code which abuses
it regardless on how many years passed since the API was released.
That's not always the fault of API creators.

With best regards,
Tagir Valeev.

Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[Stuart Marks]

On 12/9/17 1:20 AM, Stephen Colebourne wrote:

On 8 December 2017 at 00:33, Stuart Marks  wrote:

Please review this changeset that introduces a new no-arg method
orElseThrow() to Optional, as a preferred alternative to the get() method.


I am willing to accept this addition as it has an obvious parallel to
the existing supplier method.

My position on removing get() has not changed however, and as such I
am displeased with the addition of the "preferred alternative" text.


Hi Stephen,

Thanks for your comments on this issue.

I know that you like get() (or at least you're OK with it). But even three years 
after the release of Java 8, I still see bad code written with get(), so I don't 
think this problem is going to go away.


Also, I should clarify that it was never the intent to remove get(), just to 
deprecate it not-for-removal. However, I realize that even deprecation has a 
cost, which is why I'm not pursuing it as part of this proposal.


s'marks

Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[forax]

- Mail original -
> De: "Stuart Marks" 
> À: "Remi Forax" , "Alan Bateman" 
> Cc: "core-libs-dev" 
> Envoyé: Samedi 9 Décembre 2017 00:48:08
> Objet: Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred 
> alternative to get()

 Please review this changeset that introduces a new no-arg method
 orElseThrow() to Optional, as a preferred alternative to the get()
 method.

>>> This looks good. The naming is consistent and I think better than the
>>> "getWhenPresent" proposed in the original thread.
>> 
>> i agree with Alan.
>> 
>> Having a name that starts with "get" as the great advantage as to be visible 
>> in
>> the IDE completion box just after the method get() so it will help people to
>> transition from get() to getWhenPresent()
> 
> I'm not sure whether you're agreeing or disagreeing. :-)

oops, read the Alan's reply too fast.

> 
> The current proposal is for orElseThrow().
> 
> Having a method that starts with "get" (such as "getWhenPresent" or
> "getOrThrow") is indeed helpful when doing auto-completion, but it sort-of
> starts to create a new family of get- methods that overlap with the existing
> orElse- methods in an odd way. I think the no-arg orElseThrow() fits better 
> with
> the existing three orElse- methods. This leaves get() as the outlier, which is
> ok if we maybe eventually deprecate it.

ok, make sense.

> 
>> BTW, i do not see the point to not deprecate get() at the same time.
> 
> Much of the resistance to the earlier proposal was about deprecation of get(),
> so I wanted to set that aside in order to proceed with introduction of this
> alternative.

ok

> 
> s'marks

Rémi

Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[forax]

- Mail original -
> De: "Stuart Marks" 
> À: "Remi Forax" 
> Cc: "core-libs-dev" 
> Envoyé: Samedi 9 Décembre 2017 01:00:06
> Objet: Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred 
> alternative to get()

> Please, let's not derail this thread. :-)
> 
> I think would be a good thing to think about, so I've filed a JIRA issue to
> track it:
> 
> https://bugs.openjdk.java.net/browse/JDK-8193279
> 
> s'marks


Thanks,
Rémi

> 
> 
> On 12/8/17 1:45 AM, Remi Forax wrote:
>> Let's gently derail this thread, the is another pain point with the current
>> optional API,
>> it seems that are no simple way to transform an Optional to an
>> OptionalInt and vice-versa.
>> 
>> It's painful because we start to see interface that returns by example
>> OptionalInt while the implementation you want to connect with return an
>> Optional.
>> 
>> The only workaround seems to be to use a Stream/IntStream:
>>   Optional -> OptionalInt
>> optional.stream().mapToInt(x -> x).findFirst()
>>   OptionalInt -> Optional
>> optionalInt.stream().boxed().findFirst();
>> 
>> I think, Optional should have the method mapTo*/flatMapTo* and
>> Optional[Primitive] the method boxed().
>> 
>> regards,
>> Rémi
>> 
>> - Mail original -
>>> De: "Stuart Marks" 
>>> À: "core-libs-dev" 
>>> Envoyé: Vendredi 8 Décembre 2017 01:33:41
>>> Objet: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred
>>> alternative to get()
>> 
>>> Hi all,
>>>
>>> Please review this changeset that introduces a new no-arg method 
>>> orElseThrow()
>>> to Optional, as a preferred alternative to the get() method.
>>>
>>> Corresponding methods are also added to OptionalDouble, Int, and Long.
>>>
>>> The orElseThrow() method is functionally identical to get(). It has some
>>> affinity with the existing orElseThrow(exceptionSupplier) method, being
>>> equivalent to orElseThrow(NoSuchElementException::new).
>>>
>>> The get() method is functionally unchanged. It is NOT being deprecated, 
>>> although
>>> some wording in the doc has been added to point to orElseThrow() as the
>>> "preferred alternative." This is part of a (slow) migration away from
>>> Optional.get(), which has an obvious, attractive name that is often misused.
>>> These issues have been discussed on this list previously:
>>>
>>>  
>>> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-April/040484.html
>>>
>>> Please note that much of that discussion was about the then-proposed 
>>> deprecation
>>> of Optional.get(), which is NOT part of this proposal. There are no plans to
>>> deprecate Optional.get() at this time. This proposal ONLY introduces a new
>>> method orElseThrow() that is identical in function to get().
>>>
>>> Bug report:
>>>
>>>  https://bugs.openjdk.java.net/browse/JDK-8140281
>>>
>>> Webrev:
>>>
>>>  http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.1/
>>>
>>> Thanks,
>>>
> >> s'marks

Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[Stephen Colebourne]

On 8 December 2017 at 00:33, Stuart Marks  wrote:
> Please review this changeset that introduces a new no-arg method
> orElseThrow() to Optional, as a preferred alternative to the get() method.

I am willing to accept this addition as it has an obvious parallel to
the existing supplier method.

My position on removing get() has not changed however, and as such I
am displeased with the addition of the "preferred alternative" text.

Stephen

Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[Stuart Marks]

Please, let's not derail this thread. :-)

I think would be a good thing to think about, so I've filed a JIRA issue to 
track it:


https://bugs.openjdk.java.net/browse/JDK-8193279

s'marks


On 12/8/17 1:45 AM, Remi Forax wrote:

Let's gently derail this thread, the is another pain point with the current 
optional API,
it seems that are no simple way to transform an Optional to an 
OptionalInt and vice-versa.

It's painful because we start to see interface that returns by example OptionalInt 
while the implementation you want to connect with return an Optional.

The only workaround seems to be to use a Stream/IntStream:
  Optional -> OptionalInt
optional.stream().mapToInt(x -> x).findFirst()
  OptionalInt -> Optional
optionalInt.stream().boxed().findFirst();

I think, Optional should have the method mapTo*/flatMapTo* and 
Optional[Primitive] the method boxed().

regards,
Rémi

- Mail original -

De: "Stuart Marks" 
À: "core-libs-dev" 
Envoyé: Vendredi 8 Décembre 2017 01:33:41
Objet: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred 
alternative to get()



Hi all,

Please review this changeset that introduces a new no-arg method orElseThrow()
to Optional, as a preferred alternative to the get() method.

Corresponding methods are also added to OptionalDouble, Int, and Long.

The orElseThrow() method is functionally identical to get(). It has some
affinity with the existing orElseThrow(exceptionSupplier) method, being
equivalent to orElseThrow(NoSuchElementException::new).

The get() method is functionally unchanged. It is NOT being deprecated, although
some wording in the doc has been added to point to orElseThrow() as the
"preferred alternative." This is part of a (slow) migration away from
Optional.get(), which has an obvious, attractive name that is often misused.
These issues have been discussed on this list previously:

 http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-April/040484.html

Please note that much of that discussion was about the then-proposed deprecation
of Optional.get(), which is NOT part of this proposal. There are no plans to
deprecate Optional.get() at this time. This proposal ONLY introduces a new
method orElseThrow() that is identical in function to get().

Bug report:

 https://bugs.openjdk.java.net/browse/JDK-8140281

Webrev:

 http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.1/

Thanks,

s'marks

Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[Stuart Marks]

Please review this changeset that introduces a new no-arg method
orElseThrow() to Optional, as a preferred alternative to the get()
method.


This looks good. The naming is consistent and I think better than the
"getWhenPresent" proposed in the original thread.


i agree with Alan.

Having a name that starts with "get" as the great advantage as to be visible in 
the IDE completion box just after the method get() so it will help people to transition 
from get() to getWhenPresent()


I'm not sure whether you're agreeing or disagreeing. :-)

The current proposal is for orElseThrow().

Having a method that starts with "get" (such as "getWhenPresent" or 
"getOrThrow") is indeed helpful when doing auto-completion, but it sort-of 
starts to create a new family of get- methods that overlap with the existing 
orElse- methods in an odd way. I think the no-arg orElseThrow() fits better with 
the existing three orElse- methods. This leaves get() as the outlier, which is 
ok if we maybe eventually deprecate it.



BTW, i do not see the point to not deprecate get() at the same time.


Much of the resistance to the earlier proposal was about deprecation of get(), 
so I wanted to set that aside in order to proceed with introduction of this 
alternative.


s'marks

Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[Remi Forax]

Let's gently derail this thread, the is another pain point with the current 
optional API,
it seems that are no simple way to transform an Optional to an 
OptionalInt and vice-versa.

It's painful because we start to see interface that returns by example 
OptionalInt while the implementation you want to connect with return an 
Optional.

The only workaround seems to be to use a Stream/IntStream:
 Optional -> OptionalInt
   optional.stream().mapToInt(x -> x).findFirst()
 OptionalInt -> Optional
   optionalInt.stream().boxed().findFirst();

I think, Optional should have the method mapTo*/flatMapTo* and 
Optional[Primitive] the method boxed().

regards,
Rémi 

- Mail original -
> De: "Stuart Marks" 
> À: "core-libs-dev" 
> Envoyé: Vendredi 8 Décembre 2017 01:33:41
> Objet: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred 
> alternative to get()

> Hi all,
> 
> Please review this changeset that introduces a new no-arg method orElseThrow()
> to Optional, as a preferred alternative to the get() method.
> 
> Corresponding methods are also added to OptionalDouble, Int, and Long.
> 
> The orElseThrow() method is functionally identical to get(). It has some
> affinity with the existing orElseThrow(exceptionSupplier) method, being
> equivalent to orElseThrow(NoSuchElementException::new).
> 
> The get() method is functionally unchanged. It is NOT being deprecated, 
> although
> some wording in the doc has been added to point to orElseThrow() as the
> "preferred alternative." This is part of a (slow) migration away from
> Optional.get(), which has an obvious, attractive name that is often misused.
> These issues have been discussed on this list previously:
> 
> 
> http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-April/040484.html
> 
> Please note that much of that discussion was about the then-proposed 
> deprecation
> of Optional.get(), which is NOT part of this proposal. There are no plans to
> deprecate Optional.get() at this time. This proposal ONLY introduces a new
> method orElseThrow() that is identical in function to get().
> 
> Bug report:
> 
> https://bugs.openjdk.java.net/browse/JDK-8140281
> 
> Webrev:
> 
> http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.1/
> 
> Thanks,
> 
> s'marks

Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[Remi Forax]

- Mail original -
> De: "Alan Bateman" 
> À: "Stuart Marks" , "core-libs-dev" 
> 
> Envoyé: Vendredi 8 Décembre 2017 10:10:15
> Objet: Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred 
> alternative to get()

> On 08/12/2017 00:33, Stuart Marks wrote:
>> Hi all,
>>
>> Please review this changeset that introduces a new no-arg method
>> orElseThrow() to Optional, as a preferred alternative to the get()
>> method.
>>
> This looks good. The naming is consistent and I think better than the
> "getWhenPresent" proposed in the original thread.

i agree with Alan.

Having a name that starts with "get" as the great advantage as to be visible in 
the IDE completion box just after the method get() so it will help people to 
transition from get() to getWhenPresent()

BTW, i do not see the point to not deprecate get() at the same time.

> 
> -Alan

Rémi

Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[Alan Bateman]

On 08/12/2017 00:33, Stuart Marks wrote:

Hi all,

Please review this changeset that introduces a new no-arg method 
orElseThrow() to Optional, as a preferred alternative to the get() 
method.


This looks good. The naming is consistent and I think better than the 
"getWhenPresent" proposed in the original thread.


-Alan

Re: RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[Brian Burkhalter]

On Dec 7, 2017, at 4:33 PM, Stuart Marks  wrote:

> Please review this changeset that introduces a new no-arg method 
> orElseThrow() to Optional, as a preferred alternative to the get() method.

Looks OK to me.

> Corresponding methods are also added to OptionalDouble, Int, and Long.
> 
> The orElseThrow() method is functionally identical to get(). It has some 
> affinity with the existing orElseThrow(exceptionSupplier) method, being 
> equivalent to orElseThrow(NoSuchElementException::new).
> 
> The get() method is functionally unchanged. It is NOT being deprecated, 
> although some wording in the doc has been added to point to orElseThrow() as 
> the "preferred alternative." This is part of a (slow) migration away from 
> Optional.get(), which has an obvious, attractive name that is often misused. 
> These issues have been discussed on this list previously:
> 
>http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-April/040484.html

Quite a discussion. Good thing you opted not to name it 
“Optional.dudeCallIsPresentBeforeCallingMe().”

Brian

RFR(s): 8140281 add no-arg Optional.orElseThrow() as preferred alternative to get()

[Stuart Marks]

Hi all,

Please review this changeset that introduces a new no-arg method orElseThrow() 
to Optional, as a preferred alternative to the get() method.


Corresponding methods are also added to OptionalDouble, Int, and Long.

The orElseThrow() method is functionally identical to get(). It has some 
affinity with the existing orElseThrow(exceptionSupplier) method, being 
equivalent to orElseThrow(NoSuchElementException::new).


The get() method is functionally unchanged. It is NOT being deprecated, although 
some wording in the doc has been added to point to orElseThrow() as the 
"preferred alternative." This is part of a (slow) migration away from 
Optional.get(), which has an obvious, attractive name that is often misused. 
These issues have been discussed on this list previously:


http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-April/040484.html

Please note that much of that discussion was about the then-proposed deprecation 
of Optional.get(), which is NOT part of this proposal. There are no plans to 
deprecate Optional.get() at this time. This proposal ONLY introduces a new 
method orElseThrow() that is identical in function to get().


Bug report:

https://bugs.openjdk.java.net/browse/JDK-8140281

Webrev:

http://cr.openjdk.java.net/~smarks/reviews/8140281/webrev.1/

Thanks,

s'marks