On Tue, 20 Apr 2021 17:42:27 GMT, Stuart Marks <sma...@openjdk.org> wrote:
>> I decided to show a complete static method in the example, so it could be
>> copied to user utility class as is. Not sure if it's reasonable to add
>> `assert cls.isRecord();` there. Also I don't know whether there's a
>> limitation on max characters in the sample code. Probable a line break in
>> `static <T extends Record>\nConstructor<T> getCanonicalConstructor(Class<T>
>> cls)` is unnecessary.
>>
>> ---
>> Aside from this PR, I've found a couple of things to clean up in
>> `java.lang.Class`:
>> 1. There's erroneous JavaDoc link in `getSimpleName()` JavaDoc (introduced
>> by @jddarcy in #3038). It should be `#isArray()` instead of `isArray()`.
>> 2. Methods Atomic::casAnnotationType and Atomic::casAnnotationData have
>> unused type parameters `<T>`.
>> 3. Probably too much but AnnotationData can be nicely converted to a record!
>> Not sure, probably nobody wants to have `java.lang.Record` initialized too
>> early or increasing the footprint of such a basic class in the metaspace, so
>> I don't insist on this.
>>
>>
>> private record AnnotationData(
>> Map<Class<? extends Annotation>, Annotation> annotations,
>> Map<Class<? extends Annotation>, Annotation> declaredAnnotations,
>> // Value of classRedefinedCount when we created this AnnotationData
>> instance
>> int redefinedCount) {
>> }
>>
>>
>> Please tell me if it's ok to fix 1 and 2 along with this PR.
>
> Thanks for writing this example.
>
> I think that the example lines can be longer. I'd suggest putting the main
> part of the method declaration on the same line as `static <T extends
> Record>`, but leaving the `throws` clause on the next line.
>
> I think including the small cleanups (1) and (2) in this PR is fine. Changing
> `AnnotationData` to be a record seems like it might have other effects, so
> I'd leave that one out.
>
> One other thing I'd like to see is a link to this example code from places
> where people are likely to look for it. The class doc for `java.lang.Record`
> has a definition for "canonical constructor" so it would be nice to link to
> the example here. Something like "For further information about how to find
> the canonical constructor reflectively, see Class::getRecordComponents."
> (With appropriate javadoc markup.) This could either be a parenthetical
> comment somewhere in the "canonical constructor" discussion, or possibly a
> separate paragraph in the `@apiNote` section below.
@stuart-marks thank you for review. How about this note in Record class? I
wrote it in a more general manner, without mentioning canonical constructors
explicitly. Is it enough?
-------------
PR: https://git.openjdk.java.net/jdk/pull/3556
