How to design a class for unit tests

Question

I have a Java class like the following:

public class MyClass {

    /** Database Connection. */
    private dbCon;

    public MyClass() {
        dbCon = ...
    }

    public void doSomethingWith(MyData data) {
        data = convertData(data);
        dbCon.storeData(data);
    }

    private MyData convertData(MyData data) {
        // Some complex logic...
        return data;
    }
}

since the true logic of this class lies in the convertData() method, I want to write a Unit Test for this method. So I read this post

How do I test a private function or a class that has private methods, fields or inner classes?

where a lot of people say that the need to test a private method is a design smell. How can it be done better?

I see 2 approaches:

Extract the convertData() method into some utility class with a public api. But I think this would be also bad practice since such utility classes will violate the single responsibilty principle, unless I create a lot of utility classes with maybe only one or two methods.
Write a second constructor that allows injection of the dbCon, which allows me to inject a mocked version of the database connection and run my test against the public doSomething() method. This would be my preferred approach, but there are also discussions about how the need of mocking is also a code smell.

Are there any best practices regarding this problem?

You need to inject `dbCon` and test `doSomethingWith`. "the need of mocking is also a code smell." - how, where, who? — luk2302, Feb 28 '18 at 12:27
If `convertData` doesn't depend on fields in `MyClass` (i.e. is a pure function), there's no harm in making it `public static` IMHO. Java somewhat forces you into this "everything must be an object" mindset and away from the functional principles that lend themselves _so_ well to unit testing. — Thomas, Feb 28 '18 at 12:29

score 4 · Accepted Answer · answered Feb 28 '18 at 12:39

Extract the convertData() method into some utility class with a public api. But I think this would be also bad practice since such utility classes will violate the single responsibility principle, unless I create a lot of utility classes with maybe only one or two methods.

You interpretation of this is wrong. That is exactly what the SRP and SoC (Separation of Concerns) suggests

public interface MyDataConverter {
    MyData convertData(MyData data);
}

public class MyDataConverterImplementation implements MyDataConverter {
    public MyData convertData(MyData data) {
        // Some complex logic...
        return data;
    }
}

convertData implementation can be now tested in isolation and independent of MyClass

Write a second constructor that allows injection of the dbCon, which allows me to inject a mocked version of the database connection and run my test against the public doSomething() method. This would be my preferred approach, but there are also discussions about how the need of mocking is also a code smell.

Wrong again. Research Explicit Dependency Principle.

public class MyClass {
    private DbConnection dbCon;
    private MyDataConverter converter;

    public MyClass(DbConnection dbCon, MyDataConverter converter) {
        this.dbCon = dbCon;
        this.converter = converter;
    }

    public void doSomethingWith(MyData data) {
        data = converter.convertData(data);
        dbCon.storeData(data);
    }
}

MyClass is now more honest about what it needs to perform its desired function.

It can also be unit tested in isolation with the injection of mocked dependencies.

How to design a class for unit tests

1 Answers1