How do i use dependency injection correct?

Question

So, this is my first time using Ninject, everything worked great, until i ran into a problem where Some service classes needed addition ModelState argument in constructor. Problem was solved by answer on this question : answer on previous question

Now next problem were that i have about 40 service classes which need to have access to ModelState in constructor, therefor i choose to use generics, and i really don't know if this is actually legit in any way? or do i have to redo it?

private readonly IServiceFactory<IAccountService, AccountService> service;
public AccountController(IServiceFactory<IAccountService, AccountService> asf)
{           
  this.service = asf;
}

RegisterServices from Ninject:

private static void RegisterServices(IKernel kernel)
{
    kernel.Bind(typeof(IServiceFactory<,>)).To(typeof(ServiceFactory<,>));
}

Generics:

public interface IServiceFactory<T, Y> where T : class  where Y : class
{
     T Create(ModelStateDictionary modelState);
}

public class ServiceFactory<T, Y> : IServiceFactory<T, Y>
    where T : class
    where Y : class
{

    public T Create(ModelStateDictionary modelState)
    {
        var x = (T) Activator.CreateInstance(typeof (Y), new ModelStateWrapper(modelState));
        return x;
    }
}

What i tryed to achieve, is that i wanted to avoid creating 40 factory interfaces and 40 factorys to make exatcly the same thing. My solution works, but does it actually support Dependency Injection? i cant really figure it out.

Answer 1

If you note the last part of my answer on your previous question, you can also just pass the reference to ModelState directly through the IAccountService interface. In this case, that would simplify things, but given the lack of context in your previous question it was unclear which would be the better choice.

public class AccountController : Controller
{
    private readonly ILanguageService languageService;
    private readonly ISessionHelper sessionHelper;
    private readonly IAccountService accountService;

    public AccountController(ILanguageService languageService, ISessionHelper sessionHelper, IAccountService accountService)
    {
        if (languageService == null)
            throw new ArgumentNullException("languageService");
        if (sessionHelper == null)
            throw new ArgumentNullException("sessionHelper");
        if (accountService == null)
            throw new ArgumentNullException("accountService");

        this.languageService = languageService;
        this.sessionHelper = sessionHelper;
        this.accountService = sessionHelper;
    }

    [HttpPost]
    public ActionResult PostSomething()
    {

        // Pass model state of the current request to the service.
        this.accountService.DoSomething(new ModelStateWrapper(this.ModelState));

    }
}

This is clearly easier than injecting 40 factories, and would make your DI configuration simpler.

The only downside of this approach (if you can call it that) is that your IAccountService interface now requires an implemenatation of IValidationDictionary to be passed. This means that every concrete implementation of IAccountService must have a dependency on ModelState's abstraction, IValidationDictionary. If that is the expectation of the design, then that is fine.

Also, you should put guard clauses in your class to ensure that if the DI container fails to provide an instance an exception will be thrown. The readonly keyword in the private fields makes it impossible to change the value of those fields outside of the constructor. Basically, these 2 things combined guarantee that you will have an instance that is initialized in the constructor and will remain constant throughout the lifetime of the object.

Final Thoughts

DI is by its nature complex. If you want to get a good answer, you should provide more context about the problem. I realize that SO says that you should post the minimum required code, but questions about DI generally require more context to answer than other questions because of its complexity.

Had I known you were trying to solve a problem for 40 similar services or had known about the relevant part of the IAccountService members that use the dependency (which you still haven't provided), my answer probably would have been different. It could still change if you provide more context, but there are only so many assumptions I can make about what you are trying to achieve. It helps a lot if you fill in the blanks.