<Sound Dev> [10] Review Request: 8181566 JavaSound javadoc clarification

[Alex Menkov]

Hi Sergey,

I'm fine with this ("the same" (for 2nd case) looks more clear to me, 
but "identical" is also ok)

--alex

On 07/30/2017 16:07, Sergey Bylokhov wrote:
> Hi, Alex.
> Thank you for review!
> I would like to propose and discuss other text:
>
> If the "equals()" method is overridden to not use "==":
>
>     /**
>      * Indicates whether the specified object is equal to this info object,
>      * returning {@code true} if the objects are equal.
>      *
>      * @param  obj the reference object with which to compare
>      * @return {@code true} the specified object is equal to this info
>      *         object; {@code false} otherwise
>      */
>     public final boolean equals(Object obj) {
>
>
> And two versions if the "==" from Object is used:
>
>     /**
>      * Indicates whether the specified object is equal to this info object,
>      * returning {@code true} if the objects are identical.
>      *
>      * @param  obj the reference object with which to compare
>      * @return {@code true} the specified object is equal to this info
>      *         object; {@code false} otherwise
>      */
>     public final boolean equals(Object obj) {
>
> or:
>
>     /**
>      * Indicates whether the specified object is equal to this info object,
>      * returning {@code true} if the objects are the same.
>      *
>      * @param  obj the reference object with which to compare
>      * @return {@code true} the specified object is equal to this info
>      *         object; {@code false} otherwise
>      */
>     public final boolean equals(Object obj) {
>
>
> In all versions the text in "@return " tag is short and the same. But description have different words about the objects: "identical"/"equal"/"the same".
>
> ps: The text for the current fix was copied from the Integer.java which I usually use when in doubts.
>
> ----- alexey.menkov at oracle.com wrote:
>
>> MidiSystem.java:
>>
>> @@ -1096,22 +1113,26 @@
>>                       return device;
>>                   }
>>               }
>>           }
>>
>> - /* Provider class not specified or cannot be found, or
>> - provider class specified, and no appropriate device available or
>> - provider class and instance specified and instance cannot be found
>> or
>> is not appropriate */
>> + /*
>> + * Provider class not specified or cannot be found, or provider
>> class
>> + * specified, and no appropriate device available or provider class
>> and
>> + * instance specified and instance cannot be found or is not
>> appropriate
>> + */
>>
>> Old comment looks better (each "or" condition in a separate line)
>>
>> - /* No default are specified, or if something is specified,
>> everything
>> - failed. */
>> + /*
>> + * No default are specified, or if something is specified,
>> everything
>> + * failed.
>> + */
>>
>> "No default is specified" or "No defaults are specified"
>>
>>
>> AudioFileFormat.java:
>>
>> -         * Finalizes the equals method.
>> +         * Indicates whether the specified object is equal to this
>> file
>> type,
>> +         * returning {@code true} if the objects are the same.
>> +         *
>> +         * @param  obj the reference object with which to compare
>> +         * @return {@code true} if this file type is the same as the
>>
>> {@code obj}
>> +         *         argument; {@code false} otherwise
>>            */
>>           @Override
>>           public final boolean equals(final Object obj) {
>>
>> The implementation checks if the objects are equal, not that the
>> objects
>> are the same
>>
>> The same for AudioFormat.equals()
>>
>> (Note that in most cases equals() methods of JavaSound classes just
>> call
>> super.equals(), i.e. they really test is the objects are the same)
>>
>> --alex
>>
>>
>> On 07/20/2017 18:46, Sergey Bylokhov wrote:
>>> The DRAFT version of CSR was created:
>>> https://bugs.openjdk.java.net/browse/JDK-8185020
>>>
>>> ----- sergey.bylokhov at oracle.com wrote:
>>>>
>>>> The fix is updated:
>>>> http://cr.openjdk.java.net/~serb/8181566/webrev.01
>>>>
>>>>> I don't see the point of prettying up the docs on the un-used,
>> commented out constants.
>>>>> Can't we just delete them ? Seems like the decision was made
>> years ago not to include them in the API
>>>>
>>>
>>> It could be removed, but I still consider them as kind of todo, I
>> hope
>>> to check them one by one at some point.
>>>