Is it good/bad practice to add more constructors just for test purposes (to mock the DOCs used in my SUT) like this :

public class A {
    private B b = new B();
    private C c = new C();

    public A(){
    }

    /***
     * used for test puproses
     * @param a
     * @param b
     */
    A(A a, B b) {
        // overwrite the existing variables
        this.a = a;
        this.b = b;
    }
}

class B {
}

class C {
}

Note that i force the new constructor visibility to be package private not to pollute the SUT's api and also choose to instantiate the DOCs inside the SUT for client usage convenience.

SUT: System under test (A)

DOC: depended-on component (B and C)

What do you think ? Thanks in advance.

New contributor
isqo is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
up vote 5 down vote accepted

I would do this regardless of testing just to avoid hard coding B and C. In fact I would reject this code if you didn't provide a way to overwrite b and c. Now, rather than being hard coded, they are simply overwritable default values. That's a good thing.

Your code looks good to me (except the typos in the second constructor, see below).

You might consider the following refactoring, which has the following advantages:

  1. It doesn't needlessly instantiate classes B and C.
  2. It allows for fields b and c to be declared final, if appropriate.

Refactoring:

public class A {
    private final B b;
    private final C c;

    public A() {
        this(new B(), new C());
    }

    public A(B b, C c) {
        this.b = b;
        this.c = c;
    }
} 
New contributor
Marco is a new contributor to this site. Take care in asking for clarification, commenting, and answering. Check out our Code of Conduct.
  • This approach works good for garbage collected languages that manage heap memory. You'll want to be careful about the new operator in languages like C++ though. – Greg Burghardt Nov 9 at 12:55
  • I like avoiding needless instantiation. Not sure I like needlessly making one constructor depend on another. – candied_orange Nov 10 at 2:43
  • 1
    @candied_orange: It's funny. I don't have problems following static methods calling each other, but I've definitely gone cross-eyed trying to figure out which constructors are calling which constructors --- and constructors are basically just static methods! I agree. You have to be very careful when calling one constructor from another. It's too easy to get your initialization logic mixed up in your head. – Greg Burghardt Nov 11 at 2:38
  • @GregBurghardt I've often written a validation function that many constructors can call just to avoid constructors calling constructors. Whole thing still feels like a kludge though. It's so much easier to create overwritible defaults in languages with named parameters. – candied_orange Nov 11 at 5:17

You just discovered a code smell, by the virtue of tests being hard to write. I suggest you provide A & B objects via dependency injection, rather than instantiating them yourself. You'll end up with a more flexible architecture, that is also readily testable via mocks.

  • You don't need to mock every dependency of an object. Only those that are difficult to set up, interact with out of process resources, or perform poorly. – Greg Burghardt Nov 9 at 12:54

In C#, I prefer to create such instances with static methods. They also carry a "ForTesting" in their name, so that their purpose becomes clear.

It could look like

internal static A CreateWithDependenciesForTesting(B b1, C c1)
{
    A instance = new A
    {
        b = b1,
        c = c1
    };
    return instance;
}

I am not a Java programmer, so I cannot tell you if (or how) that method is applicable there.

Your Answer

isqo is a new contributor. Be nice, and check out our Code of Conduct.
 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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