I'd like to use Dependency Injection (Castle Windsor) to replace some factory code I've inherited. Am I taking the correct approach?

This is what the current code looks like:

public static class WidgetFactory
{
   public static AbstractWidget CreateWidget(WidgetSpec spec)
   {
        if (spec.ModelNo == "FOO")
            return new FooWidget(spec);

        if (spec.ModelNo == "BAR")
            return new BarWidget(spec);

        if (spec.ModelNo == "BOO")
            return new BooWidget(spec);
   }
}

This is my implementation that uses DI:

app.config

<components>
  <component id="FOO" 
             service="MyCo.App.AbstractWidget" 
             type="MyCo.App.FooWidget, MyApp" 
             lifestyle="transient" />
   <component id="BAR" 
             service="MyCo.App.AbstractWidget" 
             type="MyCo.App.BarWidget, MyApp" 
             lifestyle="transient" />
    ....        

</components>

Code

static class WidgetFactory
{
    static IWindsorContainer _container = 
        new WindsorContainer(new XmlInterpreter(new ConfigResource("castle")));

    public static AbstractWidget CreateWidget(WidgetSpec spec)
    {
        return _container.Resolve<AbstractWidget>(spec.ModelNo, new { widgetSpec = spec });
    }
}

Is this the correct approach? What am I overlooking/doing wrong/misunderstanding? Should I create interfaces for the abstract classes and return them from the factory instead?

(I would prefer to stick to XML configuration for this particular application)

Edit:

Suggestion by Krzysztof Koźmic:

public interface IFactory
{
    AbstractFactory CreateWidget(WidgetSpec widgetSpec);
    void ReleaseWidget(AbstractFactory widget);
}

public class CustomTypedFactoryComponentSelector : DefaultTypedFactoryComponentSelector
{
    protected override string GetComponentName(MethodInfo method, object[] arguments)
    {
        WidgetSpec widgetSpec = arguments[0] as Widget开发者_JS百科Spec;
        if (method.Name == "CreateWidget" && arguments.Length == 1 && widgetSpec != null)
        {
            // The component mappings are stored as config settings
            // for the sake of example
            var componentName = Properties
                    .Settings
                    .Default
                    .Properties[widgetSpec.ModelNo]
                    .DefaultValue.ToString();

            return componentName;
        }

        return base.GetComponentName(method, arguments);
    }
}


container.AddFacility<TypedFactoryFacility>();
container.Register(Component.For<IFactory>().AsFactory(c => c.SelectedWith(new CustomTypedFactoryComponentSelector())));
//...
var factory = container.Resolve<IFactory>();
var widgetFactory = factory.CreateWidget(widgetSpec);

You could use Typed Factory with custom selector (see this post for an example and the documentation).

When answering questions about dependency injection here on SO, I almost always say: "use a factory". I think your solution looks pretty good ;-)

Perhaps there still is some room for improvement, though.

Because the factory is a static type, you have no choice than calling that directly from code. This makes it hard to test that code (if testability is a concern of course). What you might try is to inject the factory as a dependency in the types you are using. So instead of having a hard dependency on a static type, create a dependency on an interface. This could look like this:

public interface IWidgetFactory
{
    AbstractWidget CreateWidget(WidgetSpec spec);
}

internal class WidgetFactory : IWidgetFactory
{
   // code
}

Now you can easily register this type by its interface:

<component
    service="MyCo.App.IWidgetFactory, MyApp" 
    type="MyCo.App.WidgetFactory, MyApp" 
    lifestyle="singleton" />

Now you can request an IWidgetFactory from the container, or even better, inject the IWidgetFactory as constructor argument on the types that need to use it:

public class TypeUsingWidgets
{
    private IWidgetFactory widgetFactory;

    public TypeUsingWidgets(IWidgetFactory widgetFactory)
    {
        this.widgetFactory = widgetFactory;
    }

    public void MethodUsingWidgets()
    {
        var widget = this.factory.CreateWidget("Foo");
    }
}

Perhaps this is beneficial for your application.