2

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.

6

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.

4

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;
    }
} 
  • 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 '18 at 12:55
  • I like avoiding needless instantiation. Not sure I like needlessly making one constructor depend on another. – candied_orange Nov 10 '18 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 '18 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 '18 at 5:17
1

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 '18 at 12:54
0

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

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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