Ninject binding based on string

Question

I have a web service that's going to do things with some data being passed in (specifically InfoPath xml from a SharePoint doc library). I'm currently using Ninject to handle what form data "strategy" to load. Here's some code (question follows):

Web Service (Entry Point)

namespace Web.Services
{
    public bool AddForm(XmlDocument form, string formName)
    {
        IKernel kernel = new StandardKernel(new FormsModule());
        var ctx = kernel.Get<IPFormDataContext>(formName);

        return ctx.DoWork(form);
    }
}

Ninject Related Things

namespace Core.Modules
{
    public class FormsModule : NinjectModule
    {
        public override void Load()
        {
            Bind<IPFormDataContext>().ToSelf().Named("FormA");
            Bind<IPFormDataContext>().ToSelf().Named("FormB");
            // Snip

            Bind<IPFormDataStrategy>().To<FormAStratgey>()
                .WhenParentNamed("FormA");
            Bind<IPFormDataStrategy>().To<FormBStrategy>()
                .WhenParentNamed("FormB");
            // Snip
        }
    }
}

Pattern Related Things

namespace Core.Forms
{
    public class IPFormDataContext
    {
        private IPFormDataStrategy _ipFormDataStrategy;

        public IPFormDataContext(IPFormDataStrategy strategy)
        {
            _ipFormDataStrategy = strategy;
        }

        public bool DoWork(XmlDocument form)
        {
            return _ipFormDataStrategy.DoWork(form);
        }
    }

    public abstract class IPFormDataStrategy
    {
        public abstract bool DoWork(XmlDocument form);
    }
}

namespace Core.Forms.FormStrategies
{
    class FormAStrategy : IPFormDataStrategy
    {
        public override bool DoWork(XmlDocument form)
        {
            // Deserialize form using (xsd.exe generated) FormAData
            // and perform some operation on the resulting data.
            return resultOfWork;
        }
    }
}

FormBStrategy is much the same, as is the 7 other strategies I didn't list. I'm trying to find a way to pass in a form xml to the webservice and call the correct form deserialization based on the form type that's coming in.

The code above "works"; but it feels like I'm doing some sort of service location in Ninject, which from what I'm reading is a bad thing. But I can't think of a proper way to accomplish this. I'm not dead set on using Ninject, or any IOC/DI framework for that matter.

Is what I'm doing ... wrong? Could I get pointed in the right direction?

Answer 1

If the classes that you present in your example code are accurate (i.e. there is not a bunch more methods and properties). Then the simplest possible solution might work, and you can get rid of a number of classes / dependencies on classes.

A simple solution, that does not rely on a framework/container would be:

public static class FormsProcessing
{
    private static ConcurrentDictionary<string, Func<FormProcessor>> _registeredProcessors = new ConcurrentDictionary<string, Func<FormProcessor>>();

    public delegate bool FormProcessor(XmlDocument form);

    public static void RegisterProcessor(string formKey, Func<FormProcessor> formsProcessorFactory)
    {
        _registeredProcessors.AddOrUpdate(formKey, formsProcessorFactory, (k, current) => formsProcessorFactory);
    }

    public static FormProcessor GetProcessorFor(string formKey)
    {
        Func<FormProcessor> processorFactory;
        if (_registeredProcessors.TryGetValue(formKey, out processorFactory);
            return processorFactory();
        return null;
    }

    public static bool Process(string formKey, XmlDocument form)
    {
        var processor = GetProcessorFor(formKey);
        if (null == processor)
            throw new Exception(string.Format("No processor for '{0}' forms available", formKey));
        return processor(form);
    }
}

Usage:

namespace Web.Services
{
    public class MyServiceClass
    {
        public bool AddForm(XmlDocument form, string formName)
        {
            return FormsProcessing.Process(formName, form);
        }
    }
}

It is simple and explicit, and does not need or expose any dependency on some structure of IPFormDataContext and IPFormDataStrategy classes. The only explicit dependency you have is on a delegate that has the FormProcessor signature.

Similar to a container, you will need to perform the registrations somewhere:

FormsProcessing.RegisterProcessor("FormA", () => new FormAStrategy().DoWork);
FormsProcessing.RegisterProcessor("FormB", () => new FormBStrategy().DoWork);

Alternatively it would be easy to add some form of (convention based) auto registration by scanning assemblies for the convention (e.g. an interface signature).

Answer 2

Service Locator is generally an anti-pattern, but the important thing is to understand why it's an anti-pattern. The reasons are usually related to refactoring and type safety. I think the best way to determine if you are doing something wrong is to reduce the problem to it's absolute simplest requirements, and then judge the simplest path to get there.

As I understand, your requirements are:

1. Receive xml form data in a webservice
2. Based on the form type, call the appropriate logic

My further questions to you would be:

1. How do you identify the form type? Is it a field in the xml document? (if so, use that)
2. Are the form types likely to change often?

When it boils down to it, you have to use some sort of identifier. As @McGarnagle pointed out, magic strings can get out of sync with the code. You could use the Type name of the Strategy class, but it has the same 'might get out of sync' problem.

If the form types aren't likely to change, just use a switch statement and instantiate the Strategy instances yourself. Simplicity is the best design pattern to follow for maintainability.

If they are likely to change, Service Locator could work. If your Service Locator implementation is confined to just this code, it's not going to be that terrible to maintain.

And on Ninject, I'm not sure if this benchmark is still valid, but Funq is much faster, and I like the syntax better. (If you decide to use a DI container at all)

Answer 3

Two things I don't like:

1. Creating a kernel inside the AddForm method. This should never happen, as it inverts the IoC pattern -- instead, the class that AddForm belongs to should request any dependencies it needs.
2. The use of magic strings. It doesn't seem right to require the consumers of AddForm() to send a string naming the strategy.

I'm not quite sure how to resolve this; one thing that comes to mind is to add a Func<string, IPFormDataStrategy> dependency to the class that owns AddForm (call it class X). I'm imagining something like this:

namespace Web.Services
{
    public class X
    {
        private readonly Func<string, IPFormDataStrategy> _strategyResolver;

        public X(Func<string, IPFormDataStrategy> strategyResolver)
        {
            _strategyResolver = strategyResolver;
        }

        public bool AddForm(XmlDocument form, string formName)
        {
            return _strategyResolver(formName).DoWork(form);
        }
    }
}

Then you can use ToMethod to bind Func<string, IPFormDataStrategy>:

public class FormsModule : NinjectModule
{
    public override void Load()
    {
        Bind<FormAStrategy>().ToSelf();
        Bind<FormBStrategy>().ToSelf();

        Bind<Func<string, IPFormDataStrategy>>().ToMethod(context => 
            new Func<string, IPFormDataStrategy>(formName => {
                switch (formName)
                {
                    case "FormA": 
                        return context.Kernel.Get<FormAStrategy>();
                        // Note, could also simply "return new FormAStrategy()" here.
                    case "FormB": 
                        return context.Kernel.Get<FormBStrategy>();
                    default: 
                        throw new InvalidOperationException(formName + " is unrecognized");
                }
            })
        );
    }
}

You may find this needlessly complex, and maybe it is ... I like it because it makes class X's dependency explicit (that is, get a strategy given a form name), rather than giving it access to the entire kernel. This approach also consolidates the logic for getting a strategy into a single switch statement. It still relies on the magic strings, but I'm not sure how to get around that, without knowing more context ...