views:

210

answers:

3

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.

+11  A: 

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.

Mark B
Agreed. You test the *interface*, not the internals.
Nathan Ernst
I think I am currently using pImpl in places where other techniques may be better suited. More specifically, I was using pImpl to hide the use of an external library that may be replaced in the future. I regarded this as an implementation detail and thought pImpl was a good fit but I guess there must be a better way to do it. (e.g Interface, Adapter, Strategy etc)But this has got me confused .. Can you give me an example of when pImpl is better suited than such patterns?
Rimo
@Rimo, the whole point of the `pImpl`, `pimpl`, read: http://en.wikipedia.org/wiki/Cheshire_Cat_Idiom_(programming_technique) is to *hide* implementation. from a testing perspective, you need to test does `x` input produce expected `y` output. How that comes about is an implementation detail, and is not your responsibility to test (from a unit-test perspective).
Nathan Ernst
+1 Test to the specification - not to what your understanding of the class.
David Relihan
+8  A: 

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.

tzaman
You are saying only the public methods of class A need to be tested. But don't you sometimes need to unit-test private helper methods in isolation? For instance the google test framework supports the FRIEND_TEST macro especially for this.
Rimo
@Rimo: can you use the techniques you use to test private methods of non-pimpl classes to test the pimpl class? Or am I missing something?
Michael Burr
+2  A: 

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.

Winston Ewert