CODEWITHSUNDEEP
javaspringoopspring-boot
GROX13
GROX13

Reputation: 4765

How good is it to use additional constructors just for testibility?

Currently I needed to write SecondClass and decided to write using this kind of solution because otherwise it wouldn't be testable.

@Component
public class FirstClass {

    public void doStuff() {
        System.out.println("First Class stuff!");
    }

}

@Component
public class SecondClass {

    private final Random random;
    private final FirstClass firstClass;

    @Autowired
    public SecondClass(FirstClass firstClass) {
        this(new Random(), firstClass);
    }

    public SecondClass(Random random, FirstClass firstClass) {
        this.random = random;
        this.firstClass = firstClass;
    }

    public void doOtherStuff() {
        firstClass.doStuff();
        System.out.println("Second Class stuff " + random.nextInt(10));
    }

}

My colleagues didn't liked my way of solving it and preferred implementation of SecondClass like this:

@Component
public class SecondClass {

    private Random random;
    private final FirstClass firstClass;

    @Autowired
    public SecondClass(FirstClass firstClass) {
        this.random = new Random();
        this.firstClass = firstClass;
    }

    public void doOtherStuff() {
        firstClass.doStuff();
        System.out.println("Second Class stuff " + random.nextInt(10));
    }

    public void setRandom(Random random) {
        this.random = random;
    }

}

I disagree with this kind of solution because I think that Random is the necessary part of this class, it wont be changed in the run time and that setter is only necessary for testing purposes and that's why I prefer solution with two constructors.

We also came up with this kind of constructor:

@Autowired(required = false)
public SecondClass(FirstClasss firstClass, Random random) {
    this.random = (random == null ? new Random() : random)
    ...
}

But there is actually more components injected in the constructor so it would be preferable if all of them would have been required.

I'm interested if anybody here had this kind of experience before? What do you think about this case and if there is any better way of solving this problem?

Upvotes: 4

Views: 173

Answers (3)

Little Santi
Little Santi

Reputation: 8803

You are absolutely right, my friend. Your approach is the right one, because, according to Object Oriented design, an object must be initialized to a valid state after any of its constructors is executed. So, if the random field is necessary, it must be initialized in all constructors. Moreover, if it shall not be changed later, it must be final (and have no setters). So the second solution is not right because it violates this principle.

I don't like the third solution either because it can be foolish for the client code: While the client assumes that a null is being set as the input value, actually another (not-controlled) value is used instead.

So if you need that constructor for the SecondClass to be testeable, go and add it. Because if a class is not testeable, it is also not executable. Testing is the right way to ensure that a class might be usable.

Upvotes: 1

davidxxx
davidxxx

Reputation: 131456

If you declare two constructors in SecondClass and you use one of these only in unit-test, it means that the constructor for unit-test could be replaced by a more relevant trick for your unit-test such as reflection to set the value of the random field.
You should not open your API just for unit testing when you can avoid.
You should open the your API just for unit testing when you have not the choice.

If the random field is read-only and a known value as soon as the SecondClass is instantiated, you should have a single constructor for SecondClass. And the random field should be final and valued in the constructor of SecondClass.

Upvotes: 2

Koos Gadellaa
Koos Gadellaa

Reputation: 1254

Your problems spring from having final fields. Follow Uncle Bob, and feel free to make them protected so you can write to them in your test class. You're not writing an interface, you're writing code, so access modifiers don't really matter that much, and protected is fine since it allows package-access.

Or you could look at setting them yourself in your testcase by hard-wiring them, say ReflectionUtils.setField

Upvotes: 2

Related Questions