Factory design pattern - Not use static methods because unit testing is a problem

Question

I know this question has been asked several times in stackoverflow but somehow still had some trouble figuring out a solution. The following example I think is a good case to have static methods

public class ConnectionFactory
{
     public static Connection createConnection(ConnectionType connectionType, String ipAddr, Integer port)
    {
           //Some error checking
         switch(connectionType)
         {   
             case TCP:
                  return createTcpConnection(ipAddr, port);
             case UDP:
                  return createUdpConnection(ipAddr, port);
             case RTP:
                  return createRtpConnection(ipAddr, port);
             case SCTP:
                  return createRtpConnection(ipAddr, port);
             default:
                  break;
         }
    }

    // TcpConnection, RtpConnection, SctpConnection and UdpConnection implement interface Connection
    public Connection createTcpConnection()
    {
        Connection connection = new TcpConnection();
         .....
         .....
         return connection;
    }

    public Connection createUdpConnection()
    {
        Connection connection = new UdpConnection();
        .....
        .....
        return connection;
    }

    ....
    ....
}

And suppose if I have a CommunicationService like the following

public class CommunicationService
{
    public void initConnectionPool(ConnectionType connectionType)
    {
        for(int i = 0; i < MAX_CONNECTIONS; i++)
             connectionList.add(ConnectionFactory.createConnection(connectionType, "domain.com", 40203));

        //Some more code here to do further processing
          ......
          ......
    }    

    //Some more methods
}

Like this, different communication services can create and maintain multiple type of connections.

I want to test initConnectionPool method and in a unit test environment, socket creation will definitely fail.

I can change the ConnectionFactory to a concrete class and mock it out. But isn't this case a good situation to create a class with static methods? I am not trying to maintain any state in ConnectionFactory. So, if using static methods where it may be appropriate may cause testing problems when do we use static methods? Or is it not appropriate to use static methods here?

EDIT: Solution I went with

public class CommunicationService
{
    public void initConnectionPool(ConnectionType connectionType)
    {
        for(int i = 0; i < MAX_CONNECTIONS; i++)
             connectionList.add(connectToHost(connectionType));

        //Some more code here to do further processing
          ......
          ......
    }    

    public Connection connectToHost(ConnectionType connectionType)
    {
        ConnectionFactory.createConnection(connectionType, "domain.com", 40203)
    }
    //Some more methods
}

In the test overrode connectToHost and returned a mock.

Answer 1

Another common practice is to define your own helper class which can wrap calls static methods and constructors. It is quite useful if you need to mock some third-party libraries, that you can't rewrite.

public class CommunicationService {
    private final CommunicationServiceHelper helper;
    public CommunicationService() {
        this(new CommunicationServiceHelper());
    }

    @VisibleForTesting
    CommunicationService(CommunicationServiceHelper helper) {
        this.helper = helper;
    }

    public void initConnectionPool(ConnectionType connectionType)
    {
        for(int i = 0; i < MAX_CONNECTIONS; i++)
             connectionList.add(helper.createConnection(connectionType, "domain.com", 40203));

        //Some more code here to do further processing
          ......
          ......
    }    

    //Some more methods

    @VisibleForTesting
    static class CommunicationServiceHelper {
        public Connection createConnection(ConnectionType connectionType, String ipAddr, Integer port) {
            return ConnectionFactory.createConnection(connectionType, ipAddr, port);
        }
    }
}

Answer 2

First of all: I don't see any problem with your ConnectionFactory as long as it doesn't contain any state (which it looks like it doesn't). The problems you have with testing do not arise from the static factory method in this case.

Unit testing code that interacts directly with environment elements like the file system, a database or the network is always hard. In this case you would need to mock socket creation to do so and that is probably not worth the effort. Even if you did it, the test code would be large, hard to maintain and brittle.

Please reconsider why you want to write a unit test for this class...

This connection factory might be more easily covered by an integration test. You could set up a local socket server, record the input received and mock the output sent. Something like below:

public class TestServer {

    private int port;
    private String[] responses;
    private List<String> inputLines = new ArrayList<String>();

    public TestServer(int port, String ... responses) {
        this.port = port;
        this.responses = responses;
    }

    public void run() throws IOException {

        ServerSocket serverSocket = new ServerSocket(port);
        Socket clientSocket = serverSocket.accept();
        PrintWriter out = new PrintWriter(clientSocket.getOutputStream(), true);
        BufferedReader in = new BufferedReader(new InputStreamReader(clientSocket.getInputStream()));
        String inputLine;
        int i = 0;
        while ((inputLine = in.readLine()) != null) {
            inputLines.add(inputLine);
            out.println(responses[i++]);
        }
        out.close();
        in.close();
        clientSocket.close();
        serverSocket.close();
    }

    public List<String> getInputLines() {
        return inputLines;
    }
}

Your test code might look something like below:

// setup
String sentInput = "hello";
String sentOutput = "hi there!";
int port = 4444;
TestServer server = new TestServer(port, sentOutput);
server.run();

// test 
Connection connection = ConnectionFactory.createConnection(ConnectionType.TCP, "localhost", port);
// not sure of your Connection's API
connection.open();
String receivedOutput = connection.send(sentInput);
connection.close();

// verify
List<String> inputLines = server.getInputLines();
assertEquals(sentInput , inputLines.get(0)); 
assertEquals(sentOutput, receivedOutput);

Hope this helps.

EDIT: Ok, so I slightly misunderstood your question, sorry. Above approach is still a valid solution I think. It will allow you to keep your static factory methods and run an integration test. However, your desire to unittest your service method makes perfect sense. The solution you picked (overriding connectToHost in your service) seems pretty good to me. Another way to do it is like this:

Create an interface to wrap the dependency:

public interface ConnectionProvider {
    Connection provideConnection(ConnectionType connectionType);
}

Wrap the call to your factory and add a setter:

private ConnectionProvider connectionProvider = new ConnectionProvider() {
    public Connection provideConnection(ConnectionType connectionType) {
        return ConnectionFactory.createConnection(connectionType, "domain.com", 40203);
    }
};

public void setConnectionProvider(ConnectionProvider connectionProvider) {
    this.connectionProvider = connectionProvider;
}

Call your connectionProvider instead of directly calling the factory:

public void initConnectionPool(ConnectionType connectionType) {
    for (int i = 0; i < MAX_CONNECTIONS; i++) {
        connectionList.add(connectionProvider.provideConnection(connectionType));
    }
    // Some more code here to do further processing
    // ......
    // ......
}

In your unittest, use the setter to provide a mock.

Answer 3

I always make my singletons facades.

public interface IConnectionFactory //i know that it's not a correct java naming
{
     Connection createConnection(ConnectionType connectionType, String ipAddr, Integer port);
}

public class ConnectionFactory
{
    private IConnectionFactory _implementation;

    public static Connection createConnection(ConnectionType connectionType, String ipAddr, Integer port)
    {
        return _implementation.createConnection(connectionType, ipAdd, port);
    }

    //DO assign a factory before doing anything else.
    public static void AssignFactory(IConnectionFactory implementation)
    {
         _implementation = implementation;
    }
}

Doing it like this makes singletons flexible and you can easily switch implementations.

Answer 4

A pattern that might suit you is the singleton pattern, which give you the best of both worlds: static-like behaviour with instance methods. In its simplest form, it looks like this:

public class ConnectionFactory {

    private static ConnectionFactory INSTANCE = new ConnectionFactory();

    private ConnectionFactory() {} // private constructor

    public static ConnectionFactory getInstance() {
        return INSTANCE;
    }

    public void someMethod() {
        ...
    }
}

Callers use it like this:

ConnectionFactory.getInstance().someMethod();

java.util.Calendar is an example of a JDK class that uses this pattern

Answer 5

I think you should read this article: Static Methods are Death to Testability (Google Testing Blog).

Despite of your ConnectionFactory class doesn't maintain any state information, I would suggest you to create concrete class and go like this:

public class ConnectionFactory
{
    public Connection createConnection(ConnectionType connectionType, String ipAddr, Integer port)
    {
         //Some error checking
         switch(connectionType)
         {   
             case TCP:
                  return createTcpConnection(ipAddr, port);
             case UDP:
                  return createUdpConnection(ipAddr, port);
             case RTP:
                  return createRtpConnection(ipAddr, port);
             case SCTP:
                  return createRtpConnection(ipAddr, port);
             default:
                  break;
         }
    }

    // TcpConnection, RtpConnection, SctpConnection and UdpConnection implement interface Connection
    public Connection createTcpConnection()
    {
        Connection connection = new TcpConnection();
        ...
        return connection;
    }

    public Connection createUdpConnection()
    {
        Connection connection = new UdpConnection();
        ...
        return connection;
    }
    ...    
}

public class CommunicationService
{
    private ConnectionFactory connectionFactory;

    public CommunicationService()
    {
        this(new ConnectionFactory());
    }

    public CommunicationService(ConnectionFactory factory)
    {
        connectionFactory = factory;
    }

    public void initConnectionPool(ConnectionType connectionType)
    {
        for(int i = 0; i < MAX_CONNECTIONS; i++)
             connectionList.add(connectionFactory.createConnection(connectionType, "domain.com", 40203));
        ...
    }    
    ...
}

The rest of your code will not change at all, but for testing purposes you will be able to create TestConnectionFactory class:

public class TestConnectionFactory : ConnectionFactory
{
    public override Connection createTcpConnection()
    {
        ...
        return testTcpConnection;
    }

    public override Connection createUdpConnection()
    {
        ...
        return testUdpConnection;
    }
}

and use it for testing the CommunicationService like this:

CommunicationService service = new CommunicationService(new TestConnectionFactory());
// Tests
...

Answer 6

As far as testing is concerned, it's easy to solve

public class ConnectionFactory

    static boolean test = false;

    public static Connection createConnection(ConnectionType connectionType, String ipAddr, Integer port)

        if(test) ...
        else ...


public class ConnectionFactoryTest // in the same package
    public static void enableTest(){ ConnectionFactory.test=true; }

// in some other test classes
    ConnectionFactoryTest.enableTest();
    ConnectionFactory.createConnection(...);

The test flag isn't volatile. It's thread safe in production code. Most likely if(test) will be optimized off by JVM.

Answer 7

Make an immutable ConnectionFactory class initialised with a ConnectionType instance, IP address and port:

public class ConnectionFactory {
     private final ConnectionType connectionType;
     private final String ipAddr;
     private final Integer port;

     public ConnectionFactory(final ConnectionType connectionType) {
          this.connectionType = connectionType;
          this.ipAddr = ipAddr;
          this.port = port;
     }

     public Connection createConnection() {
        // TODO your code here
        ...  
     }
     ....
}

This way method initConnectionPool would take an instance of ConnectionFactory as an argument and you'd have no problems passing whatever mock/stub/concrete implementation you need during testing and other situations.

Answer 8

If you use a library like JMockIt then you can mock out static methods for unit testing.

Answer 9

I think its better to use static methods over here.

In test environment or any other environment - ConnectionFactory should be initialized using different set of properties.

So in effect you should have different set of properties (containing connectiontype, port etc) for different environments and you can init using appropriate one.