c++unit-testingprivate-methodsnon-member-functions

Unit test private methods by making them free functions


In the 2017 cppcon videos, I came across a talk by Klaus Iglberger which was entitled "Free Your Functions!". In this talk, the speaker talked about how switching to free functions could easy up the process of testing private methods (See at 19:00). The idea is that you pull the private method out of the class (you make it a free function) and it becomes testable.

At first, I found the idea interesting, but then the more I thought about it, the less I understood how this is actually supposed to work. For example, let's say I have the following (dummy) class:

class SomeClass
{
public:
    SomeClass();
    ~SomeClass();

    void someTask();

private:

    void someComplexTask();
    void someOtherComplexTask();

};

void SomeClass::someTask()
{
    someComplexTask();
    someOtherComplexTask();
}

// private tasks implementations...

Then someComplexTask() and someOtherComplexTask() are private methods. This means that they are implementation details, i.e. that they can only be called inside SomeClass (or friends). It seems to me that if you make them free functions, yes they become testable, but they are no longer private, nor a mere implementation detail specific to SomeClass. In fact, they can be called from anywhere in the code...

So my question is: why is Mr. Iglberger's point valid?


Solution

  • This is a clear indication that you have a design flaw. If you have a private function that you need to test and you have to bend backwards to make it work then something is wrong. You have missed something. Your design doesn't work.

    His point is not to just make private functions free. He is not saying: "get all your private functions and make them free functions". He is saying that functionality that needs to be tested shouldn't be an implementation detail because if you need to test it that is an indication the functionality is useful.

    Please pay close attention to the transformation he does to the code:

    initial code:

    class X
    {
    public:
     void doSomething( ... ) {
        ...
        resetValues();
        ...
     }
     ...
    private:
     void resetValues() {
         for( int& value : values_ )
            value = 0;
     }
     std::vector<int> values_;
    };
    

    He pulls resetValues out of X but it makes it operate on a std::vector<T>, not on X:

    void resetValues( std::vector<int>& vec )
    {
     for( int& value : vec )
       value = 0;
    }
    

    Now resetValues is a functionality that can be reused and tested. As it truly has nothing to do with X, but with resetting all values of a vector it is a valid design to make it a free function instead of a private X method.

    I like how Rann Lifshitz put it in his comment:

    I think the better way to go here is to understand that some private functions are, in fact, common utility functions