Mondo Extracto
We need to figure out where within the open function to call size. Our call could go near the end of open, immediately before the code sends a message to the descriptor object. But since size re-opens a file, we need to ensure that the new WAV file gets closed first.
Unfortunately, the structure of open presents quite a challenge. Code all the way up to the call to the descriptor is riddled with file reads and writes. Writing a test able to execute the entire function remains fairly challenging. (We could pass a real, vetted WAV file to open, but that would net us a slow and dependent test.)
Instead, we refactor open with the goal of deriving some functions that we might stub or mock. After about a dozen minutes of generally safe function extract operations, our code looks much healthier. They still contain ugly spots, but our functions are getting close to being readily digestible.
| wav/13/WavReader.cpp | |
| | void WavReader::open(const std::string& name, bool trace) { |
| | rLog(channel, "opening %s", name.c_str()); |
| | |
| | ifstream file{name, ios::in | ios::binary}; |
| | if (!file.is_open()) { |
| | rLog(channel, "unable to read %s", name.c_str()); |
| | return; |
| | } |
| | |
| | ofstream out{dest_ + "/" + name, ios::out | ios::binary}; |
| | |
| | FormatSubchunk formatSubchunk; |
| | FormatSubchunkHeader formatSubchunkHeader; |
| | readAndWriteHeaders(name, file, out, formatSubchunk, formatSubchunkHeader); |
| | |
| | DataChunk dataChunk; |
| | read(file, dataChunk); |
| | |
| | rLog(channel, "riff header size = %i" , sizeof(RiffHeader)); |
| | rLog(channel, "subchunk header size = %i", sizeof(FormatSubchunkHeader)); |
| | rLog(channel, "subchunk size = %i", formatSubchunkHeader.subchunkSize); |
| | rLog(channel, "data length = %i", dataChunk.length); |
| | |
| | auto data = readData(file, dataChunk.length); // leak! |
| | |
| | writeSnippet(name, file, out, formatSubchunk, dataChunk, data); |
| | } |
| | |
| | void WavReader::read(istream& file, DataChunk& dataChunk) { |
| | file.read(reinterpret_cast<char*>(&dataChunk), sizeof(DataChunk)); |
| | } |
| | |
| | char* WavReader::readData(istream& file, int32_t length) { |
| | auto data = new char[length]; |
| | file.read(data, length); |
| | //file.close(); // istreams are RAII |
| | return data; |
| | } |
| | |
| | void WavReader::readAndWriteHeaders( |
| | const std::string& name, |
| | istream& file, |
| | ostream& out, |
| | FormatSubchunk& formatSubchunk, |
| | FormatSubchunkHeader& formatSubchunkHeader) { |
| | RiffHeader header; |
| | file.read(reinterpret_cast<char*>(&header), sizeof(RiffHeader)); |
| | // ... |
| | } |
| | |
| | void WavReader::writeSnippet( |
| | const string& name, istream& file, ostream& out, |
| | FormatSubchunk& formatSubchunk, |
| | DataChunk& dataChunk, |
| | char* data |
| | ) { |
| | uint32_t secondsDesired{10}; |
| | if (formatSubchunk.bitsPerSample == 0) formatSubchunk.bitsPerSample = 8; |
| | uint32_t bytesPerSample{formatSubchunk.bitsPerSample / uint32_t{8}}; |
| | |
| | uint32_t samplesToWrite{secondsDesired * formatSubchunk.samplesPerSecond}; |
| | uint32_t totalSamples{dataChunk.length / bytesPerSample}; |
| | |
| | samplesToWrite = min(samplesToWrite, totalSamples); |
| | |
| | uint32_t totalSeconds{totalSamples / formatSubchunk.samplesPerSecond}; |
| | |
| | rLog(channel, "total seconds %u ", totalSeconds); |
| | |
| | dataChunk.length = dataLength( |
| | samplesToWrite, |
| | bytesPerSample, |
| | formatSubchunk.channels); |
| | out.write(reinterpret_cast<char*>(&dataChunk), sizeof(DataChunk)); |
| | |
| | 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); |
| | |
| | //out.close(); // ostreams are RAII |
| | } |
Since writeSnippet now takes an input stream and an output stream, it no longer depends upon the file system. Writing a test for it, and perhaps also for read and readData, now seems reasonable. We’d want to refactor readAndWriteHeaders (not fully shown) into more manageable chunks before writing tests, but doing so wouldn’t take long.
We even uncovered a likely memory leak. Breaking code into smaller functions can make defects obvious.
As we refactored, we deleted the “to-do” comments and commented-out calls to close the streams. We’ll need to revisit our choice shortly, but it is not necessary to explicitly close a file (std::ofstream supports RAII), and close is not part of the std::ostream interface. While our analysis is probably sufficient, now is the time to run whatever other tests we have, manual or automated.
Are we ready yet to add code to support our file size story? A little more refactoring might make it even simpler, but let’s see what we can do to test writeSnippet now that it’s a small, focused function. We’ll write a few tests that characterize its various behaviors.