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

[David Holmes]

Just realized this has come in too late ...

Joe Darcy said the following on 07/20/11 05:49:
> Agreed; I've posted a BlenderRev corresponding to the current patch at:
> 
>    http://cr.openjdk.java.net/~darcy/7007535.4/BR-7007535.html

Thanks. So now I can more readily see that the doc for Executable isn't 
quite suitable for Constructor in a few places:

getDeclaringClass:

183     /**
184      * Returns the {@code Class} object representing the class or 
interface
185      * that declares the method represented by this executable object.
186      */

For Constructor "method" should be "constructor". But I think, looking 
at the terminology elsewhere that the above could be rewritten as:

"Returns the Class object representing the class or interface that 
declares the executable represented by this object."

getParameterTypes:

219      * represented by this object.  Returns an array of length
220      * 0 if the underlying method takes no parameters.

method -> executable

getGenericParameterTypes:

229      * parameter types, in declaration order, of the method 
represented by
230      * this executable object. Returns an array of length 0 if the
231      * underlying method takes no parameters.

Again change to " the executable represented by this object".
And then: underlying method -> underlying executable

241      *     parameter types of the underlying method, in declaration 
order
243      *     if the generic method signature does not conform to the 
format
247      *     types of the underlying method refers to a non-existent type
250      *     the underlying method's parameter types refer to a 
parameterized

method -> executable

In fact do a search for "method" in Executable and most occurrences 
should be changed to "executable".


Finally, in getModifiers, why delete the "as an integer. The Modifier 
class should be used to decode the modifiers. " ?


BTW I also find the multiple "Specified by:" references to be quite 
strange. There can only be one specification for a method.

David
-----

> Thanks,
> 
> -Joe
> 
> Mike Duigou wrote:
>> This looks good to me but I agree with David that it's probably 
>> important to look at the generated javadoc and specdiff output before 
>> finalizing.
>>
>> Mike
>>
>> On Jul 18 2011, at 00:51 , Joe Darcy wrote:
>>
>>  
>>> 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.
>>>     
>>
>>   
>