Hi Joe, Peter,
On 29 okt 2013, at 07:09, Joe Darcy <[email protected]> wrote:
>
> Your comments, along with some spec refinements from the other OpenJDK list,
> are reflected in the next iteration of the webrev:
>
> http://cr.openjdk.java.net/~darcy/8005294.5/
>
> Thanks,
>
259 if (result.length == 0 && // Neither directly nor indirectly
present
260 this instanceof Class && // the element is a class
261 AnnotationType.getInstance(annotationClass).isInherited()) {
// Inheritable
262 ...
j.l.Class is final and has an implementation of getAnnotationsByType so
everything in this branch is dead code. I understand you might want to document
how the lookup is supposed to happen if you have inheritance semantics for your
AnnotatedElement but IMHO this isn't the way to do it. Just delegate to
getDeclaredAnnotationsByType.
291 * @since 1.8
292 */
293 default <T extends Annotation> T getDeclaredAnnotation(Class<T>
annotationClass) {
I keep forgetting that this one is new to 8. My reservations about making this
a default was based on (the misconception) that this was a 5 method. This
method looks good.
For getDeclaredAnnotationsByType() the idea of having two implementations of
the same logic, one taking arrays as arguments and the other Maps, seems wrong.
I would strongly prefer if we only had one implementation, calling into
AnnotationSupport from this method. Something like
default <T extends Annotation> T[] getDeclaredAnnotationsByType(Class<T>
annotationClass) {
Objects.requireNonNull(annotationClass);
return
AnnotationSupport.getDirectlyAndIndirectlyPresent(Arrays.stream(getDeclaredAnnotations()).collect(Collectors.toMap(
(a -> a.getClass()),
Function.identity())),
annotationClass);
}
I can't see these default methods as the code we should be spending time
optimizing.
cheers
/Joel
