[PATCH] JDK-7033681 - Improve the documentation of Arrays.asList

Jaikiran Pai jai.forums2013 at gmail.com
Thu Sep 20 02:46:04 UTC 2018


Hello Stuart,


On 19/09/18 11:06 PM, Stuart Marks wrote:
>  
>
>> While adding this, I realized that the current javadoc doesn't mention
>> anything about the behaviour of this method when a null array is passed
>> to it. The implementation currently throws a NullPointerException. So I
>> went ahead and updated the javadoc to include:
>>
>>      * @throws NullPointerException if the passed array is {@code null}
>
> Strictly speaking this isn't necessary, as the class doc has the
> following statement:
>
>>  * <p>The methods in this class all throw a {@code
>> NullPointerException},
>>  * if the specified array reference is null, except where noted.
>

I hadn't noticed that :)
>
>
> **
>
> The patch you sent is pretty good as it stands, but there were a
> couple minor things that I think could still be improved. In the
> interest of time, instead of asking you to make the changes, I went
> ahead and modified the patch myself. The modifications are as follows:
>
> -     * <p>The returned list can be changed only in certain ways.
> Operations
> -     * that would change the size of the returned list leave the list
> unchanged
> -     * and throw {@link UnsupportedOperationException}.
>
> The first sentence didn't seem right to me, as it's quite vague. After
> some thought it finally occurred to me that what's necessary is a
> statement about the optional methods of the Collection interface. I've
> edited this paragraph thus:
>
> +     * <p>The returned list implements the optional Collection
> methods, except those
> +     * that would change the size of the returned list. Those methods
> leave
> +     * the list unchanged and throw {@link
> UnsupportedOperationException}.

This looks fine to me.
>
> In the last paragraph I inverted the sense of "not unmodifiable" and
> added a link to the Collections.unmodifiableList method.
>
> +     * <p>The list returned by this method <em>is modifiable.</em>
> +     * To create an unmodifiable list, see
> +     * {@link Collections#unmodifiableList Collections.unmodifiableList}
> +     * or <a href="List.html#unmodifiable">Unmodifiable Lists</a>.
This change too looks good.

>
>
> I've changed the <pre></pre> code samples to use <pre>{@code
> ...}</pre>. This allows use of < and > instead of HTML entities <
> and >.
Thank you, I'll keep that in mind for any future changes like this.
>
> Finally, I had to change the changeset metadata to conform to the
> OpenJDK rules. Specifically, the changeset author is required to be an
> OpenJDK user name. Since you don't have one, I've listed your email
> address in the Contributed-by line of the changeset comment.
Thank you. More of a FYI and if it matters from a process point of view
- in a couple of my earlier contributions, the sponsors have used the
"Contributed-by" line to be "Jaikiran Pai <jaikiran.pai at gmail.com>" like
here http://hg.openjdk.java.net/jdk/jdk/rev/6c394ed56b07. I don't have
any specific preference on which one is used.

>
> I've attached the modified patch. If you're ok with it, I'll proceed
> with filing a CSR.
I re-read the whole javadoc contained in your modified patch and it all
looks good to me. Thank you very much for the help in sponsoring,
reviewing and providing detailed inputs during the review.

-Jaikiran



More information about the core-libs-dev mailing list