On Apr 5, 2012, at 8:35 AM, Thomas Mortagne wrote: > On Wed, Apr 4, 2012 at 8:00 PM, Vincent Massol <[email protected]> wrote: >> Hi Thomas/All, >> >> All looks good except CM.lookupComponent() which Ï don't like a lot for the >> following reasons: >> * It's a bit long. It's longer than before and it would be nicer if it were >> as short as before or even shorter ;) >> * It's not symmetrical with other lookups like lookupList and lookupMap, >> which should theoretically be lookupComponentList() and lookupComponentMap() >> which are even longer >> >> Thus I'd prefer that the new API for lookup be named differently. >> >> I propose to use "resolve": >> >> CM.resolve(Type) >> CM.resolveList(Type) >> CM.resolveMap(Type) >> >> Alternative: >> >> CM.getInstance(Type) >> CM.getInstanceList(Type) >> CM.getInstanceMap(Type) >> >> or even the short: >> >> CM.get(Type) >> CM.getList(Type) >> CM.getMap(Type) >> >> For information Guice uses getInstance(), Picocontainer uses >> getComponentInstance(). >> >> WDYT? > > "get" sounds a bit lightweight to be for something supposed to also > initialize what you ask but if Guice use that I guess it means it's > OK. > I'm OK with any of theses suggestions so I will apply the voted one. > >> >> My preference goes to getInsta nce(): >> * users are tempted by default to start typing "get" to get an instance and >> then they'll see getInstance in autocompletion and choose it. >> * it's the same as Guice > > "getInstance" is longer than "lookup" ;)
Yes…. But I couldn't find a short one that would get everyone's agreement ;) It's still shorter than lookupComponent. There's also getComponent, getComponentList and getComponentMap which are not that bad but getInstance() seems to win everyone's opinnion so far and I think it's good that it's the same API than Guice. Thanks -Vincent >> Thanks >> -Vincent >> >> On Mar 6, 2012, at 2:51 PM, Thomas Mortagne wrote: >> >>> It's committed. >>> >>> Would be nice to have comments on the new APIs (suggest better naming, >>> etc...). It's a very important API and we really need to have >>> something we are sure of before 4.0 final. >>> >>> On Mon, Mar 5, 2012 at 6:53 PM, Thomas Mortagne >>> <[email protected]> wrote: >>>> Hi guys, >>>> >>>> I'm almost done but there is one little detail left: for easier >>>> retro-compatibility I would like to introduce a new @Role annotation >>>> and deprecate the @ComponentRole annotation. The idea is that Role >>>> will "officially" take into account generic parameters while >>>> ComponentRole does not. That way no existing component role behavior >>>> will be broken or require crappy retro-compatibility like registering >>>> automatically both generic and non generic roles (what I started to do >>>> at first). >>>> >>>> On Thu, Mar 1, 2012 at 6:20 PM, Thomas Mortagne >>>> <[email protected]> wrote: >>>>> Hi devs, >>>>> >>>>> I recently modified component manager internals and changed the role >>>>> used as key in the map from Class to Type so that it properly make a >>>>> difference between Provider<Toto> and Provider<Titi> without the need >>>>> to generate a special hint. >>>>> >>>>> For the one not aware of what Type is lets say it's what Class is >>>>> extending. Among other things Type is also extended by >>>>> ParameterizedType which is interesting here because it basically >>>>> contain a Class with it's generic parameters (it also indicate if the >>>>> type in included in another one but we don't care in our case ;)). >>>>> >>>>> To summarize: >>>>> * Provider is a Class >>>>> * Provider<String> is ParameterizedType >>>>> and both are Type >>>>> >>>>> That said I would like to change the whole ComponentManager and >>>>> related descriptors APIs to use Type as role instead of Class. >>>>> >>>>> = What it bring >>>>> >>>>> What it means is that it makes possible to cleanly represent a >>>>> Provider in which the generic type is meaningful right in the >>>>> ComponentDescriptor (right now the generic type is extract from the >>>>> implementation class when the provider is registered which is pretty >>>>> crappy) and simplify a lot the current implementation. >>>>> >>>>> But it also mean we could use this new "feature" for other components. >>>>> For example right now we have things like: >>>>> >>>>> @Component >>>>> public class DefaultStringDocumentReferenceResolver implements >>>>> DocumentReferenceResolver<String> >>>>> >>>>> and >>>>> >>>>> @Component >>>>> @Named("default/reference") >>>>> public class DefaultReferenceDocumentReferenceResolver implements >>>>> DocumentReferenceResolver<EntityReference> >>>>> >>>>> and injected by >>>>> >>>>> @Inject >>>>> DocumentReferenceResolver<String> resolver; >>>>> >>>>> and >>>>> >>>>> @Inject >>>>> @Named("default/reference") >>>>> DocumentReferenceResolver<EntityReference> resolver; >>>>> >>>>> Having Type used as role means that we could have: >>>>> >>>>> @Inject >>>>> DocumentReferenceResolver<String> resolver; >>>>> >>>>> and >>>>> >>>>> @Inject >>>>> DocumentReferenceResolver<EntityReference> resolver; >>>>> >>>>> = What it breaks >>>>> >>>>> So what's the catch ? >>>>> >>>>> Basically it would break any code that currently programmatically >>>>> register or unregister Provider since It would remove the current hack >>>>> to discover the Provider parameter (assuming that the >>>>> ComponentDescriptor now provide the complete role). But that's OK IMO >>>>> given that Provider support is very recent and I doubt it's a common >>>>> use case. >>>>> >>>>> We would obviously keep and deprecate all the existing APIs based on >>>>> Class which are pretty easy to implement since Class extends Type. >>>>> Even if removing them would not cause any issue when you build it's >>>>> not binary compatible since it's not the same method signature. >>>>> >>>>> So WDYT ? >>>>> >>>>> Here is my +1 and since we are starting a new major version it sounds >>>>> a good occasion so introduce such change. >>>>> >>>>> -- >>>>> Thomas Mortagne >>>> >>>> >>>> >>>> -- >>>> Thomas Mortagne >>> >>> >>> >>> -- >>> Thomas Mortagne >>> _______________________________________________ >>> devs mailing list >>> [email protected] >>> http://lists.xwiki.org/mailman/listinfo/devs >> >> _______________________________________________ >> devs mailing list >> [email protected] >> http://lists.xwiki.org/mailman/listinfo/devs > > > > -- > Thomas Mortagne > _______________________________________________ > devs mailing list > [email protected] > http://lists.xwiki.org/mailman/listinfo/devs _______________________________________________ devs mailing list [email protected] http://lists.xwiki.org/mailman/listinfo/devs
