On Sat, 17 Apr 2021 08:55:59 GMT, Tagir F. Valeev <tval...@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. ------------- PR: https://git.openjdk.java.net/jdk/pull/3556