Is it bad to use constructor injection and default constructors together?

Question

I'm currently trying to improve the testability of a legacy system, written in Java. The most present problem is the presence of “inner” dependencies which cannot be mocked out. The solution to this is pretty simple: Introduce dependency injection.

Unfortunately the code base is pretty large, so it would be a huge effort to introduce dependency injection throughout the whole application, up to the “bootstrapping”. For each class I want to test, I would have to change another hundred (maybe I’m exaggerating a bit here, but it definitely would be a lot) classes, which depend on the changed component.

Now to my question: Would it be ok, to use a two constructors, a default constructor which initialize the instance fields with default values and another one to allow the injection of dependencies? Are there any disadvantages using this approach? It would allow dependency injection for future uses but still doesn’t require the existing code to change (despite the class under test).

Current code (“inner” dependencies):

public class ClassUnderTest {

  private ICollaborator collab = new Collaborator();

  public void methodToTest() {
    //do something
    collab.doComplexWork();
    //do something
  }

}

With default / di constructor:

public class ClassUnderTest {

  private ICollaborator collab;

  public ClassUnderTest() {
    collab = new Collaborator();
  }

  public ClassUnderTest(ICollaborator collab) {
    this.collab = collab;
  }

  public void methodToTest() {
    //do something
    collab.doComplexWork();
    //do something
  }

}

That looks like a good strategy to me. Just make sure that you can use an instance field rather than reinstantiating a new instance at each method call. The two snippets are not equivalent. If a new instance must be created at each method invocation, the inject a factory of collaborator instead of injecting the collaborator. — JB Nizet, Jan 20 '12 at 09:36
Oh, you are right of course. I didn't think about it when I created that example. I'll edit the example. — user1159435, Jan 20 '12 at 09:42
This is called Bastard Injection, and can be problematic: http://stackoverflow.com/questions/6733667/is-there-an-alternative-to-bastard-injection-aka-poor-mans-injection-via-defa — Mark Seemann, Jan 20 '12 at 09:59
Thanks for pointing that out and for the link. Nevertheless, I think I'll have to live with it, at least at the moment. Other than that it's good to keep in mind, that my approach helps a bit in decoupling and increasing the testability, but the class still has a dependency to Collaborator at compile time. — user1159435, Jan 20 '12 at 10:21

score 0 · Accepted Answer · answered Jan 20 '12 at 09:33

0

This is absolutely fine, I do this occasionally also, especially with service-style classes where a default constructor is necessary because of legacy code or framework constraints.

By using two constructors, you're clearly separating the default collaborator objects while still allowing tests to inject mocks. You can reinforce the intent by making the second constructor package-protected, and keeping the unit test in the same package.

It's not as good a pattern as full-blown dependency injection, but that's not always an option.

answered Jan 20 '12 at 09:33

skaffman

398,947
96
818
769

Thanks a lot for your answer! I also thought about decreasing the visibility of the di constructor. I'm not sure what's the best approach in this case. A public constructor would allow easier reuse in future cases where other implementations of the dependencies should be used (which is the idea behind the whole dependency injection concept, right?). Do you see any disadvantages in keeping it public? – user1159435 Jan 20 '12 at 09:51

score 0 · Answer 2 · edited Dec 12 '13 at 12:49

I think this is perfectly OK. I would write the default-constructor in terms of the parametrized one, but that is probably a question of style and preference.

What I would be more cautious about is adding a setCollaborator()-function, because this radically changes the contract (and assumptions) for ClassUnderTest, which will lead to strange error-scenarios if called from code written by someone who doesn't know the history, haven't read the docs properly (or maybe there isn't any docs at all...)

Is it bad to use constructor injection and default constructors together?

2 Answers2