1

I have a bunch of classes that have a set of dependencies. Dependency Injection for this project will be overkill, so currently we have the following pattern in a number of cases:

public MyClass() : this(null, null) {}

public MyClass(Dependancy x, Dependancy y)
{
  this.x = x ?? new Dependancy();
  this.y = y ?? new Dependancy();
}

I don't like this code, but I'm not totally sure why. One reason would be the fact that it fiddles with parameters that are passed in, the other is that I might want the parameter to be null, and stay null.

Are there any strong reasons to avoid/use this pattern or any other, or is it basically just personal preference?

Improve this question

2 Answers 2

Reset to default
4

You do not like it because of two reasons:

  • There is no need to have a parametrized constructor to which you pass NULLs; have a second no-args constructor;
  • Having a third class (name it Factory, name it Container, does not matter) injecting those default dependencies will never be overkill, compared to the benefits.
Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

+1 DI is never overkill. Maybe you don't have the need for a container, but the pattern itself is always a better choice than newing up dependencies in the dark.
3

The thing with the code you've posted is that you are using dependency injection, you're just not using Inversion of Control. MyClass has its dependencies injected into it, but it then assumes control of what to do if they aren't as it expects. There's a few reasons this is unpleasant:

  1. The higher-level class MyClass is coupled to the lower level class Dependency.
  2. MyClass is acting as a ServiceLocator, which in itself is an anti-pattern, but also violates the Single Responsibility Principle.
  3. The parameterless constructor has hidden side-effects; the places where it's used are unknowingly constructing MyClass with its dependencies both set to Dependency.

As @Alessandro Santini says, I'd really encourage you to scrap this and use a DI / IoC container. At the very least get rid of the parameterless constructor and force anything which wants to construct a MyClass instance to supply it with appropriate dependencies. The two-argument constructor should then throw exceptions if the dependencies it's given are null.

Improve this answer

1 Comment

Good, was what I was hoping everyone would say! Thanks

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.