The pImpl idiom and Testability

Question

The pImpl idiom in c++ aims to hide the implementation details (=private members) of a class from the users of that class. However it also hides some of the dependencies of that class which is usually regarded bad from a testing point of view.

For example if class A hides its implementation details in Class AImpl which is only accessible from A.cpp and AImpl depends on a lot of other classes, it becomes very difficult to unit test class A since the testing framework has no access to the methods of AImpl and also no way to inject dependency into AImpl.

Has anyone come across this problem before? and have you found a solution?

-- edit --

On a related topic, it seems that people suggest one should only test public methods exposed by the interface and not the internals. While I can conceptually understand that statement, I often find that I need to test private methods in isolation. For example when a public method calls a private helper method that contains some non trivial logic.

Answer 1

Why does the unit test need access to the internals of A's implementation?

The unit test should be testing A, and as such should only care about input and output of A directly. If something isn't visible in A's interface (either directly or indirectly) then it may not actually need to be part of Aimpl at all (as its results aren't visible to the external world).

If Aimpl generates side effects you need to test, that indicates you should be taking a look at your design.

Answer 2

The idea behind pimpl is to not so much to hide implementation details from classes, (private members already do that) but to move implementation details out of the header. The problem is that in C++'s model of includes, changing the private methods/variables will force any file including this file to be recompiled. That is a pain, and that's why pimpl seeks to eliminate. It doesn't help with preventing dependencies on external libraries. Other techniques do that.

Your unit tests shouldn't depend on the implementation of the class. They should verify that you class actually acts as it should. The only thing that really matter is how the object interacts with the outside world. Any behavior which your tests cannot detect must be internal to the object and thus irrelevant.

Having said that, if you find too much complexity inside the internal implementation of a class, you may want to break out that logic into a separate object or function. Essentially, if your internal behavior is too complex to test indirectly, make it the external behavior of another object and test that.

For example, suppose that I have a class which takes a string as a parameter to its constructor. The string is actual a little mini-language that specifies some of the behavior the object. (The string probably comes from a configuration file or something). In theory, I should be able to test the parsing of that string by constructing different objects and checking behavior. But if the mini-language is complex enough this will be hard. So, I define another function that takes the string and returns a representation of the context (like an associative array or something). Then I can test that parsing function separately from the main object.

Answer 3

If you're doing dependency injection right, any dependencies class A as should be being passed in through its public interface - if your pImpl is interfering with your testing because of dependencies, it would seem that you're not injecting those dependencies.

Unit testing should only be concerned with the public interface that class A is exposing; what A does internally with the dependencies isn't your concern. As long as everything is being injected properly, you should be able to pass in mocks without needing to worry about A's internal implementation. In a sense, you could say testability and proper pImpl go hand-in-hand, in that a non-testable implementation is hiding details that shouldn't be hidden.

Answer 4

The pImpl idiom makes testing far easier. It's strange enough to see a set of answers on the theme of "don't test the implementation" to motivate answering so long after the OP.

In usual, non-pimpl based C++ you have a class with public and private fields. Public fields are easy to test, private fields somewhat more tedious. The division between public and private is important though, since it decreases the width of the api and usually makes later changes easier.

When using this idiom a better option is available. You can have exactly the same "public" interface as with a single class, but now there's only one private field containing a pointer of some sort, e.g.

class my_things
{
  public:
    my_things();
    ~my_things();
    void do_something_important(int);
    int also_this();
  private:
    struct my_things_real;
    std::unique_ptr<my_things_real> state;
};

The my_things_real class is expected to be visible in the same source file as the destructor of the externally visible class, but not in the header. It isn't part of the public interface, so all the fields can be public.

void my_things::do_something_important(int x) { state->doit(x); } // etc

class my_things_real // I'd probably write 'struct'
{
  public:
    int value;
    void doit(int x) { value = x; }
    int getit() { return value; }
};

Unit tests are then written against the real class. Test as much or as little of it as you like. I've deliberately called it "real" instead of "impl" to help ensure that it isn't mistaken for a mere implementation detail.

Testing this class is very easy since all the fields are public. The external interface is very small since it's defined by the other class. The wafer-thin translation layer is difficult to get wrong, but you're still welcome to test through the external api as well. This is a clear win from more significantly separating interface and implementation.

On a vaguely related note, it strikes me as absurd that so many otherwise coherent people advocate skipping unit testing for anything that is not readily accessible through the external API. The lowest level functions are hardly immune to programmer errors. Testing to verify that the api is usable is both important and orthogonal to verifying that the implementation details are correct.

Answer 5

The unit testing should put the implementation class thru its paces. Once the PIMPL class is in the picture, you are already into the realm of "integration" - and hence U/T does not apply as such. PIMPL is all about hiding the implementation - you are not supposed to know the class setup of the implementation.