Is it a good practice to pass the Ninject kernel around?

Question

I'm writing a small framework that executed a couple of tasks. Some om the tasks needs specific properties which are injected through Ninject.

Let's say that I have the following constructor in my base class that represents a single Task:

protected DDPSchedulerTask(ILogger logger, List<string> platforms, IBackOfficeDataStore backOfficeDataStore, ICommonDataStore commonDataStore)
{
    _logger = logger;
    _platforms = platforms;
    _backOfficeDataStore = backOfficeDataStore;
    _commonDataStore = commonDataStore;
}

Those properties are needed for all the tasks, so I inject them using Ninject with the following Ninject module.

public class DDPDependencyInjectionBindingConfiguration : NinjectModule
{
    #region NinjectModule Members

    /// <summary>
    ///     Loads the module into the kernel.
    /// </summary>
    public override void Load()
    {
        Bind<Scheduler>().ToSelf(); // Make sure that the 'Scheduler' is resolved to itself.
        Bind<ILogger>().ToMethod(context => LogFactory.Create()); // Make sure that an instance of an ILogger is created through the LogFactory.

        // Straightforward binding.
        Bind<ICommonDataStore>().To<Common>();
        Bind<IBackOfficeDataStore>().To<BtDbInteract>();
        Bind<IDirectoryResolver>().To<Demo>();
    }

    #endregion
}

My scheduler object itself if the first entry in the chain which needs to be resolved by Ninject, so I'm resolving this manually through Ninject.

var schedulerInstance = kernel.Get<Scheduler>();

Now, my scheduler have a method which adds tasks to a list, so not by using Ninject:

var tasksList = new List<DDPSchedulerTask>
{
    new AWNFileGeneratorTask(_logger, availablePlatforms, _backOfficeDataStore, _commonDataStore)
};

Then, all those tasks are being executed. Now, some of those tasks does require additional dependencies which I would like to resolve through Ninject but how should I do this?

Inside a task, I've created a property with the Inject attribute, but the object stays nulls.

[Inject]
private IDirectoryResolver directoryResolver { get; set; }

Anyone has an idea on how this can be resolved?

I can pass the kernel around to the different tasks, but something's telling me that this isn't the correct approach.

Kind regards

Alula

Answer 1

In such cases I usually use a Factory pattern. In the scheduler you can have the task factory as a dependency. This factory can also have multiple methods for creating different types of tasks.

class DDPSchedulerTaskFactory : IDDPSchedulerTaskFactory
{
    DDPSchedulerTaskFactory(ILogger logger, List<string> platforms, IBackOfficeDataStore backOfficeDataStore, ICommonDataStore commonDataStore)
    {
        _logger = logger;
        _platforms = platforms;
        _backOfficeDataStore = backOfficeDataStore;
        _commonDataStore = commonDataStore;
    }

    public IDDPSchedulerTask CreateNormal(){
        return new DDPSchedulerTask(
            _logger,
            _platforms,
            _backOfficeDataStore,
            _commonDataStore);
    }

    public IDDPSchedulerTask CreateSpecial(someAdditionalParameter){
        return new AnotherDDPSchedulerTask(
            _logger,
            _platforms,
            _backOfficeDataStore,
            _commonDataStore,
            someAdditionalParameter);
    }

    public IDDPSchedulerTask CreateTaskWithDifferentDependenties(yetAnotherParameter){
        return new AnotherDDPSchedulerTask(
            _logger,
            _platforms,
            yetAnotherParameter);
    }
}

Than in your scheduler you can add tasks like that:

class Scheduler{
    IDDPSchedulerTaskFactory _taskFactory;
    public Scheduler(IDDPSchedulerTaskFactory taskFactory){
        _taskFactory = taskFactory; // factory gets all the needed dependencies for all tasks from DI
    }   
    ...

    public void ConfigureTasks(){
        _tasks.Add(_taskFactory.CreateNormal());
        _tasks.Add(_taskFactory.CreateSpecial("Some important message"));
        _tasks.Add(_taskFactory.CreateTaskWithDifferentDependenties(123));
    }
}

Theiss

Ok, but something in your code isn't right. In your create method the return type is set to DDPSchedulerTask but you're returning a DDPSchedulerTaskFactory. Also, can you explain a bit more your solution? From where should I call the create method because I want the additional properties also be injected by Unity. – Alula

Thanks for the correction. You mind showing me from where I need to call this factory because I would like to keep using Ninject to ensure Dependency Injection. I don't see the glue to Ninject here. I still need to pass in the someAdditionalParameters where I want the Ninject kernel to resolve this. – Alula

@Alula Do you want the scheduler to add/configure all the tasks or do you want to call some AddTask() method on the scheduler from the outside? – Theiss

The scheduler itself is managing the creation of the tasks, nothing from the outside. If it was from the outside. But the problem is that the scheduler can define for example 5 tasks, where each task has 4 parameters which are automatticly resolved through Dependency Injection because those properties were injected in the constructor of the scheduler. But let's say that each of those 5 tasks, all needs another property, I don't want to add all those properties on the constructor of the scheduler. Instead I want the scheduler to resolve the dependencies for the task. So kernel sharing? – Alula

@Alula I think, what you are talking about is a service locator, which is claimed to be an any-pattern. Ideally you want to use the kernel in one place only - some entry-point. And let it propagate the dependencies further. See the edit. – Theiss

Ok. I know that the Service Locator is an anti pattern and that's exactely the reason why I would like to avoid that. But this current implementation does mean that I need to create a factory that has 1 method for each unique task, and all the dependencies required for all the tasks should be injected in my TaskSchedulerFactory, is that correct? I can't help it but I have the feeling that something's missing here. Anyway, I'll accept your answer because I do believe this is the solution. – Alula

@Alula This may seem to look like a lot of code, but it's kind of declarative - no place for bugs there, no need to test it. If you really have loads of tasks you can always compose the factory from smaller, more specialized ones. You can also create a factory where Create method takes "params object[]" and creates the tasks by reflection. Although I personally prefer the solution form the answer. – Theiss

You should consider using library Ninject.Extensions.Factory in this case. – Lackaday

You do not have to use reflection in this case. This can be easily done using the factory pattern. – Lackaday

@Pawel, please explain this because right now I'm building a factory where I pass in a type for which to create a type. I don't want a factory with a lot of methods where each method represents a single task. That just doesn't seem right. – Alula

Ok so look at my example. – Lackaday

Answer 2

You should take advantage of Ninject.Extensions.Factory.

Just create an interface that represents your tasks factory. Then pass information to Ninject that is a factory And he would create for you full implementations of this interface.

using System;
using System.Collections.Generic;
using Ninject;
using Ninject.Extensions.Factory;

internal class Program
{
    public static void Main(string[] args)
    {
        IKernel ninjectKernel = new StandardKernel();

        ninjectKernel.Bind<DDPSchedulerTask>().ToSelf();
        ninjectKernel.Bind<AWNFileGeneratorTask>().ToSelf();
        ninjectKernel.Bind<IDirectoryResolver>().To<DirectoryResolver>();
        ninjectKernel.Bind<ITaskFactory>().ToFactory();

        var mainTask = ninjectKernel.Get<DDPSchedulerTask>();
        mainTask.CreateDbSchedulerTask();
        mainTask.CreateAwnFileTask();

        Console.ReadLine();
    }
}

public interface ITaskFactory
{
    TTask CreateTask<TTask>() where TTask : DDPSchedulerTask;
}

public class DDPSchedulerTask
{
    private readonly ITaskFactory _tasksFactory;
    private readonly List<DDPSchedulerTask> _tasksList;

    public DDPSchedulerTask(ITaskFactory tasksFactory)
    {
        _tasksFactory = tasksFactory;

        _tasksList = new List<DDPSchedulerTask>();
    }

    public void CreateAwnFileTask()
    {
        var task = _tasksFactory.CreateTask<AWNFileGeneratorTask>();
        _tasksList.Add(task);
    }

    public void CreateDbSchedulerTask()
    {
        var task = _tasksFactory.CreateTask<DDPSchedulerTask>();
        _tasksList.Add(task);
    }
}

public class AWNFileGeneratorTask : DDPSchedulerTask
{
    [Inject]
    public IDirectoryResolver DirectoryResolver { get; set; }

    public AWNFileGeneratorTask(ITaskFactory tasksFactory)
        : base(tasksFactory)
    {
    }
}

public interface IDirectoryResolver
{
}

public class DirectoryResolver : IDirectoryResolver
{
}

@gisek As noted Giseke dependency injection via property is not the best solution. You can also use a constructor injection in this example.

public class AWNFileGeneratorTask : DDPSchedulerTask
{
    private IDirectoryResolver _directoryResolver;

    public AWNFileGeneratorTask(ITaskFactory tasksFactory, IDirectoryResolver directoryResolver)
        : base(tasksFactory)
    {
        _directoryResolver = directoryResolver;
    }
}

Extra params injection:

public interface ITaskFactory
{
    DDPSchedulerTask CreateDDPSchedulerTask();
    AWNFileGeneratorTask CreateAWNFileGeneratorTask(string extraParam);
}

public class AWNFileGeneratorTask : DDPSchedulerTask
{
    private IDirectoryResolver _directoryResolver;
    private string _extraParam;

    public AWNFileGeneratorTask(ITaskFactory tasksFactory, IDirectoryResolver directoryResolver,
        string extraParam)
        : base(tasksFactory)
    {
        _extraParam = extraParam;
        _directoryResolver = directoryResolver;
    }
}

public void CreateAwnFileTask()
{
    var task = _tasksFactory.CreateAWNFileGeneratorTask("extra");
    _tasksList.Add(task);
}

Lackaday

This way however you get bound to Ninject - no way to replace the IOC container easily. (Talking about Inject attribute) – Theiss

There is no problem to inject IDirectoryResolver by the constructor. It should also work. For example: public AWNFileGeneratorTask(ITaskFactory tasksFactory, IDirectoryResolver directoryResolver) : base(tasksFactory) – Lackaday

Ok. And how about passing additional parameters (not dependencies) to the tasks constructors? – Theiss

Where is the implementation of the ITaskFactory ? – Alula

There is no implemetation of ITaskFactory. Ninject would automatically implement this factory. – Lackaday

What about the comment made by @gisek ? – Alula

The answer is contained at the bottom of my answer :) If you want to pass additional parameters you need to change the interface. – Lackaday

Doesn't look bad. Less code than my solution, although the compiler will not help you avoid runtime errors concerning extra parameters here. Especially after refactoring. Guess the choice depends on the individual preferences. :) – Theiss

Exactly both solutions have their advantages and disadvantages. I posted my answer to note that you can also do it that way. – Lackaday

Damn, I cannot accept multiple answers as accepted because I like them both. However, I'm awarding the kudos to the answer posted by @gisek because his solution does work and he was first. – Alula