Reading from files, single responsibility principle, and testability

Say that your program needs to read some kind of structured data from a file. In a object oriented language the obvious thing to do is encapsulate the parser and the storage of its results in a class. Perhaps something like:

class DataReader { public: DataReader(std::string filename) { std::ifstream in("filename") if ( in.good() ) { parse(in); } } //... }

Indeed some of the legacy code my project calls had one of these as the main access point for a bunch of important functionality, and I've been asked to extend it and authorized to do some re-factoring along the way. Whoohoo!

But I can't afford to break existing code (or at least I have to know what I'm breaking and why...), which means I need some detailed test. Yes. It doesn't have a test suite. I mentioned that it was legacy code, didn't I? And that is where the problems start, because how do you write a self-contained test for DataReader?

• Actually creating a (or more likely several) test files break self-containment and means your test code needs to know where to look for the files, and test can fail for reason that have nothing to do with DataReader. Yuck!
• You could encode the test input as strings in your tests, create and write a temporary file, run the test on that file, then clean up the temporary file. That solves the self-containment and "where to find it" problems and doesn't clutter up the disk with a large number of similar test files. But it still breaks if there is a problem with the disk. More problematically all the set-up and tear-down code is a place to write bugs, and interferes with the clarity of your test. Still pretty bad.
• The third option is to refactor to make it clear that opening the file and dealing with the contents are separate "thing"s for the purposes of the Single Responsibility Principle. The class should take a stream not a filename. Now to avoid breaking existing code we can relax our pedantic insistence of separation of concerns by leaving the existing c'tor as a convenience but defering most of the work to a new c'tor that takes a stream. Something like: class DataReader { public: DataReader(std::istream in) { if (in.good()) { parse(in); } } DataReader(std::string filename) : Datareader(std::ifstream(filename)) {} //... } Then we use a std::istringstream in the test eliding much of the setup and all of the tear-down.