Is this a bad use of a static property?
If I have a class with a service that I want all derived classes to have access to (like a security object or a repository), I could 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 I've been seeing static properties everywhere lately as something that can be absolutely avoided. This seems cleaner to me than adding an ISecurity parameter to the constructor of every derived class I do. Considering everything I've read lately, I'm still wondering:
Is this an acceptable use of dependency injection, or am I violating some important design principle that might haunt me again? I am not doing unit tests at the moment, so maybe if I was then I would suddenly understand the answer to my question. Honestly, although I probably won't change my design, if there is another important reason why I should change it, I would very well be able to.
Edit . I made a couple of silly mistakes the first time I wrote this code ... it has been fixed. Just thought I'd point this out in case anyone notices :)
Edit : SWeko makes every effort to ensure that all receiver classes use the same implementation. In cases where I have used this design, the service is always a single, so it effectively enforces a pre-existing requirement. Naturally, this would be bad design if it were not.
source to share
This design can be problematic for several reasons.
You've already mentioned unit testing, which is quite important. This static dependency can make testing much more difficult. When the fake ISecurity
should ever be anything other than a Null Object , you will have to remove the fake implementation on the test break down. Removing it during a test break prevents other tests from being affected when you forget to remove that fake object. Failure to test makes your test more difficult. Not that hard, but it adds up when a lot of tests break code and you have a hard time finding a bug in your test suit when one test forgets to trigger the break. You must also make sure that the registered fake itemISecurity
is thread safe and will not affect other tests that may run in parallel (test environments like MSTest run tests in parallel for obvious performance reasons).
Another possible problem with injecting a dependency as static is that you are forcing that dependency to ISecurity
be single-handed (and probably thread-safe). This prohibits, for example, any interceptors and decorators that have a different lifestyle than singleton
Another issue is that removing this constructor dependency disables any analysis or diagnostics that the DI framework might do on your behalf. Since you are manually installing this dependency, the framework is not aware of this dependency. In a sense, you are transferring responsibility for managing dependencies back to your application logic, rather than letting Root Composition control how dependencies are linked together. The application should now know that it is ISecurity
actually thread safe. This is a responsibility that generally belongs to the root staff.
The fact that you want to keep this dependency in the base type might even be a sign of a violation of a general design principle: Single Responsibility Principle (SRP). It bears some resemblance to a design mistake I made in the past. I had a set of business operations that all inherited from the base class. This base class has implemented all sorts of actions like transaction management, logging, audit completion, adding fault tolerance and ... adding security checks. This base class became an unmanaged God Object . It was unmanageable, simply because he had too many responsibilities; he violated the PSA. Here's my story if you want to know more about it.
So, instead of having this security issue (it's probably a cross-cutting issue) implemented in the base class, try removing the base class together and using a decorator to add security for those classes. You can wrap each class with one or more decorators, and each decorator can handle one specific problem. This makes each decorator class easy to follow because they will follow the SRP.
source to share
The problem is that it doesn't actually do dependency injection even though it is encapsulated in the class definition. Admittedly,
static Security _security;
will be worse than Security
, but still the instances A
will not be able to use any security passed to them by the caller, they must depend on the global setting of a static property.
What I am trying to say is that your usage is no different from:
public static class Globals
{
public static ISecurity Security {get; set;}
}
source to share