ClassRegistrationPicoContainer should maybe not extend PicoContainer

Aslak,

I opened Pico-16 just before you to deal with this very issue I will close 16 and move comments to this one.

The code attached extracts the concern of component registration out into an interface - "ComponentRegistrar":

public interface ComponentRegistrar

So its now possible to chain ComponentRegistrar impls to add
behaviour to component registration. Example: provide a logging instance to constructors without having to worry about logging in configuration files etc. Can be done like this:

final InputSourceComponentRegistrar registrar =
new InputSourceComponentRegistrarImpl(
new StringBasedComponentRegistrarImpl(
new LogEnablingComponentRegistrar(
new DefaultComponentRegistrarImpl())
));

final ExperimentalPicoContainer container =
new ExperimentalPicoContainer(
new DefaultComponentFactory(),registrar);

registrar.registerComponents(new InputSource(
new StringReader(xml)));

LogEnablingComponentRegistrar does this:

public void registerComponent(Class type, Class impl) {
final Class firstParameterType =
impl.getConstructors()[0].getParameterTypes()[0];
if (firstParameterType == Log.class)

delegate.registerComponent(type,impl);
}

So this allows me to configure this component:

public LoggableMockComponentImpl(Log log, int port)

with this XML:

"<components>" +
" <component class=\"LoggableMockComponentImpl\">" +
" <param type=\"java.lang.Integer\">12345</param>" +
" </component>" +
"</components>";.

There is no mention of logging, since the logging dependency is handled centrally by the decorator.

ExperimentalPicoContainer is copy of HierarchicalPicoContainer but with all the registration stuff delegated to a ComponentRegistrar instance.

Few things about this:

• It means we can insert all the check methods (like checkConcrete() etc) into the decorator chain instead of hard coding them into a default impl, if we want to. Additionally, PicoContainer implementers now have a means of inserting their own check methods that we didn't think of.
• It keeps ExperimentalPicoContainer small and focussed (I did not clean it up as much as possible in the attached code).
• If the number of potential instances of ComponentRegistrar is considered too large, we can provide classes that bundle them into sensible "families".

I agree that ClassRegistrationPicoContainer should be renamed to ComponentRegistrar and not extend PicoContainer, mainly because composition is more flexible than inheritance.

But it's a bit unclear to me the propsed chainging could work. I assume the "specialised" String/InputSource/Whatever ComponentRegistrars will be implementations of ComponentRegistrar that add extra methods (taking String, InputSource etc) as arguments.

These extra methods are not exposed via the interface. Having taken a look at the code from PICO-16 I see that chaining is not generic. Some implementations (like the InputSourceRegistrar) can only chain to a StringBasedComponentRegistrar.

I think there are some potentially dangerous combinations that can be made here, and to avoid them, I think we should also not let the StringBased and InputSource ComponentRegistrars extend from ComponentRegistrar, but be standalone interfaces.

Thoughts?

Aslak

Aslak,

> But it's a bit unclear to me the propsed chainging could work. I assume the
> "specialised" String/InputSource/Whatever ComponentRegistrars will be
> implementations of ComponentRegistrar that add extra methods (taking
> String, InputSource etc) as arguments.
>
> These extra methods are not exposed via the interface.

I don't understand - StringBasedComponentRegistrar is an interface that extends ComponentRegistrar and adds

public void registerComponent(String typeName, String implName);

> Having taken a look
> at the code from PICO-16 I see that chaining is not generic. Some
> implementations (like the InputSourceRegistrar) can only chain to a
> StringBasedComponentRegistrar.

Right - but thats an implementation thing, not an interface thing.

> I think there are some potentially dangerous combinations that can be made
> here, and to avoid them, I think we should also not let the StringBased
> and InputSource ComponentRegistrars extend from ComponentRegistrar, but be
> standalone interfaces.

I think there is no danger in letting the String/InputSource interfaces extend ComponentRegistrar. I think we should do it. I may have made things look dangerous by implementing the String/InputSource interfaces the way I did, making the chaining look arbitrary.

Are we talking past each other?

Cheers,
Mike.

Paul, Jon and Aslak had a long discussion about this. The introduction of the ComponentRegistry actually implements this. The RegistrationPicoContainer interface now merely acts as a conveinent facade to ComponentRegistry (at least in the DefaultPicoContainer, and some other classes).

We therefore believe this can be closed (partly as implemented elsewhere, partly as won't fix).