Reputation: 13367
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?
Upvotes: 0
Views: 169
Reputation: 172666
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:
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.
Upvotes: 1