2

I'm trying to add ninject to the following setup -

public class BaseController : Controller
{
    protected ILogger Logger {get;}
    public BaseController() { Logger = new MyLogger(); }
}
public class Controller1Controller : BaseController { ... }
public class Controller2Controller : BaseController { ... }
....
public class ControllerNController : BaseController { ... }

With ninject, adding an ILogger parameter to BaseController works great -

public class BaseController : Controller
{
    protected ILogger Logger {get;}
    public BaseController(ILogger logger) { Logger = logger; }
}

but now it also requires adding a constructor in each of the child controllers because the base class no longer has a parameterless constructor-

public class Controller1Controller : BaseController { 
    public Controller1Controller(ILogger logger) : base(logger) { }
}

There are over 50 child controllers and going forward it might become a bit of a maintenance problem if we have to add/remove more dependencies. Further, the code being added to each controller is exactly the same.

Is there a way to keep the child controllers as they are (without any constructors) but still make that change to the BaseController?

Uwe Keim
  • 39,551
  • 56
  • 175
  • 291
Achilles
  • 1,099
  • 12
  • 29

3 Answers3

1

Simple answer: No (You have to update your derived class constructors)

Explanation: For injected instance of ILogger to be passed across derived and base classes, you need to update your derived constructors with ILogger parameter and then forward the parameter to the base. Unless, there is a wiser solution to this situation.

See the first point in disadvantages section on wiki DI Disadvantages (DI comes with this baggage)

Additionally: You can create a protected member in the base class to hold the ILogger instance, which will make this protected instance accessible to all the derived classes.

Update: Partially agree with tomludd's solution of using Property injection. See this SO answer. For more DI techniques, please do read articles/blogs from Mark Seemann or his book DI in .NET

Community
  • 1
  • 1
Numan
  • 3,918
  • 4
  • 27
  • 44
  • Thank you. Why partially agree with tomludd's solution? – Achilles Jan 17 '17 at 10:02
  • @Achilles: Because, with setter/property injection, you are forced to make inject-able properties "public". And in this case, the DI framework is dictating these rules. Additionally, please see the link in my answer to an SO question. Also, the solution of accessing the kernel in base controller is kind of an anti-pattern and you are defeating the purpose of employing DI. You are doing service location see this blog post: http://blog.ploeh.dk/2010/02/03/ServiceLocatorisanAnti-Pattern/ – Numan Jan 17 '17 at 10:22
  • Thank you, makes sense now. Thanks for the informative links as well. – Achilles Jan 17 '17 at 10:27
  • Good reading, thanks again. There was another problem that I found when I actually ran the project - The injected properties are set after the BaseController's constructor has finished running, so they can't be used in the constructor for other stuff. I ended up implementing IInitializable as well to get around that problem until I get around to implementing a better solution. – Achilles Jan 17 '17 at 10:43
  • @Achilles: :) Those are some of the quirks if you don't do constructor injection. – Numan Jan 17 '17 at 12:18
1

Sorry but no.

As an alternative solution you can get ILogger from Ninject: Using property injection instead of constructor injection

Community
  • 1
  • 1
tomludd
  • 96
  • 6
  • Thanks. This solved the problem, but with the caveat that the properties must be public and have setters, so it (from what I've seen in my testing so far) won't work with non-public or readonly properties. – Achilles Jan 17 '17 at 10:01
  • If you have access to Ninject Kernel then you can get it like this: `public class BaseController : Controller { protected ILogger Logger {get;} public BaseController(){ Logger = Kernel.Get(); } }` – tomludd Jan 17 '17 at 10:09
  • @tomludd: This is not DI, this is service location. And it is kind of an anti pattern. see: http://blog.ploeh.dk/2010/02/03/ServiceLocatorisanAnti-Pattern – Numan Jan 17 '17 at 10:27
  • @Nauman I'm well aware, it is just a simple solution to the problem. – tomludd Jan 17 '17 at 11:00
  • @tomludd: Excellent! No offences meant and none taken as well, I guess :) – Numan Jan 17 '17 at 12:04
0

Try this in BaseController constructor

public BaseController(ILogger logger = null) 
{ 
    if (logger != null)
        Logger = logger;
    else
        Logger = new MyLogger();
}
kgzdev
  • 2,770
  • 2
  • 18
  • 35