Signs Your Class Secretly Has Too Many Responsibilities
A practical test for spotting Single Responsibility Principle violations in real code, beyond the obvious if-else chain: multiple branches and a monster method hiding in plain sight.
The previous post landed on one test for spotting a class with too many responsibilities: why would I need to open this class again? If the answer keeps landing on the same class for reasons that have nothing to do with each other, that class is doing more than one job.
That test is easy to state and harder to apply cold, on real code you didn't just write yourself. Here are two places this violation actually shows up in practice, roughly in the order you'll run into them. A third, and the most common one of all, gets its own post right after this.
Sign 1: Multiple If-Else Branches, With One Exception
The fly() method from the previous post is exactly what this sign looks like: one method, one branch per bird type. The one thing worth adding here is the exception, because it's the part people forget.
Multiple branches are usually a violation of SRP. Not always. If the branches are part of an algorithm, part of the actual business logic, they're not a violation at all. Take checking whether a year is a leap year:
bool isLeapYear(int year) {
if (year % 100 == 0) {
return false;
} else if (year % 4 == 0) {
return true;
} else {
return false;
}
}There are multiple branches here too. But this isn't SRP breaking down. It's just what a leap-year check is. You cannot write this logic without some branching. The branches aren't standing in for "different types of things being handled by one class." They're one algorithm, doing the one job its name says it does.
Compare that to the fly() method from the previous post. Its branches don't compute one answer, they select entirely different behavior per bird type. That's the real tell. Branches inside one self-contained calculation are fine. Branches that exist only to ask "which type is this?" and then do something structurally different for each answer are the SRP violation.
Sign 2: The Monster Method
A monster method is a method that does a lot more than its name says it does. The name promises one thing. The body delivers several.
Here's a saveToDatabase method that looks reasonable at first glance:
void saveToDatabase(const User& user, const Database& db) {
std::string query = "INSERT INTO users VALUES ('" + user.email + "', '" + user.name + "')";
Database connection;
connection.setUrl(db.url);
connection.setPassword(db.password);
connection.execute(query);
}The name says: save a user to the database. Now look at what it actually does.
- It builds a raw SQL query string.
- It creates a database connection and configures it.
- It runs the query.
That's three separate jobs wearing the name of one. And the problem isn't just that the method is doing extra work. Both of those extra jobs will be needed again, somewhere else in the codebase, by code that has nothing to do with saving a user.
Someday you'll need that exact same query, or that exact same connection, from a completely different place. When that day comes, you copy the code instead of reusing it.
That's a second, related principle showing up uninvited: DRY, Don't Repeat Yourself. Every place that needs a query or a connection either repeats this code or reimplements it, and now the same bug has to be fixed in every copy.
The fix is to pull each job out into its own place:
std::string createUserQuery(const User& user) {
return "INSERT INTO users VALUES ('" + user.email + "', '" + user.name + "')";
}
Database createConnection(const Database& db) {
Database connection;
connection.setUrl(db.url);
connection.setPassword(db.password);
return connection;
}
void saveToDatabase(const User& user, const Database& db) {
std::string query = createUserQuery(user);
Database connection = createConnection(db);
connection.execute(query);
}saveToDatabase now does exactly what its name says: it saves a user, by calling the two things it needs. Nothing here is thrown away, the query still has to be built and the connection still has to be created. The only change is where that code lives. Now createUserQuery and createConnection can be called from anywhere else in the codebase that needs them, with zero duplication.
This is where SRP violations show up most often in real code: not as an obvious if-else chain, but as duplicated code, because a job that should have lived in its own method or class got buried inside a bigger one instead.
There's one more sign left, and it's the one that hides in plain sight in almost every real codebase, a package everyone has seen and nobody questions. The next post covers it, and the surprising case where the exact same mistake lives inside a language's own standard library.
Keep reading