Hi Peter,

On 10/25/2013 05:28 AM, Peter Levart wrote:
On 10/25/2013 12:29 PM, Peter Levart wrote:

Hi Joe,

So, the order must be respected, right.

There's a bug. I think you wanted to call:

((Class) this).getSuperclass()

Instead of:

thisClass.getSuperclass()

...which would always return Object.class...


Well, this is not the only change needed, of course. What about something like the following (prefer iteration to recursion and use AnnotationType.isInherited() which is faster):


default <T extends Annotation> T[] getAnnotationsByType(Class<T> annotationClass) {
        T[] result = getDeclaredAnnotationsByType(annotationClass);
        if (result.length == 0 && this instanceof Class &&
AnnotationType.getInstance(annotationClass).isInherited()) {
            for (Class<?> clazz = ((Class<?>) this).getSuperclass();
                 clazz != null; clazz = clazz.getSuperclass()) {
result = clazz.getDeclaredAnnotationsByType(annotationClass);
                if (result.length > 0) {
                    break;
                }
            }
        }
        return result;
    }


Mulling over your suggested code, I've come up with

default <T extends Annotation> T[] getAnnotationsByType(Class<T> annotationClass) {
        /*
         * Definition of associated: directly or indirectly present OR
         * neither directly nor indirectly present AND the element is
         * a Class, the annotation type is inheritable, and the
         * annotation type is associated with the superclass of the
         * element.
         */
        T[] result = getDeclaredAnnotationsByType(annotationClass);

if (result.length == 0 && // Neither directly nor indirectly present
            this instanceof Class && // the element is a class
AnnotationType.getInstance(annotationClass).isInherited()) { // Inheritable
            Class<?> clazz = ((Class<?>) this).getSuperclass();
            if (clazz != null) {
                // Determine if the annotation is associated with the
                // superclass
                result = clazz.getAnnotationsByType(annotationClass);
            }
        }

        return result;
    }

This version has the structure of being "obviously correct" since the sequence of checks in the code closely follows the definition of associated used in AnnotatedElement. While the code you have to directly climb the superclass chain should be correct, I'd prefer to simply delegate to whatever getAnnotationsByType implementation is being used by java.lang.Class (which would not necessarily be this one).

In terms of stack overflow concerns, if this implementation were used by java.lang.Class, the stack size is small and should easily be able to accomodate even very long superclass chains dozens of levels deep.

Thanks for the feedback,

-Joe

Reply via email to