How I can to avoid IoC locator in next case?

113 views Asked by At

In brief: I want resolve interface by prop in entity. I have self hosted wcf and use ninject for DI. My working code for example:

//program.cs
...
private static StandardKernel CreateKernel()
{
    var kernel = new StandardKernel();

    kernel.Bind<IDbConnectionFactory>().ToMethod(c =>
        new OrmLiteConnectionFactory(
            conString, 
            SqlServerDialect.Provider))
        .InSingletonScope();
        
    kernel.Bind<IControllerProccessor>().To<ControllerProccessor>()
        .WhenInjectedInto<HelloWorldService>().WithConstructorArgument(kernel);
        
    kernel.Bind<IControllerProccessor>().To<Vendor1Proccessor>()
        .Named("vendor1"); 
        
    kernel.Bind<IControllerProccessor>().To<Vendor2Proccessor>()
        .Named("vendor2"); 

    return kernel;
}
...

//IControllerProccessor.cs
public interface IControllerProccessor
{
    SimpleController Ctr { get; set; }
    bool sendMsg(string msg);
}

//Vendor1Proccessor.cs
public class Vendor1Proccessor : IControllerProccessor
{
    public SimpleController Ctr {get; set;}

    public bool sendMsg(string msg)
    {
        //specific to vendor code, for example calls to vendor1 SDK
        Console.WriteLine("Controller id: {0} vendor:{1} recivied msg: {2}",
            Ctr.Id,
            "Vendor1Class",
            msg);
        return true;
    }
}

//Vendor2Proccessor.cs
public class Vendor2Proccessor : IControllerProccessor
{
    public SimpleController Ctr { get; set; }

    public bool sendMsg(string msg)
    {
        //specific to vendor code, for example calls to vendor1 SDK
        Console.WriteLine("Controller id: {0} vendor:{1} recivied msg: {2}",
            Ctr.Id,
            "Vendor2Class",
            msg);
        return true;
    }
}

//ControllerProccessor.cs
public class ControllerProccessor : IControllerProccessor
{
    public SimpleController Ctr {get; set;}

    private readonly IKernel kernel;

    public ControllerProccessor(IKernel _kernel)
    {
        kernel = _kernel;
    }

    public bool sendMsg(string msg)
    {
        var param = new Ninject.Parameters.PropertyValue("Ctr", Ctr);
        return kernel.Get<IControllerProccessor>(Ctr.Vendor, param).sendMsg(msg);
    }
}

//HelloWorldService.cs
public class HelloWorldService : IHelloWorldService
{
    private readonly IDbConnectionFactory dbFactory;
    private readonly IControllerProccessor ctrProccessor;

    public HelloWorldService(IDbConnectionFactory _dbFactory, IControllerProccessor _ctrProccesor)
    {
        dbFactory = _dbFactory;
        ctrProccessor = _ctrProccesor;
    }

    public bool sendMsgToAllControllers(string msg)
    {
        var db = dbFactory.Open();
        var controllers = db.Select<SimpleController>();

        foreach(var ctr in controllers)
        {
            ctrProccessor.Ctr = ctr;
            ctrProccessor.sendMsg(msg);
        }
        db.Close();

        return true;
    }

}

//SimpleController.cs
[DataContract]
[Alias("SimpleController")]
public class SimpleController
{
    [AutoIncrement]
    [DataMember]
    public int? Id { get; set; }
    [DataMember]
    public string Vendor { get; set; }
}

When I call sendMsgToAllControllers("TEST_MESSAGE") console output:

Controller id: 2 vendor:Vendor1Class recivied msg: TEST MESSAGE
Controller id: 3 vendor:Vendor2Class recivied msg: TEST MESSAGE
Controller id: 4 vendor:Vendor2Class recivied msg: TEST MESSAGE

How I can refactor above implementation so that it was in DI style, and dont use IoC locator anti-pattern (or in my case this is not anti-pattern) ?

In future I will move implementations (vendor1, vendor2, etc..) in separate assembly and do runtime binding. (Here I want to plugin system)

I also would appreciate any suggestions to improve my code. Very thanks.

Modification of implementation

After thinking process I came to the following: I remove ControllerProccessor class instead I create ControllerProcessorFactory:

public class ControllerProcessorFactory : IControllerProcessorFactory
{        
    private readonly IResolutionRoot resolutionRoot;

    public ControllerProcessorFactory(IResolutionRoot _resolutionRoot)
    {
        resolutionRoot = _resolutionRoot;
    }

    public IControllerProcessor Create(SimpleController ctr)
    {
        IControllerProcessor processor = resolutionRoot.Get<IControllerProcessor>(ctr.Vendor);
        processor.Ctr = ctr;
        return processor;
    }
}

In Bindings:

kernel.Bind<IControllerProcessorFactory>().To<ControllerProcessorFactory>();
kernel.Bind<IControllerProcessor>().To<Vendor1Processor>()
    .Named("vendor1");
kernel.Bind<IControllerProcessor>().To<Vendor2Processor>()
    .Named("vendor2"); 

Usage (wcf class):

public class HelloWorldService : IHelloWorldService
{
    private readonly IDbConnectionFactory dbFactory;
    private readonly IControllerProcessorFactory ctrProcessorFactory;

    public HelloWorldService(IDbConnectionFactory _dbFactory, IControllerProcessorFactory _ctrProcFactory)
    {
        dbFactory = _dbFactory;
        ctrProcessorFactory = _ctrProcFactory;
    }

    public bool sendMsgToAllControllers(string msg)
    {
        var db = dbFactory.Open();
        var controllers = db.Select<SimpleController>();

        foreach(var ctr in controllers)
        {
            var ctrProcessor = ctrProcessorFactory.Create(ctr);
            ctrProcessor.sendMsg(msg);
        }
        db.Close();

        return true;
    }
}
2

There are 2 answers

4
BatteryBackupUnit On BEST ANSWER

kernel.Get<IControllerProccessor>(Ctr.Vendor, param) is service locator, but then again that doesn't always mean "it" is a problem. If it's easily interchangable, then it's not a big deal (well at least that's the opinion of some). Easily interchangeable? Create a specific factory interface whose only responsibility is to return all the processors. The implementation then would consist of exactly return kernel.Get<IControllerProccessor>(Ctr.Vendor, param);. As long as the implementation is part of the composition root this specfici dependency on ninject is ok.

Shorter Alternative

Now, to be honest, your design looks superfluosly complicated to me, but then again, i don't know all the details. So for now i'm just using the name parameter, but you can easily add parameters (almost) as you like, it'll still work:

Just inject a Func<string, IControllerProcessor> instead of the kernel:

public ControllerProccessor(
     Func<string,IControllerProcessor>> controllerProcessorFactory)

You can then specfiy a binding as follows:

private static IControllerProcessor CreateSpecificControllerProcessor(
    IResolutionRoot resolutionRoot, string vendorName)
{
    return resolutionRoot.Get<IControllerProcessor>(vendorName);
}

Bind<Func<IControllerProcessor>()
    .ToConstant(vendorName => CreateSpecficiControllerProcessor(this.Kernel, vendorName));

Instead of specifying the Binding for the Func it might be possible to use Ninject.Extensions.Factory. Note however, that when you use this extension it won't be possible to Bind any Func manually anymore (the binding will be overriden by the extensions generation mechanism).

1
ewassef On

Look at Dynamic module loading in their documentation. i.e.

    kernel.Load("*.dll");

But make sure you do this at startup so you don't overwhelm your system at runtime. As far as your pattern, I would recommend using the GetAll() method on the kernel as that will give you more flexibility and control

    IEnumerable<IControllerProccessor> processors = kernel.GetAll<IControllerProccessor>();
    foreach(var processor in processors)
        processor.sendMsg(....);