For the sake of argument here\'s a sample function that prints contents of a giv

ID: 642522 • Letter: F

Question

For the sake of argument here's a sample function that prints contents of a given file line-by-line.

Version 1:

void printFile(const string & filePath) {
fstream file(filePath, ios::in);
string line;
while (std::getline(file, line)) {
cout << line << endl;
}
}
I know it is recommended that functions do one thing at one level of abstraction. To me, though code above does pretty much one thing and is fairly atomic.

Some books (such as Robert C. Martin's Clean Code) seem to suggest breaking the above code into separate functions.

Version 2:

void printFile(const string & filePath) {
fstream file(filePath, ios::in);
printLines(file);
}

void printLines(fstream & file) {
string line;
while (std::getline(file, line)) {
printLine(line);
}
}

void printLine(const string & line) {
cout << line << endl;
}
I understand what they want to achieve (open file / read lines / print line), but isn't it a bit of overkill?

The original version is simple and in some sense already does one thing - prints a file.

The second version will lead to a large number of really small functions which may be far less legible than the first version.

Wouldn't it be, in this case, better to have the code at one place?

At which point does the "Do One Thing" paradigm become harmful?

Explanation / Answer

Of course, this just begs the question "What is one thing?" Is reading a line one thing and writing a line another one? Or is copying a line from one stream to the other to be considered one thing? Or copying a file?

There is no hard, objective answer to that. It's up to you. You can decide. You have to decide. The "do one thing" paradigm's main goal is probably to produce code that's as easy to understand as possible, so you can use that as a guideline. Unfortunately, this isn't objectively measurable either, so have to rely on your gut feeling and the "WTF?" count in code review.

IMO a function consisting of only a single line of code is rarely ever worth the trouble. Your printLine() has no advantage over using std::cout << line << ' '1 directly. If I see printLine(), I have to either assume it does what its name says, or look it up and check. If I see std::cout << line << ' ', I know immediately what it does, because this is the canonical way of outputting the content of a string as a line to std::cout.

However, another important goal of the paradigm is to allow code reuse, and that's much more objective a measure. For example, in your 2nd version printLines() could easily be written so that it is an universally useful algorithm that copies lines from one stream to another:

void copyLines(std::istream& is, std::ostream& os)
{
std::string line;
while( std::getline(is, line) );
os << line << ' ';
}
}
Such an algorithm could reused in other contexts as well.

You can then put everything specific to this one use case into a function which calls this generic algorithm:

void printFile(const std::string& filePath) {
std::ifstream file(filePath.c_str());
printLines(file, std::cout);
}

Navigate

For the safety of the workers and the protection of the equipment in a water tre

For the sake of argument, let us assume that His Honor, Judge Sushi intends to r

Integrity-first tutoring: explanations and feedback only — we do not complete graded work. Learn more.

For the sake of argument here\'s a sample function that prints contents of a giv

Question

Explanation / Answer

Related Questions

Navigate