views:

116

answers:

4

I'm writing an RPC middleware in C++. I have a class named RPCClientProxy that contains a socket client inside:

class RPCClientProxy {
  ...
  private:
    Socket* pSocket;
  ...
}

The constructor:

RPCClientProxy::RPCClientProxy(host, port) {
  pSocket = new Socket(host, port);
}

As you can see, I don't need to tell the user that I have a socket inside.

Although, to make unit tests for my proxies it would be necessary to create mocks for sockets and pass them to the proxies, and to do so I must use a setter or pass a factory to the sockets in the proxies's constructors.

My question: According to TDD, is it acceptable to do it ONLY because the tests? As you can see, these changes would change the way the library is used by a programmer.

+3  A: 

What you describe is a perfectly normal situation, and there are established patterns that can help you implement your tests in a way that won't affect your production code.

One way to solve this is to use a Test Specific Subclass where you could add a setter for the socket member and use a mock socket in the case of a test. Of-course you would need to make the variable protected rather than private but that's probably no biggie. For example:

class RPCClientProxy 
{
    ...
    protected:
    Socket* pSocket;
    ...
};

class TestableClientProxy : public RPCClientProxy 
{
    TestableClientProxy(Socket *pSocket)
    {
        this->pSocket = pSocket;
    }
};

void SomeTest()
{
    MockSocket *pMockSocket = new MockSocket(); // or however you do this in your world.
    TestableClientProxy proxy(pMockSocket);
    ....
    assert pMockSocket->foo;
}

In the end it comes down to the fact that you often (more often than not in C++) have to design your code in such a way as to make it testable and there is nothing wrong with that. If you can avoid these decisions leaking out into the public interfaces that may be better sometimes, but in other cases it can be better to choose, for example, dependency inject through constructor parameters above say, using a singleton to provide access to a specific instance.

Side note: It's probably worth taking a look through the rest of the xunitpatterns.com site: there are a whole load of well established unit-testing patterns to understand and hopefully you can gain from the knowledge of those who have been there before you :)

jkp
This is probably the best route to go
Gutzofter
That won't work if there's no default constructor in RPCClientProxy.
Noah Roberts
@Noah Roberts: no sure it wont. It was an example to be adapted as required! You can still have a test specific subclass and implement a constructor that adds the extra parameter so there is nothing wrong with the idea.
jkp
It's an interesting method that I might have use for, but it does depend on there being access to at least one constructor that doesn't do a lot of work. In this case, depending on what the Socket's constructor does the system could already be failing when you call the (host, port) constructor. Thus I'd say the answer by Matthew should be preferred in a perfect world, and this one used when it can, if it has to be.
Noah Roberts
So the best choice here is to implement the necessary constructors as private and use them in the "friend" testable classes, right? And I need to create and interfaces to the mockable objects too.
Leandro
+3  A: 

I don't adhere to a certain canon i would say if you think you would benefit from testing through a mock socket the do it, you could implement a parallel constructor

RPCClientProxy::RPCClientProxy(Socket* socket) 
{
   pSocket = socket
}

Another option would be to implement a host to connect to for testing that you can configure to expect certain messages

Harald Scheirich
+3  A: 

Your issue is more a problem of design.

If you ever with to implement another behavior for Socket, you're toasted, as it involves rewriting all the code that created sockets.

The usual idea is to use an abstract base class (interface) Socket and then use an Abstract Factory to create the socket you wish depending on the circumstances. The factory itself could be either a Singleton (though I prefer Monoid) or passed down as arguments (according to the tenants of Dependency Injection). Note that the latter means no global variable, which is much better for testing, of course.

So I would advise something along the lines of:

int main(int argc, char* argv[])
{
  SocketsFactoryMock sf;

  std::string host, port;
  // initialize them

  std::unique_ptr<Socket> socket = sf.create(host,port);
  RPCClientProxy rpc(socket);
}

It has an impact on the client: you no longer hide the fact that you use sockets behind the scenes. On the other hand, it gives control to the client who may wish to develop some custom sockets (to log, to trigger actions, etc..)

So it IS a design change, but it is not caused by TDD itself. TDD just takes advantage of the higher degree of control.

Also note the clear resource ownership expressed by the use of unique_ptr.

Matthieu M.
I like the factory, but socket is not the only component of my middleware I'd like to mock. In the RPCClientProxy I have: RPCCallHash: When the client sends a request I need to put an entry in the hash that represents a running call; RPCCallQueue: When the server returns a response the middleware enqueues a response object. vector<RPCCallHandler*>: They are threads responsible for dequeue a response and call some callback function. The number of threads is configurable. So, what to you think? Would it be a good idea to make factories to all components from my middleware I'd like to mock?
Leandro
Just a note. You're not actually toasted in such situations. You can turn any constructor into an abstract factory assuming the point where you want to create the new abstraction is exactly the object that is being created by clients. See http://stackoverflow.com/questions/2884814/is-using-a-c-virtual-constructor-generally-considered-good-practice/2884907#2884907
Noah Roberts
Also, unique_ptr is C++0x. Besides that, this is of course the 'correct' answer.
Noah Roberts
@Noah: exact you can use the Handle/Body idiom to move use the factory, however this requires a "Singleton" factory with all the issues of using a global variables in multithreaded environment.
Matthieu M.
@Leandro: that's the issue with Dependency Injection, if you want to do it properly you end up passing a lot of items around. Although you could conceivably group them in a blob so as to pass only one object.
Matthieu M.
Singletons don't have the same issues in MT as global variables do. Singletons can be made MT safe if one needs to. Globals require all clients to use them in a MT safe manner.
Noah Roberts
@Noah: unless you provide a "private" global only accessibly through MT safe methods. The initialization / destruction of Singletons in MT environment (although definitely possible) is often implemented badly ;)
Matthieu M.
A: 

As others have pointed out, a factory architecture or a test-specific subclass are both good options in this situation. For completeness, one other possibility is to use a default argument:

RGCClientProxy::RPCClientProxy(Socket *socket = NULL)
{
    if(socket == NULL) {
        socket = new Socket();
    }
    //...
}

This is, perhaps somewhere between the factory paradigm (which is ultimately the most flexible, but more painful for the user) and newing up a socket inside your constructor. It has the benefit that existing client code doesn't need to be modified.

Barry Wark