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.

Adoree

Your ConnectionFactory is already a concrete (non-abstract) class. The opposite of static would be 'dynamic', although I don't often see it used in this context. A class with only static methods is commonly referrec to as a 'utility class'. – Disconnected

Answer 1

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

Bricky

Answer 2

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
...

Subdual

The blogger is wrong. Mocking a static method doesn't require mocking the entire call graph. Mocking ConnectionFactory does not require mocking sock APIs. Static or not, the complexity is the same. – Brewington

Yes static methods are really a pain when it comes to testing. I was thinking in the lines of your solution by making the ConnectionFactory a concrete class by implementing an interface and have a TestConnectionFactory for unit tests implementing the same interface and injecting it as a singleton in the services. – Adoree

@Prasanna, interface is even better. Anyway I would use a concrete class. Decide what will be best in your case. – Subdual

@irreputable, I will remain at the opinion that testing with static methods involved is more painful. – Subdual

there's no difference. it's one line to switch to the mock impl. – Brewington

@irreputable, your solution makes the ConnectionFactory test-aware what is not good in my opinion. – Subdual

The problems with testing do not arise from static methods, but from static state. – Disconnected

Answer 3

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.

Kironde

Answer 4

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.

Furculum

Why in the age of IoC frameworks like Guice would a singleton facade be a good alternative? – Andorra

Why? Because all applications don't use IoCs. Especially legacy applications. Facades makes singletons more plainless. – Furculum

Setup is usually made BEFORE any other threads are started. But for clarity I removed the check in createConnection method. – Furculum

Answer 5

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.

Andorra

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.

Brewington

Answer 7

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

Enthymeme

Answer 8

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.

Disconnected

I did not want to test ConnectionFactory but the method in CommunicationService that is tightly coupled with the static method call in ConnectionFactory – Adoree

Answer 9

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);
        }
    }
}

Coif