Unit Testing Private Method in Resource Managing Class (C++)

Question

I previously asked this question under another name but deleted it because I didn't explain it very well.

Let's say I have a class which manages a file. Let's say that this class treats the file as having a specific file format, and contains methods to perform operations on this file:

class Foo {
    std::wstring fileName_;
public:
    Foo(const std::wstring& fileName) : fileName_(fileName)
    {
        //Construct a Foo here.
    };
    int getChecksum()
    {
        //Open the file and read some part of it

        //Long method to figure out what checksum it is.

        //Return the checksum.
    }
};

Let's say I'd like to be able to unit test the part of this class that calculates the checksum. Unit testing the parts of the class that load in the file and such is impractical, because to test every part of the getChecksum() method I might need to construct 40 or 50 files!

Now lets say I'd like to reuse the checksum method elsewhere in the class. I extract the method so that it now looks like this:

class Foo {
    std::wstring fileName_;
    static int calculateChecksum(const std::vector<unsigned char> &fileBytes)
    {
        //Long method to figure out what checksum it is.
    }
public:
    Foo(const std::wstring& fileName) : fileName_(fileName)
    {
        //Construct a Foo here.
    };
    int getChecksum()
    {
        //Open the file and read some part of it

        return calculateChecksum( something );
    }
    void modifyThisFileSomehow()
    {
        //Perform modification

        int newChecksum = calculateChecksum( something );

        //Apply the newChecksum to the file
    }
};

Now I'd like to unit test the calculateChecksum() method because it's easy to test and complicated, and I don't care about unit testing getChecksum() because it's simple and very difficult to test. But I can't test calculateChecksum() directly because it is private.

Does anyone know of a solution to this problem?

Homes

Answer 1

Basically, it sounds like you want a mock to make unit testing more feasible. The way you make a class targetable for unit testing independantly of the object hierarchy and of external dependencies is through dependency injection. Create a class "FooFileReader" like so:

class FooFileReader
{
public:
   virtual std::ostream& GetFileStream() = 0;
};

Make two implementations, one that opens a file and exposes it as a stream (or an array of bytes if that is what you really need.) The other is a mock object that just returns test data designed to stress your algorithm.

Now, make the foo constructor have this signature:

Foo(FooFileReader* pReader)

Now, you can construct foo for unit testing by passing a mock object, or construct it with a real file using the implementation that opens the file. Wrap construction of the "real" Foo in a factory to make it simpler for clients to get the correct implementation.

By using this approach, there's no reason not to test against " int getChecksum()" since its implementation will now use the mock object.

Duodecimal

+1 I suppose this could work for this specific scenario -- but I'm hoping for a more general answer that would work for any kind of resource managing class, be it registry key, file, pipe, etc. Sorry I wasn't quite clear enough in the question :( – Homes

Answer 2

One way would be to extract the checksum method out into its own class and have a public interface with which to test.

Doctrinaire

How is this any better than making the checksum method public in the first place? – Homes

Of course you wouldn't want checksum to be public in Foo; it just doesn't fit the interface of the class. My thinking was that it really isn't the responsibility of Foo to calculate the checksum and that it should be pulled out into it's own class. Having done that, you would test it the same way you'd use it from Foo. – Doctrinaire

Answer 3

The simple, direct answer is to make your unit-test class a friend of the class under test. This way, the unit test class can access calculateChecksum() even though it's private.

Another possibility to look at is that Foo appears to have a number of unrelated responsibilities, and might be due for re-factoring. Quite possibly, computing a check sum shouldn't really be part of Foo at all. Instead, calculating a checksum might be better off as a general purpose algorithm that anybody can apply as needed (or possibly, kind of the reverse -- a functor to use with another algorithm like std::accumulate).

Changsha

+1 I like this idea... is there any way to do this with boost::test? – Homes

@BillyONeal: You'd probably be better off asking that as a separate question -- offhand, I'm not sure, but somebody else (who's less likely to see these comments) very well might. – Changsha

Answer 4

I'd start by extracting the checksum-calculation code into it's own class:

class CheckSumCalculator {
    std::wstring fileName_;

public:
    CheckSumCalculator(const std::wstring& fileName) : fileName_(fileName)
    {
    };

    int doCalculation()
    {
        // Complex logic to calculate a checksum
    }
};

This makes it very easy to test the check-sum calculation in isolation. You could however take it one step further and create a simple interface:

class FileCalculator {

public:
    virtual int doCalculation() =0;
};

And the implementation:

class CheckSumCalculator : public FileCalculator {
    std::wstring fileName_;

public:
    CheckSumCalculator(const std::wstring& fileName) : fileName_(fileName)
    {
    };

    virtual int doCalculation()
    {
        // Complex logic to calculate a checksum
    }
};

And then pass the FileCalculator interface to your Foo constructor:

class Foo {
    std::wstring fileName_;
    FileCalculator& fileCalc_;
public:
    Foo(const std::wstring& fileName, FileCalculator& fileCalc) : 
        fileName_(fileName), 
        fileCalc_(fileCalc)
    {
        //Construct a Foo here.
    };

    int getChecksum()
    {
        //Open the file and read some part of it

        return fileCalc_.doCalculation( something );
    }

    void modifyThisFileSomehow()
    {
        //Perform modification

        int newChecksum = fileCalc_.doCalculation( something );

        //Apply the newChecksum to the file
    }
};

In your real, production code you would create a CheckSumCalculator and pass it to Foo, but in your Unit Test code you could create a Fake_CheckSumCalculator (that, for example always returned a known predefined checksum).

Now, even though Foo has a dependency on CheckSumCalculator, you can construct and Unit Test these two classes in complete isolation.

Effect

Yes, I suppose this works, but does this not break the entire point of making the checksum calculation private in the first place? It seems this is a very roundabout way of making the function public. How does this improve encapsulation over just making this a public function? – Homes

BillyONeal: private protects data, operations without data are fangless and need no protection. – Pentheus

If I had to choose a design trade-off, I would much rather have smaller classes that can be instantiated and tested independently in a test harness vs. larger classes with many private methods that are difficult to test. This may mean that some of your library's "inner workings" are now exposed to the world, but if the result is better tested code, it's usually worth it. – Effect

Answer 5

#ifdef TEST
#define private public
#endif

// access whatever you'd like to test here

Wavelength

If the test code and the actual code were compiled together this would work. Unfortunately, the code I'm testing is in a static library, and the unit tests are in an .EXE. :( – Homes

It doesnt matter. private/public access is only enforced by the compiler, and doesnt exist in run time. so it would work just the same. Just use this define before the include of the header you're using, and you're all set. – Wavelength

at least it'll usually work. In theory, the compiler would be free to do things so it didn't though. Anywhere there's an access specifier, it's allowed to rearrange the order of items in memory, so it could, for example, put all the public items together, then all the protected and finally all the private. That's mostly theoretical though... – Changsha

Answer 6

Well the preferred way in C++ for file IO is by stream. So in the example above it would make much more sense maybe to inject a stream instead of a file name. For example,

Foo(const std::stream& file) : file_(file)

In that way you could use std::stringstream for unit testing and have full control of the test.

If you do not want to use streams then the standard example of a RAII pattern defining a File class can be used. The "simple" way to proceed then is to create a pure virtual interface class File and then an implementation of the interface. The Foo class would then use the interface class File. For example,

Foo(const File& file) : file_(file)

Testing is then done by simply creating a simple subclass to File and injecting that instead (stubbing). Creating a mock class (see Google Mock for example) can also be done.

However, you probable want to unit test the File implementation class as well and since it is RAII, it in turn needs some dependency injection. I usually try to create an pure virtual interface class that just provide the basic C file operations (open, close, read, write, etc. or fopen, fclose, fwrite, fread, etc). For example,

class FileHandler {
public:
    virtual ~FileHandler() {}
    virtual int open(const char* filename, int flags) = 0;
    // ... and all the rest
};

class FileHandlerImpl : public FileHandlerImpl {
public:
    virtual int open(const char* filename, int flags) {
        return ::open(filename, flags);
    }
    // ... and all the rest in exactly the same maner
};

This FileHandlerImpl class is so simple that I do not unit test it. However, the benefit is that using it in the constructor of the FileImpl class I can easily unit test the FileImpl class. For example,

FileImple(const FileHandler& fileHandler, const std::string& fileName) : 
    mFileHandler(fileHandler), mFileName(fileName)

The only drawback so far is that the FileHandler has to be passed around. I have thought of using the FileHandle interface to actually provide a static instance set/get-methods that can be used to get a single global instance of a FileHandler object. Although not really a singleton and thus still unit testable it is not a elegant solution. I guess passing a handler around is the best option right now.

Encircle