Search code examples
c#asp.net-web-apininject

Ninject WebAPI dependency with a parameter


I am trying to set up Ninject for the first time using this little library. I have a base controller which currently looks like this:

public class BaseController : ApiController
{

    // Private properties
    private IModelFactory factory;

    // Public properties
    public IServiceInterface Connection { get; private set; }
    public IModelFactory Factory { get { return this.factory ?? (this.factory = new ModelFactory()); } }

    /// <summary>
    /// Default constructor
    /// </summary>
    public BaseController()
    {

        // Get our database settings
        var setting = ConfigurationManager.AppSettings["Server"].ToUpper();
        var type = (setting == "LIVE") ? ServiceInterface.ServerType.Azurerelay_live : ServiceInterface.ServerType.Azurerelay_test;

        try
        {

            // Apply to our public properties
            this.Connection = new ServiceInterface(type);

        } catch
        {

            // Throw a 401 error
            throw new HttpResponseException(System.Net.HttpStatusCode.Unauthorized);
        }
    }
}

I would like to use Ninject on this constructor, but I can't seem to figure out how to do the ServiceInterface because it requires a parameter. I tried to bind the service like this:

// List and Describe Necessary HttpModules
// This class is optional if you already Have NinjectMvc
public class NinjectHttpModules
{
    //Return Lists of Modules in the Application
    public static NinjectModule[] Modules
    {
        get
        {
            return new[] { new MainModule() };
        }
    }

    //Main Module For Application
    public class MainModule : NinjectModule
    {
        public override void Load()
        {
            //TODO: Bind to Concrete Types Here
            Kernel.Bind<IServiceInterface>().To<ServiceInterface>();
            Kernel.Bind<IFactory>().To<Factory>();
        }
    }
}

and changing my constructor to this:

public class BaseController : ApiController
{

    // Public properties
    public readonly IServiceInterface Connection;
    public readonly IModelFactory Factory;

    /// <summary>
    /// Default constructor
    /// </summary>
    public BaseController(IServiceInterface connection, IModelFactory factory)
    {

        try
        {

            // Apply to our public properties
            this.Connection = connection;
            this.Factory = factory;

        } catch
        {

            // Throw a 401 error
            throw new HttpResponseException(System.Net.HttpStatusCode.Unauthorized);
        }
    }
}

But that won't work because the ServiceInterface requires a parameter to state if it is live or not. Does anyone know how I can get this code to work?


Solution

  • Prevent reading configuration values from places like the AppSettings at runtime. Not only is there no need to read them more than once (performance), but scattering their use throughout the application has a profound effect on maintainability and reliability. Instead, move the retrieval of configuration values to the startup path of the application. This:

    • Prevents having to read them over and over again, allows the application to fail fast when a value is missing.
    • Improves testability because classes loose the dependency on the configuration system.
    • Keeps access to those configuration values in one single location, making it much easier to change the application or the structure of the configuration files.

    Your Composition Root should look something like this:

    public override void Load()
    {
        var setting = ConfigurationManager.AppSettings["Server"].ToUpper();
        var type = (setting == "LIVE")
            ? ServiceInterface.ServerType.Azurerelay_live
            : ServiceInterface.ServerType.Azurerelay_test;
    
        Kernel.Bind<IServiceInterface>().ToMethod(c => new ServiceInterface(type));
        Kernel.Bind<IModelFactory>().ToMethod(c => new ModelFactory());
    }
    

    Another thing is that you should Favor composition over inheritance. Base classes are a sign of design problems. So ditch the base class and inject IModelFactory and IServiceInterface into the constructors of concrete classes that require their use:

    public class HomeController : ApiController
    {
        private readonly IModelFactory modelFactory;
        private readonly IServiceInterface serviceInterface;
    
        public HomeController(IModelFactory modelFactory, IServiceInterface serviceInterface) 
        {
            this.modelFactory = modelFactory;
            this.serviceInterface = serviceInterface;
        }
    
        // actions here
    }
    

    Also prevent from doing anything in your constructors except storing incoming dependencies. You injection constructors should be simple. This includes things like throwing exceptions. Prevent throwing exceptions from constructors, because this makes resolving your object graph unreliable.

    Refrain from using the Service Locator anti-pattern, make sure components only have one constructor, don't inject any runtime data into them and look out for constructor over-injection, since this is an indication of violating the Single Responsibility Principle.