Safe Refactoring to Support Testing

So that we can begin to add support for multiple channels, let’s figure out where it might be useful to verify functionality in the open function.

Near its end appears a number of calculations—total seconds, how many samples to write, where to start, and so on. After all these calculations is a for loop that appears to write samples to an output file. (This chunk of code appears highlighted in the earlier listing of open.) We’ll need to change a calculation or two plus fix the loop in order to support multiple channels.

The most interesting piece of code is the loop. Let’s write tests around that...but how? We’d need to set up lots of information to get that far in the open function.

Instead, let’s isolate the loop code to its own member function and test it directly.

Q.:

You’re going to change the code? Don’t you run the risk of breaking things?

A.:

Yes. This is how we will add a small test increment that covers the code we must change. Extracting a chunk of code into its method is one of the few code transformations that we can do in a very safe manner.

Q.:

It’s a bit of work to extract a function and put the prototype into the header. Isn’t there an easier way?

A.:

Method extract is the simplest approach we have. It would take hours to get the prior part of open tested.

Q.:

I now understand and buy into why it’s better to have testable code. But you’ll have to expose the method as public, a choice that many of my fellow programmers would find contemptible.

A.:

If absolutely required, you can employ other techniques that result in less-open code. For example, you can define the method as protected and then create a test derivative that exposes it as public. It seems like a lot of extra effort—and maintenance—for little gain. I prefer the simpler approach.

Remind your fellows that it’s better that we know the code works than worry about the unlikely scenario of exposed code getting abused.

You might also appease them by writing a prudent comment that explains why the function is exposed. Exposing a member might also highlight the design flaw that it exhibits feature envy. In turn, that exposure might prod someone to fix the design problem.

Our approach to extracting a function involves small, rote steps.

1. We type the call to the yet-to-be-extracted function where it belongs.

   wav/2/WavReader.cpp
   ​	uint32_t startingSample{
   ​	totalSeconds >= 10 ? 10 * formatSubchunk.samplesPerSecond : 0};
   *	writeSamples(out, data, startingSample, samplesToWrite, bytesPerSample);
   ​
   ​	rLog(channel, ​"writing %u samples"​, samplesToWrite);
   ​	​for​ (​auto​ sample = startingSample;
   ​	sample < startingSample + samplesToWrite;
   ​	sample++) {
   ​	​auto​ byteOffsetForSample = sample * bytesPerSample;
   ​	​for​ (uint32_t byte{0}; byte < bytesPerSample; byte++)
   ​	out.put(data[byteOffsetForSample + byte]);
   ​	}
   ​	rLog(channel, ​"completed writing %s"​, name.c_str());
   ​	descriptor_->add(dest_, name,
   ​	totalSeconds, formatSubchunk.samplesPerSecond, formatSubchunk.channels);

2. We add a corresponding function declaration before the open function by simply doing a copy/paste of the function call in the code. (We’ll move it out of the header once we get it compiling.) We type the function’s return type (void) and the braces. We keep it as a free function for now, which will allow us to get the parameter types correct without having to replicate that effort in the prototype.

   wav/2/WavReader.cpp
   ​	​void​ writeSamples(out, data, startingSample, samplesToWrite, bytesPerSample) {
   ​	}

3. We add type information to the signature for writeSamples. We use the compiler to let us know if the call to writeSamples doesn’t match the signature, and we fix problems when the compiler points to them. Once we get it right, we add the prototype to WavReader.h, and we scope the implementation of writeSamples to the class (WavReader::).

   wav/3/WavReader.cpp
   ​	​void​ WavReader::writeSamples(ofstream& out, ​char​* data,
   ​	uint32_t startingSample,
   ​	uint32_t samplesToWrite,
   ​	uint32_t bytesPerSample) {
   ​	}

   wav/3/WavReader.h
   ​	​public​:
   ​	​// ...​
   ​	​void​ writeSamples(std::ofstream& out, ​char​* data,
   ​	uint32_t startingSample,
   ​	uint32_t samplesToWrite,
   ​	uint32_t bytesPerSample);

4. We copy the for loop into the body of writeSamples and ensure it compiles. It might not compile if, for example, we forgot to pass a necessary parameter.

   wav/4/WavReader.cpp
   ​	​void​ WavReader::writeSamples(ofstream& out, ​char​* data,
   ​	uint32_t startingSample,
   ​	uint32_t samplesToWrite,
   ​	uint32_t bytesPerSample) {
   ​	rLog(channel, ​"writing %i samples"​, samplesToWrite);
   ​
   ​	​for​ (​auto​ sample = startingSample;
   ​	sample < startingSample + samplesToWrite;
   ​	sample++) {
   ​	​auto​ byteOffsetForSample = sample * bytesPerSample;
   ​	​for​ (uint32_t byte{0}; byte < bytesPerSample; byte++)
   ​	out.put(data[byteOffsetForSample + byte]);
   ​	}
   ​	}

   We remove the for loop code from the open member function.

   wav/4/WavReader.cpp
   ​	uint32_t startingSample{
   ​	totalSeconds >= 10 ? 10 * formatSubchunk.samplesPerSecond : 0};
   ​
   *	writeSamples(out, data, startingSample, samplesToWrite, bytesPerSample);
   ​
   ​	rLog(channel, ​"completed writing %s"​, name.c_str());
   ​
   ​	descriptor_->add(dest_, name,
   ​	totalSeconds, formatSubchunk.samplesPerSecond, formatSubchunk.channels);

   We compile and run any tests we have. We’ve successfully, and with reasonable safety, extracted a chunk of code to its own function. Extracting writeSamples has also increased the abstraction level of the open function.

   Sometimes you’ll encounter challenging compilation errors in your attempts to extract a function. If you ever feel like you must change lines of code to get your attempt to compile, you’ve likely extracted the wrong chunk of code. Stop and re-think your approach. Either change the scope of your extract or find a way to write a few more tests first.

Every paste from existing code introduces duplication. For that reason, many test-drivers consider copy/paste an “evil” mechanism. When crafting new code, you should maintain a mental stack of every paste operation and remember to pop off that stack by factoring out any remaining duplication.

Yet with legacy code, copy/paste is often your benevolent friend. Minimizing your actual typing, preferring instead to copy from existing structures, decreases your risk of making a dumb mistake when manipulating existing code. The steps taken here (which represent one possible approach) to extract the writeSamples member minimize typing and maximize points at which the compiler can help you catch mistakes.

We did not change any code within writeSamples. That means our only risk lies around calling the function properly. (If it had a nonvoid return, we’d also want to ensure the return value was handled appropriately.) If we didn’t intend to change any code in writeSamples, we wouldn’t worry about writing tests for it. We’d instead focus on writing tests around the passing of arguments to writeSamples. We’ll give that a shot a bit later.