Factory support seems buggy when using an abstract factory

[George Mauer]

Perhaps this is just a knowledge limitation but here is something that
I noticed.
It seems that to instantiate a class with the factory facility it is
necessary to provide a concrete type. So if I have the example (to
borrow from http://bugsquash.blogspot.com/2008/12/castle-windsor-factory-method-support.html)
public interface IUserRepository { }
public class UserRepositoryImplementation : IUserRepository { }
public class UserRepositoryFactory {
public IUserRepository Create() {
return new UserRepositoryImplementation();
}
}


then the following passes:
[Test]
public void Resolve_from_factory() {
var container = new WindsorContainer();
container.AddFacility("factory.support", new FactorySupportFacility
());
container.AddComponent<UserRepositoryFactory>("userRepo.factory");

container.Register(
Component.For<IUserRepository>()
.ImplementedBy<UserRepositoryImplementation>()
.Attribute("factoryId").Eq("userRepo.factory")
.Attribute("factoryCreate").Eq("Create")
);
Assert.IsNotNull(container.Resolve<IUserRepository>());
}

Great! But I had to know ahead of time the specific implementation
the factory method would provide, even though the factory method
itself outputs an interface. So what happens if we're using an
abstract factory?
let's change the code as follows but keep the test the same:
public class UserRepositoryImplementation2 : IUserRepository { }
public class UserRepositoryFactory {
public IUserRepository Create() {
return new UserRepositoryImplementation();
}
}

Still works! So not only is the
".ImplementedBy<UserRepositoryImplementation>()" line unnecessary, it
is downright misleading since we are now resolving a completely
different class.

Any thoughts?

[Craig Neuwirt]

So mayne throw a configuration exception when a factory is configured as well as an implementation type?

[George Mauer]

I'm sorry, I don't understand what you mean. I don't think that an
exception needs to be thrown. I think the ImplementedBy clause (type=
in xml) should not be necessary if you're using a factory method since
the type is already stated in the create method.

On Jan 22, 7:50 pm, Craig Neuwirt <cneuw...@gmail.com> wrote:
> So mayne throw a configuration exception when a factory is configured as
> well as an implementation type?
>
> On Thu, Jan 22, 2009 at 5:00 PM, George Mauer <gma...@gmail.com> wrote:
>
> > Perhaps this is just a knowledge limitation but here is something that
> > I noticed.
> > It seems that to instantiate a class with the factory facility it is
> > necessary to provide a concrete type.  So if I have the example (to
> > borrow from

> >http://bugsquash.blogspot.com/2008/12/castle-windsor-factory-method-s...
> > )

[mausch]

I'd rather leave it to the developer (as it is now) to specify the
type instead of letting the container infer it automatically, as it is
more flexible. When I define type="IUserRepository", not only does the
container register that interface but also the factory implementation
could return anything that implements IUserRepository. If I didn't
have to specify the type and the factory returned UserRepository (the
impl), the container would register UserRepository, not
IUserRepository. Then the container wouldn't be able to resolve any
component that depended on IUserRepository.

I hope I understood your point right.

[Craig Neuwirt]

I was just thinking that it doesn't make sense to specify and ImplementBy<> if you are configured to use a factory.  Its an ambiguous configuration which should probably throw an exception just like an exception is thrown if you use an ImplementedBy<> with an abstract class.

[George Mauer]

What you are saying doesn't allow for an abstract factory.
public interface ICarProvider { }
public class FerrariProvider : ICarProvider {}
public class HondaProvider : ICarProvider {}

public class AbstractCarProviderFactory {
public ICarProvider Create(User currentUser) {
if(currentUser.FiscalStability == FiscalStability.MrMoneyBags)
return new FerrariProvider();
else
return new HondaProvider();
}
}

It should be possible to register this as
container.Register(
Component.For<ICarProvider>().
.Attribute("factoryId").Eq("ICarProviderFactory")

.Attribute("factoryCreate").Eq("Create")
);

However you cannot do this currently, you have to add
an .ImplementedBy<> block even though it makes no sense in this
context at all!

On Jan 23, 7:03 am, Craig Neuwirt <cneuw...@gmail.com> wrote:
> I was just thinking that it doesn't make sense to specify and ImplementBy<>
> if you are configured to use a factory.  Its an ambiguous configuration
> which should probably throw an exception just like an exception is thrown if
> you use an ImplementedBy<> with an abstract class.
>

[Craig Neuwirt]

I think we are on the same page, but can you send me an actual testcase that I can compile and run to be certain I understand what you want.  I expect the tests to fail.

thanks,
  craig

[George Mauer]

Sure thing, here you go: http://pastebin.com/f32d19455

I would further argue that even if using a concrete factory an
ImplementedBy<> clause is still redundant since it is specified by the
factory method signature

[Craig Neuwirt]

Thanks, I'll review later today or this weekend.

[Craig Neuwirt]

This is now fixed on trunk.  Thanks for bringing it to our attention,

craig