JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method
Joe Darcy
joe.darcy at oracle.com
Tue Feb 28 17:03:24 UTC 2012
Hi David,
Belatedly catching up on email, please review the patch below to address
the issues you've raised. I searched for "method" and replaced it with
"executable" as appropriate throughout the javadoc of the class.
Thanks,
-Joe
--- a/src/share/classes/java/lang/reflect/Executable.java Tue Feb 28
17:00:28 2012 +0400
+++ b/src/share/classes/java/lang/reflect/Executable.java Tue Feb 28
09:01:27 2012 -0800
@@ -180,7 +180,7 @@
/**
* Returns the {@code Class} object representing the class or
interface
- * that declares the method represented by this executable object.
+ * that declares the executable represented by this object.
*/
public abstract Class<?> getDeclaringClass();
@@ -215,18 +215,18 @@
* Returns an array of {@code Class} objects that represent the formal
* parameter types, in declaration order, of the executable
* represented by this object. Returns an array of length
- * 0 if the underlying method takes no parameters.
+ * 0 if the underlying executable takes no parameters.
*
- * @return the parameter types for the method this object
+ * @return the parameter types for the executable this object
* represents
*/
public abstract Class<?>[] getParameterTypes();
/**
* Returns an array of {@code Type} objects that represent the formal
- * parameter types, in declaration order, of the method represented by
- * this executable object. Returns an array of length 0 if the
- * underlying method takes no parameters.
+ * parameter types, in declaration order, of the executable
represented by
+ * this object. Returns an array of length 0 if the
+ * underlying executable takes no parameters.
*
* <p>If a formal parameter type is a parameterized type,
* the {@code Type} object returned for it must accurately reflect
@@ -236,16 +236,16 @@
* type, it is created. Otherwise, it is resolved.
*
* @return an array of {@code Type}s that represent the formal
- * parameter types of the underlying method, in declaration order
+ * parameter types of the underlying executable, in declaration
order
* @throws GenericSignatureFormatError
* if the generic method signature does not conform to the format
* specified in
* <cite>The Java™ Virtual Machine Specification</cite>
* @throws TypeNotPresentException if any of the parameter
- * types of the underlying method refers to a non-existent type
+ * types of the underlying executable refers to a non-existent type
* declaration
* @throws MalformedParameterizedTypeException if any of
- * the underlying method's parameter types refer to a parameterized
+ * the underlying executable's parameter types refer to a
parameterized
* type that cannot be instantiated for any reason
*/
public Type[] getGenericParameterTypes() {
@@ -277,15 +277,15 @@
* type, it is created. Otherwise, it is resolved.
*
* @return an array of Types that represent the exception types
- * thrown by the underlying method
+ * thrown by the underlying executable
* @throws GenericSignatureFormatError
* if the generic method signature does not conform to the format
* specified in
* <cite>The Java™ Virtual Machine Specification</cite>
- * @throws TypeNotPresentException if the underlying method's
+ * @throws TypeNotPresentException if the underlying executable's
* {@code throws} clause refers to a non-existent type declaration
* @throws MalformedParameterizedTypeException if
- * the underlying method's {@code throws} clause refers to a
+ * the underlying executable's {@code throws} clause refers to a
* parameterized type that cannot be instantiated for any reason
*/
public Type[] getGenericExceptionTypes() {
@@ -330,7 +330,7 @@
* Returns an array of arrays that represent the annotations on
* the formal parameters, in declaration order, of the executable
* represented by this object. (Returns an array of length zero if
- * the underlying method is parameterless. If the executable has
+ * the underlying executable is parameterless. If the executable has
* one or more parameters, a nested array of length zero is
* returned for each parameter with no annotations.) The
* annotation objects contained in the returned arrays are
On 07/20/2011 01:03 AM, David Holmes wrote:
> 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.
>>>
>>
More information about the core-libs-dev
mailing list