JDK 9 RFR(s): 8167981: Optional: add notes explaining intended use

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

JDK 9 RFR(s): 8167981: Optional: add notes explaining intended use

Stuart Marks
Hi all,

Please review this small set of non-normative documentation changes for
java.util.Optional and related classes.

The notes describe the primary intent of Optional to be used as a return value,
and recommend avoiding using null as the value of a variable of Optional type.
Also, a note is added to get() directing readers to look at orElse() and
orElseGet().

Corresponding changes are also made to the primitive specializations
OptionalDouble, OptionalInt, and OptionalLong.

Bug:
        https://bugs.openjdk.java.net/browse/JDK-8167981

Webrev:
        http://cr.openjdk.java.net/~smarks/reviews/8167981/webrev.0/

Thanks,

s'marks
Reply | Threaded
Open this post in threaded view
|

Re: JDK 9 RFR(s): 8167981: Optional: add notes explaining intended use

Martin Buchholz-3
+     * The methods {@link #orElse(java.lang.Object) orElse()} and

It's sad that in 2017 we still can't write
{@link #orElse(T)}
... but ... we can and should write
{@link #orElse(Object) orElse(T)}
making the source and html more readable (orElse is not nullary)

+ * {@code OptionalDouble} is primarily intended for use as a method return
type where
+ * there is a clear need to represent "no result," and where using {@code
null}
+ * is likely to cause errors. A variable whose type is {@code
OptionalDouble} should

The obvious alternative to returning OptionalDouble is returning double,
where null is not possible. I would elide """and where using {@code null}
+ * is likely to cause errors""" unless you want to explicitly compare with
the alternative of returning Double,not double.

(More generally, it seems like Optional would have more value if there was
compiler enforcement of its never-null-ness.)


On Tue, Apr 18, 2017 at 6:23 PM, Stuart Marks <[hidden email]>
wrote:

> Hi all,
>
> Please review this small set of non-normative documentation changes for
> java.util.Optional and related classes.
>
> The notes describe the primary intent of Optional to be used as a return
> value, and recommend avoiding using null as the value of a variable of
> Optional type. Also, a note is added to get() directing readers to look at
> orElse() and orElseGet().
>
> Corresponding changes are also made to the primitive specializations
> OptionalDouble, OptionalInt, and OptionalLong.
>
> Bug:
>         https://bugs.openjdk.java.net/browse/JDK-8167981
>
> Webrev:
>         http://cr.openjdk.java.net/~smarks/reviews/8167981/webrev.0/
>
> Thanks,
>
> s'marks
>
Reply | Threaded
Open this post in threaded view
|

Re: JDK 9 RFR(s): 8167981: Optional: add notes explaining intended use

Paul Sandoz

> On 18 Apr 2017, at 20:29, Martin Buchholz <[hidden email]> wrote:
>
> +     * The methods {@link #orElse(java.lang.Object) orElse()} and
>
> It's sad that in 2017 we still can't write
> {@link #orElse(T)}
> ... but ... we can and should write
> {@link #orElse(Object) orElse(T)}
> making the source and html more readable (orElse is not nullary)
>
> + * {@code OptionalDouble} is primarily intended for use as a method return
> type where
> + * there is a clear need to represent "no result," and where using {@code
> null}
> + * is likely to cause errors. A variable whose type is {@code
> OptionalDouble} should
>
> The obvious alternative to returning OptionalDouble is returning double,
> where null is not possible. I would elide """and where using {@code null}
> + * is likely to cause errors""" unless you want to explicitly compare with
> the alternative of returning Double,not double.
>

“… and where using the default value (@code {0.0d}) is likely to cause errors."

?

Paul.

> (More generally, it seems like Optional would have more value if there was
> compiler enforcement of its never-null-ness.)
>
>
> On Tue, Apr 18, 2017 at 6:23 PM, Stuart Marks <[hidden email]>
> wrote:
>
>> Hi all,
>>
>> Please review this small set of non-normative documentation changes for
>> java.util.Optional and related classes.
>>
>> The notes describe the primary intent of Optional to be used as a return
>> value, and recommend avoiding using null as the value of a variable of
>> Optional type. Also, a note is added to get() directing readers to look at
>> orElse() and orElseGet().
>>
>> Corresponding changes are also made to the primitive specializations
>> OptionalDouble, OptionalInt, and OptionalLong.
>>
>> Bug:
>>        https://bugs.openjdk.java.net/browse/JDK-8167981
>>
>> Webrev:
>>        http://cr.openjdk.java.net/~smarks/reviews/8167981/webrev.0/
>>
>> Thanks,
>>
>> s'marks
>>

Reply | Threaded
Open this post in threaded view
|

Re: JDK 9 RFR(s): 8167981: Optional: add notes explaining intended use

Stuart Marks

Martin wrote:
> + * The methods {@link #orElse(java.lang.Object) orElse()} and
> It's sad that in 2017 we still can't write
> {@link #orElse(T)}
> ... but ... we can and should write
> {@link #orElse(Object) orElse(T)}
> making the source and html more readable (orElse is not nullary)

Well we're not going to fix the tooling right now, but I take your point about
the trailing empty parens. I often use foo() as a typographic marker to indicate
that "foo" is a method, even if it's not nullary; the javadocs aren't terribly
consistent about this. However, in this case I agree that they're unnecessary
since the wording clearly indicates that it's a method, and it's also a link
directly to the method declaration. So, I've removed the parens.

> + * {@code OptionalDouble} is primarily intended for use as a method return type where
> + * there is a clear need to represent "no result," and where using {@code null}
> + * is likely to cause errors. A variable whose type is {@code OptionalDouble} should
>
> The obvious alternative to returning OptionalDouble is returning double, where null is not possible. I would elide """and where using {@code null}
> + * is likely to cause errors""" unless you want to explicitly compare with the alternative of returning Double,not double.

I did have in mind the possibility of the return of a nullable Double
(respectively, Integer and Long). I did a quick grepcode search and I was pretty
easily able to find occurrences of methods that returned nullable Integer
references. I couldn't tell whether these were intended to indicate "no result"
or whether it was just sloppy programming.

But Paul also points out that 0.0d (resp., 0 and 0L) might be used as default
values. For int and long I can also imagine -1, MIN_VALUE, etc. also being used
as sentinels for "no result."

In any case I think it's best to sidestep this entire discussion for the
purposes of this text and simply omit the mention of null as Martin suggests.
This results in

    OptionalDouble is primarily intended for use as a method return type where
    there is a clear need to represent "no result."

Full stop. (Respectively for OptionalInt and OptionalLong.) I think that's clear
enough.

Revised webrev:

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

s'marks



On 4/19/17 9:58 AM, Paul Sandoz wrote:

>
>> On 18 Apr 2017, at 20:29, Martin Buchholz <[hidden email]> wrote:
>>
>> +     * The methods {@link #orElse(java.lang.Object) orElse()} and
>>
>> It's sad that in 2017 we still can't write
>> {@link #orElse(T)}
>> ... but ... we can and should write
>> {@link #orElse(Object) orElse(T)}
>> making the source and html more readable (orElse is not nullary)
>>
>> + * {@code OptionalDouble} is primarily intended for use as a method return
>> type where
>> + * there is a clear need to represent "no result," and where using {@code
>> null}
>> + * is likely to cause errors. A variable whose type is {@code
>> OptionalDouble} should
>>
>> The obvious alternative to returning OptionalDouble is returning double,
>> where null is not possible. I would elide """and where using {@code null}
>> + * is likely to cause errors""" unless you want to explicitly compare with
>> the alternative of returning Double,not double.
>>
>
> “… and where using the default value (@code {0.0d}) is likely to cause errors."
>
> ?
>
> Paul.
>
>> (More generally, it seems like Optional would have more value if there was
>> compiler enforcement of its never-null-ness.)
>>
>>
>> On Tue, Apr 18, 2017 at 6:23 PM, Stuart Marks <[hidden email]>
>> wrote:
>>
>>> Hi all,
>>>
>>> Please review this small set of non-normative documentation changes for
>>> java.util.Optional and related classes.
>>>
>>> The notes describe the primary intent of Optional to be used as a return
>>> value, and recommend avoiding using null as the value of a variable of
>>> Optional type. Also, a note is added to get() directing readers to look at
>>> orElse() and orElseGet().
>>>
>>> Corresponding changes are also made to the primitive specializations
>>> OptionalDouble, OptionalInt, and OptionalLong.
>>>
>>> Bug:
>>>        https://bugs.openjdk.java.net/browse/JDK-8167981
>>>
>>> Webrev:
>>>        http://cr.openjdk.java.net/~smarks/reviews/8167981/webrev.0/
>>>
>>> Thanks,
>>>
>>> s'marks
>>>
>
Reply | Threaded
Open this post in threaded view
|

Re: JDK 9 RFR(s): 8167981: Optional: add notes explaining intended use

Martin Buchholz-3
This looks good.

I don't think there's any need anymore to have fully qualified class names
in @links, so just:

{@link #orElseGet(Supplier) orElseGet}

(but there's a global cleanup there)

---

I would write
"result".
instead of
"result."

(but the latter has never made sense to me even as a child)


On Wed, Apr 19, 2017 at 11:48 AM, Stuart Marks <[hidden email]>
wrote:

>
> Martin wrote:
>
>> + * The methods {@link #orElse(java.lang.Object) orElse()} and
>> It's sad that in 2017 we still can't write
>> {@link #orElse(T)}
>> ... but ... we can and should write
>> {@link #orElse(Object) orElse(T)}
>> making the source and html more readable (orElse is not nullary)
>>
>
> Well we're not going to fix the tooling right now, but I take your point
> about the trailing empty parens. I often use foo() as a typographic marker
> to indicate that "foo" is a method, even if it's not nullary; the javadocs
> aren't terribly consistent about this. However, in this case I agree that
> they're unnecessary since the wording clearly indicates that it's a method,
> and it's also a link directly to the method declaration. So, I've removed
> the parens.
>
> + * {@code OptionalDouble} is primarily intended for use as a method
>> return type where
>> + * there is a clear need to represent "no result," and where using
>> {@code null}
>> + * is likely to cause errors. A variable whose type is {@code
>> OptionalDouble} should
>>
>> The obvious alternative to returning OptionalDouble is returning double,
>> where null is not possible. I would elide """and where using {@code null}
>> + * is likely to cause errors""" unless you want to explicitly compare
>> with the alternative of returning Double,not double.
>>
>
> I did have in mind the possibility of the return of a nullable Double
> (respectively, Integer and Long). I did a quick grepcode search and I was
> pretty easily able to find occurrences of methods that returned nullable
> Integer references. I couldn't tell whether these were intended to indicate
> "no result" or whether it was just sloppy programming.
>
> But Paul also points out that 0.0d (resp., 0 and 0L) might be used as
> default values. For int and long I can also imagine -1, MIN_VALUE, etc.
> also being used as sentinels for "no result."
>
> In any case I think it's best to sidestep this entire discussion for the
> purposes of this text and simply omit the mention of null as Martin
> suggests. This results in
>
>    OptionalDouble is primarily intended for use as a method return type
> where
>    there is a clear need to represent "no result."
>
> Full stop. (Respectively for OptionalInt and OptionalLong.) I think that's
> clear enough.
>
> Revised webrev:
>
> http://cr.openjdk.java.net/~smarks/reviews/8167981/webrev.1/
>
> s'marks
>
>
>
>
> On 4/19/17 9:58 AM, Paul Sandoz wrote:
>
>>
>> On 18 Apr 2017, at 20:29, Martin Buchholz <[hidden email]> wrote:
>>>
>>> +     * The methods {@link #orElse(java.lang.Object) orElse()} and
>>>
>>> It's sad that in 2017 we still can't write
>>> {@link #orElse(T)}
>>> ... but ... we can and should write
>>> {@link #orElse(Object) orElse(T)}
>>> making the source and html more readable (orElse is not nullary)
>>>
>>> + * {@code OptionalDouble} is primarily intended for use as a method
>>> return
>>> type where
>>> + * there is a clear need to represent "no result," and where using
>>> {@code
>>> null}
>>> + * is likely to cause errors. A variable whose type is {@code
>>> OptionalDouble} should
>>>
>>> The obvious alternative to returning OptionalDouble is returning double,
>>> where null is not possible. I would elide """and where using {@code null}
>>> + * is likely to cause errors""" unless you want to explicitly compare
>>> with
>>> the alternative of returning Double,not double.
>>>
>>>
>> “… and where using the default value (@code {0.0d}) is likely to cause
>> errors."
>>
>> ?
>>
>> Paul.
>>
>> (More generally, it seems like Optional would have more value if there was
>>> compiler enforcement of its never-null-ness.)
>>>
>>>
>>> On Tue, Apr 18, 2017 at 6:23 PM, Stuart Marks <[hidden email]>
>>> wrote:
>>>
>>> Hi all,
>>>>
>>>> Please review this small set of non-normative documentation changes for
>>>> java.util.Optional and related classes.
>>>>
>>>> The notes describe the primary intent of Optional to be used as a return
>>>> value, and recommend avoiding using null as the value of a variable of
>>>> Optional type. Also, a note is added to get() directing readers to look
>>>> at
>>>> orElse() and orElseGet().
>>>>
>>>> Corresponding changes are also made to the primitive specializations
>>>> OptionalDouble, OptionalInt, and OptionalLong.
>>>>
>>>> Bug:
>>>>        https://bugs.openjdk.java.net/browse/JDK-8167981
>>>>
>>>> Webrev:
>>>>        http://cr.openjdk.java.net/~smarks/reviews/8167981/webrev.0/
>>>>
>>>> Thanks,
>>>>
>>>> s'marks
>>>>
>>>>
>>
Reply | Threaded
Open this post in threaded view
|

Re: JDK 9 RFR(s): 8167981: Optional: add notes explaining intended use

Stuart Marks


On 4/20/17 10:03 AM, Martin Buchholz wrote:
> This looks good.
>
> I don't think there's any need anymore to have fully qualified class names in
> @links, so just:
>
> {@link #orElseGet(Supplier) orElseGet}
>
> (but there's a global cleanup there)

Thanks for reviewing.

Yes, "java.util.function.Supplier" is unnecessary. "Supplier" is sufficient in
this context, since the file imports java.util.function.Supplier, and javadoc
respects imports. I've fixed this occurrence and one other similar unnecessarily
fully-qualified name in the class doc. (Along with corresponding changes in
Optional{Double,Int,Long}.)

> I would write
> "result".
> instead of
> "result."
>
> (but the latter has never made sense to me even as a child)

Yeah, this is a long-standing usage dispute over typography, probably dating
back to before there were programming languages. I don't think we're going to
resolve it here. :-)

s'marks