I pretty much finished the implementation of the meta data API related to method level constraints for Hibernate Validator (see https://github.com/gunnarmorling/hibernate-validator/commits/HV-371).

The implementation conforms with what we currently discussed, but right now I'm wondering whether exposing java.lang.reflect.Method on the API is actually a good idea. I see two issues:

Therefore I would be interested in feedback on the following change:

public interface TypeDescriptor extends ElementDescriptor { MethodDescriptor getConstraintsForMethod(String name, Class<?>... parameterTypes); //instead of MethodDescriptor getConstraintsForMethod(Method method); }

public interface MethodDescriptor extends ElementDescriptor { String getName(); List<Class<?>> parameterTypes(); //instead of Method getMethod() }

Looks fine to me. I was concerned about passing Method around as well. On 1 mars 2011, at 00:29, Gunnar Morling wrote: Hi, I pretty much finished the implementation of the meta data API related to method level constraints for Hibernate Validator (see https://github.com/gunnarmorling/hibernate-validator/commits/HV-371). The implementation conforms with what we currently discussed, but right now I'm wondering whether exposing java.lang.reflect.Method on the API is actually a good idea. I see two issues: * Retrieving method objects via the reflection APIs is somewhat nasty, in particular it requires to handle a checked NoSuchMethodException: try { Method bar = Foo.class.getDeclaredMethod( "bar", String.class ); } catch(Exception e) { throw new RuntimeException(e); } * The handling in inheritance hierarchies with overridden/implemented methods can be confusing to users not overly familiar with the reflection API. In particular there are different method objects for a base method and its implementation/overriding methods. This can be irritating when invoking MethodDescriptor#getMethod() for instance. Therefore I would be interested in feedback on the following change: public interface TypeDescriptor extends ElementDescriptor { MethodDescriptor getConstraintsForMethod(String name, Class<?>... parameterTypes); //instead of MethodDescriptor getConstraintsForMethod(Method method); ... } public interface MethodDescriptor extends ElementDescriptor { String getName(); List<Class<?>> parameterTypes(); //instead of Method getMethod() ... } WDYT? Thanks, Gunnar

Alright, I'll change this then as proposed. 2011/3/1 Emmanuel Bernard <emmanuel(a)hibernate.org&gt; Looks fine to me. I was concerned about passing Method around as well. > > On 1 mars 2011, at 00:29, Gunnar Morling wrote: > > Hi, > > I pretty much finished the implementation of the meta data API related to > method level constraints for Hibernate Validator (see > https://github.com/gunnarmorling/hibernate-validator/commits/HV-371). The > implementation conforms with what we currently discussed, but right now I'm > wondering whether exposing java.lang.reflect.Method on the API is actually a > good idea. I see two issues: > > * Retrieving method objects via the reflection APIs is somewhat nasty, in > particular it requires to handle a checked NoSuchMethodException: > > try { > Method bar = Foo.class.getDeclaredMethod( "bar", String.class ); > } > catch(Exception e) { > throw new RuntimeException(e); > } > > * The handling in inheritance hierarchies with overridden/implemented > methods can be confusing to users not overly familiar with the reflection > API. In particular there are different method objects for a base method and > its implementation/overriding methods. This can be irritating when invoking > MethodDescriptor#getMethod() for instance. > > Therefore I would be interested in feedback on the following change: > > public interface TypeDescriptor extends ElementDescriptor { > > MethodDescriptor getConstraintsForMethod(String name, Class<?>... > parameterTypes); > //instead of MethodDescriptor getConstraintsForMethod(Method method); > ... > } > > public interface MethodDescriptor extends ElementDescriptor { > > String getName(); > > List<Class<?>> parameterTypes(); > //instead of Method getMethod() > ... > } > > WDYT? > > Thanks, Gunnar > > >