You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geronimo.apache.org by Gianny Damour <gi...@optusnet.com.au> on 2008/04/01 10:18:26 UTC

Re: Heads-up - GBeanInfo via annotations

On 01/04/2008, at 7:52 AM, David Blevins wrote:

>
> On Mar 31, 2008, at 5:37 AM, Gianny Damour wrote:
>>> From: David Jencks <da...@yahoo.com>
>>>
>>> 1. I think we should try harder to separate the identification of  
>>> a constructor parameter as attribute or reference from  
>>> identifying its name.  So, I think we should use the java6 and  
>>> xbean @ParameterName annotations for names and something else for  
>>> references if necessary.
>>
>> It seems to me that xbean only defines a @ParameterNames  
>> annotation, which I think is quite efficient only when the number  
>> of parameters is small. The need of a @ParameterName annotation,  
>> targeting a parameter and not a constructor or method, really  
>> emerges when the number of parameters increases.
>>
>> If you want, I can add a @ParameterName annotation to xbean- 
>> reflect instead of adding it to geronimo-kernel.
>
>
> Annotation-wise an @ParameterName annotation would only be useful  
> if there was an @ParameterNames annotation to act as a wrapper  
> (can't have more than one annotation of the same name on a  
> target).  We already have an @ParameterNames which accepts String[]  
> so that's sort of a dead end.

I do not understand. @ParameterNames' target is either a constructor  
or a method. The @ParameterName I am referring to targets a  
parameter. It does not mandate another annotation acting as a wrapper  
to be useful. What is exactly the dead end you are hinting at?

>
> The "extra metadata beyond a name" aspect of constructor and  
> factory method injection is certainly a tricky one.  If you can get  
> by without needing anything beyond a string, that certainly makes  
> it easier.  In EJB3 we're sort of hosed in that regard as we have  
> @Resource and @EJB, which is nearly identical to the proposed  
> @Attribute/@Reference concept; having no polymorphism in  
> annotations makes it impossible to support both in a list.  If we  
> could avoid needing the distinction, there are advantages.
>
> If we do feel the need to have the distinction, I'd strongly  
> recommend "reference" being a boolean type on @Attribute or some  
> similar convention so there's only one annotation type which could  
> be used in a list.  From there "rich" constructor or factory method  
> support could be as simple as @Attributes(Attribute[]), which I'd  
> recommend be supportable on a class, constructor or method.

This idea of @Attributes(Attribute[]) or @ParameterNames may or may  
not be desirable. The more parameters, the less desirable it is (this  
is the point I tried to make). Let's consider how  
AppClientModuleBuilder would be annotated with @ParameterNames:

     @ParameterNames(names={"transactionManagerObjectName",
                 "connectionTrackerObjectName",
                 "corbaGBeanObjectName",
                 "credentialStoreName",
                 "Repositories",
                 "ConnectorModuleBuilder",
                 "ServiceBuilders",
                 "NamingBuilders",
                 "ModuleBuilderExtensions",
                 "ClientArtifactResolver",
                 "defaultClientEnvironment",
                 "defaultServerEnvironment"})
     public AppClientModuleBuilder(AbstractNameQuery  
transactionManagerObjectName,
                                   AbstractNameQuery  
connectionTrackerObjectName,
                                   AbstractNameQuery  
corbaGBeanObjectName,
                                   AbstractNameQuery  
credentialStoreName,
                                   Collection<Repository> repositories,
                                   Collection<ModuleBuilder>  
connectorModuleBuilder,
                                   Collection<NamespaceDrivenBuilder>  
serviceBuilder,
                                   Collection<NamingBuilder>  
namingBuilders,
                                   Collection<ModuleBuilderExtension>  
moduleBuilderExtensions,
                                   ArtifactResolver  
clientArtifactResolver,
                                   Environment defaultClientEnvironment,
                                   Environment  
defaultServerEnvironment) {

Inlining a @ParameterName is more readable and maintainable:

    public AppClientModuleBuilder(@ParameterName 
(name="transactionManagerObjectName") AbstractNameQuery  
transactionManagerObjectName,
                                   @ParameterName 
(name="connectionTrackerObjectName") AbstractNameQuery  
connectionTrackerObjectName,
                                   @ParameterName 
(name="corbaGBeanObjectName") AbstractNameQuery corbaGBeanObjectName,
                                   @ParameterName 
(name="credentialStoreName") AbstractNameQuery credentialStoreName,
                                   @ParameterName 
(name="Repositories") Collection<Repository> repositories,
                                   @ParameterName 
(name="ConnectorModuleBuilder") Collection<ModuleBuilder>  
connectorModuleBuilder,
                                   @ParameterName 
(name="ServiceBuilders") Collection<NamespaceDrivenBuilder>  
serviceBuilder,
                                   @ParameterName 
(name="NamingBuilders") Collection<NamingBuilder> namingBuilders,
                                   @ParameterName 
(name="ModuleBuilderExtensions") Collection<ModuleBuilderExtension>  
moduleBuilderExtensions,
                                   @ParameterName 
(name="ClientArtifactResolver") ArtifactResolver clientArtifactResolver,
                                   @ParameterName 
(name="defaultClientEnvironment") Environment defaultClientEnvironment,
                                   @ParameterName 
(name="defaultServerEnvironment") Environment  
defaultServerEnvironment) {{

As a matter of fact, I believe the JavaBean annotation  
ConstructorProperties should have been complemented with another one  
targeting parameters.

>
> The motivation for supporting it on a class is that there could be  
> several constructors you might want usable, in which case you might  
> be motivated to have all the metadata in one spot.  Ideally, if you  
> don't need to explicitly add an annotation for your plain  
> attributes and only require the annotation when some extra metadata  
> is required, you'd only need two or three @Attribute annotations on  
> a class.

The motivation to have all the metadata located at one spot may or  
may not be desirable. As per the previous example, I believe that  
putting annotations at the right place is more readable and  
maintainable. As a matter of fact, I am not a fan of the static block  
approach we are using to construct GBeanInfos as these blocks are too  
far from the code they are describing even if all the metadata is in  
one spot.

Based on David J's feedback, I think I have now better defaults and a  
better annotation by exception style.

Thanks for the feedback!
Gianny

>
> -David
>
>


Re: Heads-up - GBeanInfo via annotations

Posted by David Blevins <da...@visi.com>.
On Apr 1, 2008, at 1:18 AM, Gianny Damour wrote:
>
> On 01/04/2008, at 7:52 AM, David Blevins wrote:
>
>>
>> On Mar 31, 2008, at 5:37 AM, Gianny Damour wrote:
>>>> From: David Jencks <da...@yahoo.com>
>>>>
>>>> 1. I think we should try harder to separate the identification of  
>>>> a constructor parameter as attribute or reference from  
>>>> identifying its name.  So, I think we should use the java6 and  
>>>> xbean @ParameterName annotations for names and something else for  
>>>> references if necessary.
>>>
>>> It seems to me that xbean only defines a @ParameterNames  
>>> annotation, which I think is quite efficient only when the number  
>>> of parameters is small. The need of a @ParameterName annotation,  
>>> targeting a parameter and not a constructor or method, really  
>>> emerges when the number of parameters increases.
>>>
>>> If you want, I can add a @ParameterName annotation to xbean- 
>>> reflect instead of adding it to geronimo-kernel.
>>
>>
>> Annotation-wise an @ParameterName annotation would only be useful  
>> if there was an @ParameterNames annotation to act as a wrapper  
>> (can't have more than one annotation of the same name on a  
>> target).  We already have an @ParameterNames which accepts String[]  
>> so that's sort of a dead end.
>
> I do not understand. @ParameterNames' target is either a constructor  
> or a method. The @ParameterName I am referring to targets a  
> parameter. It does not mandate another annotation acting as a  
> wrapper to be useful. What is exactly the dead end you are hinting at?

I completely forgot about the MyConstructor(@MyAnnotation String  
myString) possibility and was thinking about the "can't have more than  
one annotation on a target" limitation.

>> The "extra metadata beyond a name" aspect of constructor and  
>> factory method injection is certainly a tricky one.  If you can get  
>> by without needing anything beyond a string, that certainly makes  
>> it easier.  In EJB3 we're sort of hosed in that regard as we have  
>> @Resource and @EJB, which is nearly identical to the proposed  
>> @Attribute/@Reference concept; having no polymorphism in  
>> annotations makes it impossible to support both in a list.  If we  
>> could avoid needing the distinction, there are advantages.
>>
>> If we do feel the need to have the distinction, I'd strongly  
>> recommend "reference" being a boolean type on @Attribute or some  
>> similar convention so there's only one annotation type which could  
>> be used in a list.  From there "rich" constructor or factory method  
>> support could be as simple as @Attributes(Attribute[]), which I'd  
>> recommend be supportable on a class, constructor or method.
>
> This idea of @Attributes(Attribute[]) or @ParameterNames may or may  
> not be desirable. The more parameters, the less desirable it is  
> (this is the point I tried to make). Let's consider how  
> AppClientModuleBuilder would be annotated with @ParameterNames:

The inlined approach is definitely the nicer approach.  I totally  
forgot about the ability to annotate individual parameters in the  
method signature.

There's a sneaky way to get the param names from the debug info in the  
class (code is in xbean).  That should allow us to extend the  
annotation by exception approach to constructors and factory methods  
(if we support them some day).  I'd hope we then use the same  
annotation for fields methods or constructor/factoryMethod params.

>> The motivation for supporting it on a class is that there could be  
>> several constructors you might want usable, in which case you might  
>> be motivated to have all the metadata in one spot.  Ideally, if you  
>> don't need to explicitly add an annotation for your plain  
>> attributes and only require the annotation when some extra metadata  
>> is required, you'd only need two or three @Attribute annotations on  
>> a class.
>
> The motivation to have all the metadata located at one spot may or  
> may not be desirable. As per the previous example, I believe that  
> putting annotations at the right place is more readable and  
> maintainable. As a matter of fact, I am not a fan of the static  
> block approach we are using to construct GBeanInfos as these blocks  
> are too far from the code they are describing even if all the  
> metadata is in one spot.

I don't care for that approach myself either, just throwing it out  
there as something that'd be easy to support and give some style  
options.  I can see it being a nice option in the configure by  
exception mindset where there might be just one annotation you need to  
supply and it possibly has some very long and detailed attributes that  
might be annoying to have in a constructor (or to repeat if there's  
more than one constructor).

-David