February 1, 2011
How to open file in testable way?
One of the most common problems in Test-Driven Development are parts of code which need to make some calls to your system environment. In general in such situation you are forced to use some global functions which are really hard to test or mock. So lets look on some code and lets try to make it more testable.
#include <fstream>
#include <string>
double calculate(const std::string& filepath) const
{
std::ifstream file(filepath);
double sum = 0;
while (file.good())
{
double number;
file >> number;
sum += number;
}
return sum;
}
This is very easy piece of code – a function which calculates sum of doubles from given file. Code is easy but writing tests for it is rather hard… or maybe even impossible (well it is possible to test it using some preprocessor magic but this is my last resort).
We can split this code into some sections:
#include <fstream>
#include <string>
double calculate(const std::string& filepath) const
{
// open file stream
std::ifstream file(filepath);
// load numbers from stream and calculate sum
double sum = 0;
while (file.good())
{
double number;
file >> number;
sum += number;
}
// return result, file is closed in destructor
return sum;
}
We know that std::ifstream inherit from std::istream class. So we could rewrite this code as:
#include <fstream>
#include <string>
double calculate(const std::string& filepath) const
{
// open file stream
std::auto_ptr<std::istream> stream(new std::ifstream(filepath));
double sum = 0;
// load numbers from stream and calculate sum
while (stream->good())
{
double number;
(*stream) >> number;
sum += number;
}
// return result, auto_ptr destructor destroy istream object
// which destroys ifstream (virtual destructor)
return sum;
}
We almost make our code free of calls to std::ifstream. The only thing left is creation of std::ifstream object. But we could refactor it to:
#include <fstream>
#include <string>
class FileSystem
{
public:
virtual std::auto_ptr<std::istream> open(const std::string& filePath) const
{
return std::auto_ptr<std::istream>(new std::ifstream(filepath));
}
};
class Calculator
{
public:
Calculator(FileSystem* filesystem) : filesystem(filesystem) {}
double calculate(const std::string& filepath) const
{
// open file stream
std::auto_ptr<std::istream> stream = filesystem->open(filepath);
double sum = 0;
// load numbers from stream and calculate sum
while (stream->good())
{
double number;
(*stream) >> number;
sum += number;
}
// return result, auto_ptr destructor destroy istream object
// which destroys ifstream (virtual destructor)
return sum;
}
private:
FileSystem* filesystem;
};
It is much better now! Code for calculate is free of global/static function calls so it is testable. The only think we have to do (to test calculate method) is to inject double of FileSystem (using polymorphism – then FileSystem::open method need to be virtual, or by making calculate method template – then dynamic polymorphism is not needed).
Of course we need to inject FileSystem in our production code but this is not as bad as someone may think. In my opinion it force programmer to think about design of his/her code instead of using global/static function calls where ever someone need to.
Above technique is called Dependency Injection (DI) and it is worth learning because it solves great number of problems when you have to make your code testable.
I hope you enjoy this reading… even if not I am waiting for your comments.
Happy testing!
If calculate took an istream & instead of a filename, you could avoid a lot of this trouble. You could then just build a string stream and test your calculator that way.
Any time you see yourself constructing a type locally, ask yourself if you would ever want to mock that variable. If you would want to mock it, then it shouldn’t be constructed locally, but passed in as a parameter at some point. The passing could happen during Calculator’s constructor call, or to the call to calculate itself.
Thanks for your comment Ben!
Your proposal for changing this code is very good – I will do that if I will implement or refactor such code. But the idea for this post was to show how not to change function type. Sometimes it is just needed to have such function which takes file path instead of stream and I was trying to show how to handle it. Your proposal makes code free of opening file (this is very good because it is not needed in such code like this) but I need to have opening file to show how you can handle such situation. Maybe code was to trivial but I want it to be as easy as possible.
Thanks one again for sharing your idea.