There is only so much to be said on the principles vs. advice topic, and having said that, it’s time to go and learn from the advice. Except that, in some cases, the advice is bad. Specifically, when Uncle Bob was “demystifying” the Single Responsibility Principle at Hanselminutes, he (unintentionally) gave some examples of harmful coding practices. One of these leads back to functional programming, and the other to unreadable and therefore unmaintainable code. I will discuss the former here and the latter in the next post.
Bob Martin gives an example of an Employee class that has methods like Payroll(), WriteToDB(), GenerateReport(), etc. So he suggests to split it into EmployeePayrollCalculator, EmployeeReportGenerator, etc., because the method of calculating salary can change independently of persistence and report generation. The benefit of this separation somehow misses me. Sure, Uncle Bob explains that otherwise any change causes a major recompilation in C++, but really it’s only a relink. The harm, on the other hand, is obvious: It’s called the Functional Decomposition Antipattern.
A class represents a concept. That is why a class should have an obvious name. Design is about enabling oneself to program in the same terms one thinks in. Classes correspond to terms that are nouns, functions to terms that are verbs. A noun can have several verbs associated with it, that don’t necessarily all correspond to the same responsibility. EmployeePayrollCalculator is a bad class because it is a very artificial “noun”; it doesn’t represent a concept.
In too many cases, responsibilities can’t be separated for various reasons. Sometimes joining responsibilities in a class is dictated by an interface that joins them. You will say that this means the interface has been badly designed. But an interface corresponds to the needs of the code that uses it. For example, the article on SRP on objectmentor.com (where Uncle Bob is the president) gives the example of a modem class that has dialing / hanging-up functionality as well as sending / receiving functionality, and suggests splitting those into separate interfaces:
However, there are two responsibilities being shown here. The first responsibility is connection management. The second is data communication. The
hangupfunctions manage the connection of the modem, while the
recvfunctions communicate data.
Should these two responsibilities be separated? Almost certainly they should. The two sets of functions have almost nothing in common. They’ll certainly change for different reasons. Moreover, they will be called from completely different parts of the applications that use them. Those different parts will change for different reasons as well.
I can envision a function that dials up, sends data and hangs up. Why not? And if it does, a single interface would be better. Actually, the separation suggested in the article will usually be the case, but for a completely different reason: because
recv will comprise a more generic stream interface. This is because of abstraction levels, not responsibilities.
The other reason not to separate classes in such cases is that, contrary to the quotation above, both kinds of functionality can have a lot in common, like maybe a private function for low-level hardware access. The article later concedes that the separation may have to be between the interfaces only, not the classes, for “reasons having to do with the details of the hardware or OS”. Similar reasons will exist in most cases.
Moreover, some data will be common, too. If we replace the modem by a socket, we won’t be able to split the class in two, because the socket handle (or port number) will be common.
Another example given in the article is a rectangle class, which consists of geometric and GUI functionality. Accoring to Object Mentor, we should split it into GeometricRectangle, that has no GUI functionality, and GUIRectangle, that owns a GeometricRectangle. This is undoubtably correct, but why do we need the SRP to understand it? I could have given any number of reasons for this separation before I had heard of the SRP: That geometry and GUI are very different levels of abstraction, that geometry usually resides in a separate library, that some rectangles are never drawn, etc.
So is the SRP totally useless? No, but it should be applied differently. The similarity between the Employee class and Functional Decomposition is not accidental. It is because the Single Responsibility Principle is a good rule of thumb for splitting code into functions. Each function should have only one reason to change.
However, even when applying the SRP to functions, Uncle Bob takes it to the extreme. But that will be the subject of the next post.