JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

David Holmes David.Holmes at oracle.com
Mon Jul 18 10:44:29 UTC 2011


Hi Joe,

Seems okay. Can you use blenderrev (or specdiff or some other tool) to 
actually compare the generated javadoc output?

One stylistic comment. The {@inheritDoc} in the main comment block of 
each method is superfluous as the default behaviour is to inherit those 
javadoc attributes ie this:

     /**
      * {@inheritDoc}
      */

is unnecessary, and this:

     /**
       * {@inheritDoc}
       * @throws GenericSignatureFormatError {@inheritDoc}
       * @since 1.5
       */

is equivalent to:

     /**
       * @throws GenericSignatureFormatError {@inheritDoc}
       * @since 1.5
       */

Cheers,
David

Joe Darcy said the following on 07/18/11 17:51:
> Peter and David.
> 
> Thanks for the careful review; the @throws information still needs its 
> own {@inheritDoc}; I've uploaded a webrev with this and other corrections:
> 
> http://cr.openjdk.java.net/~darcy/7007535.4
> 
> More comments interspersed below.
> 
> Thanks,
> 
> -Joe
> 
> Peter Jones wrote:
>> Hi Joe,
>>
>> On Jul 15, 2011, at 1:49 AM, David Holmes wrote:
>>  
>>> On 14/07/2011 12:21 PM, joe.darcy at oracle.com wrote:
>>>    
>>>> Please code review my JDK 8 changes for
>>>>
>>>> 7007535: (reflect) Please generalize Constructor and Method
>>>> http://cr.openjdk.java.net/~darcy/7007535.3
>>>>
>>>> To summarize the changes, a new superclass is defined to capture the 
>>>> common
>>>> functionality of java.lang.reflect.Method and 
>>>> java.lang.reflect.Constructor.
>>>> That superclass is named "Executable" along the lines of
>>>> javax.lang.model.ExecutableElement, which models constructors and 
>>>> methods in
>>>> the JSR 269 language model.
>>>>
>>>> Both specification and implementation code are shared. To preserve 
>>>> the right
>>>> @since behavior, it is common that in Method/Constructor the javadoc 
>>>> for a
>>>> method will now look like:
>>>>
>>>> /**
>>>> * {@inheritDoc}
>>>> * @since 1.5
>>>> */
>>>>       
>>> Unless they have fixed/changed javadoc (entirely possible) it used to 
>>> be that the above would not cause @throws declarations for unchecked 
>>> exceptions to be inherited - you have/had to explicitly repeat them as:
>>>
>>> @throws <exception-type> {@inheritDoc}
>>>     
>>
>> Yes, that would seem to be needed for some of the inherited getters of 
>> generics info, which specify unchecked exception types.
>>
>>  
>>>> Since Executable is being created in JDK 8, it would be incorrect for
>>>> methods in that class to have an @since of 1.5; adding the @since in
>>>> Method/Constructor preserves the right information.
>>>>       
>>
>> In Executable.java, getAnnotation and getDeclaredAnnotations do have 
>> "@since 1.5"-- oversight?
>>   
> 
> Yes; that was incorrect.
> 
>> In Constructor.java and Method.java, getExceptionTypes has "@since 
>> 1.5", but that method has existed in those classes since 1.1.
>>
>>   
> Fixed.
> 
>> In Executable.java:
>>
>>  216     /**
>>  217      * Returns an array of {@code Class} objects that represent 
>> the formal
>>  218      * parameter types, in declaration order, of the method
>>  219      * represented by this {@code Method} object.  Returns an 
>> array of length
>>  220      * 0 if the underlying method takes no parameters.
>>  221      *
>>  222      * @return the parameter types for the method this object
>>  223      * represents
>>
>> At least "{@code Method}" needs to be generalized, and perhaps all 
>> occurrences of "method"?
>>   
> Corrected.



More information about the core-libs-dev mailing list