Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

I started using TDD for the first time on a project I started recently.

I have a few objects with dependencies and structures that look something like this:

public class MyClass
{
    private readonly IDependency1 _dependency1;
    private readonly IDependency2 _dependency2;

    public MyClass(IDependency1 d1, IDependency2 d2)
    {
        _dependency1 = d1;
        _dependency2 = d2;
    }

    public MyClass()
    {
        _dependency1 = new Dependency1();
        _dependency2 = new Dependency2();
    }
}

For tests I use Mock objects for the dependencies but in the implementation I sometimes let the constructor new up a concrete object.

It feels like I am cheating by doing this but I'm not sure why it is bad in practice.

Is this considered bad practice? If so why?

share|improve this question

3 Answers 3

up vote 3 down vote accepted

First, it's better to chain constructors, so that you're using only one piece of code.

public MyClass() : this(new Dependency1(), new Dependency2()) { }

This is much more maintainable over time, specifically if you ever want to do something else in both constructors.

As to whether this is good practice ... it's not the best practice, but it's perfectly acceptable, when you can't or don't want to use Dependency Injection. It's particularly useful when you're adding tests to a new module in legacy code and want to avoid reengineering the entire project.

And when I say Dependency Injection, I don't necessarily mean an IOC container or service locator. I mean that if you CAN pass the dependencies from the calling code to the constructor then do so, even if it means a little more code.

Much better to decouple your implementation from the calling code as much as possible. You might want to reuse it later with an entirely different implementation.

For more information, look at the Dependency Inversion Principle.

share|improve this answer

Ya don't do this. By putting new in the 2nd constructor, you're doing a few things that will make your life harder over time:

  • creating a hard, compile-time dependency between your class and the implementation classes you're instantiating.

  • making decisions at coding time about which implementation classes to bake into the class, which goes against the idea of dependency injection

  • creating more code that you have to test and maintain

  • breaking "do not repeat yourself" in a subtle way, because the 2nd constructor does in code what your bean factory is doing through configuration

So you won't be smitten by the God of Dependency Injection, but you're creating extra work for you and your team.

Even if you're not using mocks, you'd be better off letting the client instantiate & inject the concrete classes to use. Or else write up a Builder or Factory, and let it make those decisions about concrete classes.

...the acid test is, how much smaller, and therefore more maintainable, is your code without that second constructor?

(full disclosure: sometimes I find it makes sense to do this, but I would reserve it as a special case rather than a general practice.)

share|improve this answer

Your second constructor should call the first one. Otherwise it looks fine for me.

public MyClass() : this(new Dependency1(), new Dependency2())
{
    // should be empty here
}

In the test suite

  • test MyClass works with mock objects (with constructor MyClass(IDependency1, IDependency2))
  • test default constructor MyClass() call MyClass(IDependency1, IDependency2) with correct concrete type

and you are good to go

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.