If I have a class with a service that I want all derived classes to have access to (say a security object, or a repository) then I might do something like this:
public abstract class A
{
static ISecurity _security;
public ISecurity Security { get { return _security; } }
public static void SetSecurity(ISecurity security) { _security = security; }
}
public class Bootstrapper
{
public Bootstrapper()
{
A.SetSecurity(new Security());
}
}
It seems like lately I see static properties being shunned everywhere as something to absolutely avoid. To me, this seems cleaner than adding an ISecurity parameter to the constructor of every single derived class I make. Given all I've read lately though, I'm left wondering:
Is this is an acceptable application of dependency injection or am I violating some major design principle that could come back to haunt me later? I am not doing unit tests at this point so maybe if I were then I would suddenly realize the answer to my question. To be honest though I probably won't change my design over that, but if there is some other important reason why I should change it then I very well might.
Edit: I made a couple stupid mistakes the first time I wrote that code... it's fixed now. Just thought I'd point that out in case anyone happened to notice :)
Edit: SWeko makes a good point about all deriving classes having to use the same implementation. In cases where I've used this design, the service is always a singleton so it effectively enforces an already existing requirement. Naturally, this would be a bad design if that weren't the case.
UPDATE: In DIPP&P, Mark Seemann and I call this pattern Ambient Context and define it as an anti-pattern. A excerpt of our reasoning for calling it an anti-pattern can be found here, and the remaining of my answer below states the same concerns.
This design could be problematic for a couple of reasons.
You already mention unit testing, which is rather important. Such static dependency can make testing much harder. When the fake ISecurity ever has to be anything else than a Null Object implementation, you will find yourself having to removing the fake implementation on test tear down. Removing it during test tear down prevents other tests from being influenced when you forget to remove that fake object. A test tear down makes your test more complicated. Not that much complicated, but having this adds up when many tests have tear down code and you'll have a hard time finding a bug in your test suit when one test forget to run the tear down. You will also have to make sure the registered ISecurity fake object is thread-safe and won't influence other tests that might run in parallel (test frameworks such as MSTest run tests in parallel for obvious performance reasons).
Another possible problem with injecting the dependency as static, is that you force this ISecurity dependency to be a singleton (and probably to be thread-safe). This disallows for instance to apply any interceptors and decorators that have a different lifestyle than singleton
Another problem is that removing this dependency from the constructor disables any analysis or diagnostics that could be done by the DI framework on your behalf. Since you manually set this dependency, the framework has no knowledge about this dependency. In a sense you move the responsibility of managing dependencies back to the application logic, instead of allowing the Composition Root to be in control over the way dependencies are wired together. Now the application has to know that ISecurity is in fact thread-safe. This is a responsibility that in general belongs to the Composition Root.
The fact that you want to store this dependency in a base type might even be an indication of a violation of a general design principle: The Single Responsibility Principle (SRP). It has some resemblance with a design mistake I made myself in the past. I had a set of business operations that all inherited from a base class. This base class implemented all sorts of behavior, such as transaction management, logging, audit trailing, adding fault tolerance, and.... adding security checks. This base class became an unmanageable God Object. It was unmanageable, simply because it had too many responsibilities; it violated the SRP. Here's my story if you want to know more about this.
So instead of having this security concern (it's probably a cross-cutting concern) implemented in a base class, try removing the base class all together and use a decorator to add security to those classes. You can wrap each class with one or more decorators and each decorator can handle one specific concern. This makes each decorator class easy to follow because they will follow the SRP.