Spllitting large method for testing

Question

I'm working on some project and now I'm struggling with testing. I have the following class:

public interface Connector{ }

public class ConnectorFactory{


     public Connector create(){
         //Very complicated logic
     }
}

It's absolutely necessary to test this class. But this method is too large to be easily tested. The solution I found is to split in into default-access methods like this:

public class ConnectorFactory{


     public Connector create(){
         initState();
         checkIncomingParameters();
         applyTemplates();
         prepareResult();
         InputStream is = openInputStream();
         return createFromInputStream(is);
     }

     void initState(){ 
         //... 
     }

     void checkIncomingParameters(){ 
         //... 
     }

     String applyTemplates(){
         //...
     }

     void prepareResult(){
         //...
     }

     InputStream openInputStream(){
         //...
     }

     Connector createFromInputStream(InputStream is){
         //...
     }
}

Each of these default-access methods are easily tested. By the only reason they are default-access ones is testing.

Is it common to do like that?

Each one of those steps (single responsibility) can be it's own package private class which can be tested independently. Using mockito, the test for `ConnectorFactory` can verify the mock of each step is invoked in the expected order. — Andrew S, May 15 '17 at 14:27
As @AndrewS said, please take this one step further and make some of these functions into classes. While you're at it, do not forget PROPER dependency injection - do not call `new` as part of any constructors. — , May 15 '17 at 14:32

score 1 · Answer 1 · answered May 15 '17 at 14:38

So the question seems to be: we have all these methods that together create something, shouldn't they be private? Is testing a sufficient reason to expose them to the world?

Well, not the world, by default they are package-private, but still...

But let's turn the question around: why are these methods in the same class in the first place? The obvious answer is: well, that's how things were before, but that is not a good one. When writing testable code, we strive, among other things, for Loose Coupling and High Cohesion. Your original code is tightly coupled and not very cohesive. It is complex enough that it has its own init(), then templates are probably hard-coded, and somewhere deep inside it opens an inputstream on its own.

To improve on that design, you should modularize the logic. Somewhere there's a input stream parser, somewhere there's a template engine, and somewhere a result is put together and returned. You should make each of these a class of its own, with proper APIs, and then you'll see your worries disappear.

score 1 · Answer 2 · edited May 23 '17 at 12:34

The methods that are specific to only one class should be made private, that ensures abstraction. You should not just make change their access modifier just for the purpose of testing. There are ways to test private methods and you can look at this previous stack overflow post:

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

These are some other things I keep in mind while testing :

Make sure you have dependency injection
Its always good to make your program as modular as possible, its easy to read, easy to resuse and also easy to Test. So, I think it's good that you broke the method into smaller ones.

Spllitting large method for testing

2 Answers2