Single responsibility principle - am I overusing it?

Question

For reference - http://en.wikipedia.org/wiki/Single_responsibility_principle

I have a test scenario where in one module of application is responsible for creating ledger entries. There are three basic tasks which could be carried out -

• View existing ledger entries in table format.
• Create new ledger entry using create button.
• Click on a ledger entry in the table (mentioned in first pointer) and view its details in next page. You could nullify a ledger entry in this page.

(There are couple more operation/validations in each page but fore sake of brevity I will limit it to these)

So I decided to create three different classes -

• LedgerLandingPage
• CreateNewLedgerEntryPage
• ViewLedgerEntryPage

These classes offer the services which could be carried out in those pages and Selenium tests use these classes to bring application to a state where I could make certain assertion.

When I was having it reviewed with on of my colleague then he was over whelmed and asked me to make one single class for all. Though I yet feel my design is much clean I am doubtful if I am overusing Single Responsibility principle

Answer 1

Citing @YannisRizos here:

That's an instinctual approach and it's probably correct, as what's more probable to change is the business logic of managing ledgers. If that happens, it'd be a lot easier to maintain one class than three.

I disagree, at least now, after about 30 years of programming experience. Smaller classes IMHO are better to maintain in almost every case I can think of in cases like this one. The more functions you put into one class, the less structure you will have, and the quality of your code degrades - each day a little bit. When I understood Tarun correctly, each of these 3 operations

LedgerLandingPage

CreateNewLedgerEntryPage

ViewLedgerEntryPage

is a use case, each of these classes consist of more than one function to perform the related task, and each one can be developed and tested separately. So creating classes for each of them groups things together which belong together, making it much easier to identify the part of the code to be changed if something is to change.

If you have common functionalities between those 3 classes, they belong either in a common base class, the Ledger class itself (I assume you have one, at least, for the CRUD operations) or in a separate helper class.

If you need more arguments for creating lots of smaller classes, I recommend to have a look into Robert Martin's book "Clean Code".

Answer 2

There isn't a definitive implementation of the Single Responsibility Principle, or any other principle. You should decide based on what's more probable to change.

As @Karpie writes in an earlier answer:

You only need one class, and the single responsibility of that class is to manage ledgers.

That's an instinctual approach and it's probably correct, as what's more probable to change is the business logic of managing ledgers. If that happens, it'd be a lot easier to maintain one class than three.

But that should be examined and decided per case. You shouldn't really care for concerns with minuscule possibility of change and concentrate applying the principle on what's more likely to change. If the logic of managing ledgers is a multilayer process and layers are prone to change independently or are concerns that should be separate in principle (logic and presentation), then you should use separate classes. It's a game of probability.

A similar question on the subject of overusing design principles, that I think you'd find interesting: Design patterns: when to use and when to stop doing everything using patterns

Answer 3

Let's examine these classes:

• LedgerLandingPage: The role of this class is to show a list of ledgers. As the application grows, you will probably want to add methods for sorting, filtering, highliting and transparently fuse in data from other data sources.

• ViewLedgerEntryPage: This page shows the ledger in detail. Seems pretty straight forward. As long as the data is straight forward. You can nullify the ledger here. You could also want to correct it. Or comment it, or attach a file, receipt or external booking number or whatever. And once you allow editing, you definitely want to show a history.

• CreateLedgerEntryPage: If the ViewLedgerEntryPage has full editing capability, the responsibility of this class can possibly be fulfilled by editing a "blank entry", which may or may not make sense.

Theses examples may seem a little far fetched, but the point is, that from a UX we're talking features here. A single feature is definitely a a distinct, single responsibility. Because the requirements to that feature can change and the requirements of two different features can change into two opposing directions, and then you wish you had stuck with the SRP from the beginning.
But conversely, you can always slap a facade onto three classes, if having a single interface is more convenient for whatever reason you can think of.

---

There is such a thing as SRP overuse: If you require a dozen classes to read the contents of a file, it's rather safe to assume you're doing just that.

If you look at the SRP from the other side, it actually says, that a single responsibility should be taken care of by a single class. Please note however, that responsibilities exist only within abstraction levels. So it makes perfectly sense for a class from a higher abstraction level to carry out its responsibility by delegating work to lower level component, which is best achieved through dependency inversion.

Answer 4

Do those classes have only one reason to change?

If you do not know it (yet), then you might have stepped into the speculative design / over engineering trap (too many classes, too complex design patterns applied). You have not satisfied YAGNI. In early stages of the software life cycle (some) requirements might be not clear. Thus the direction of change is currently not visible for you. If you realize that, keep it in one or a minimal amount of classes. However this comes with a liability: As soon as the requirements and the direction of change are clearer you need to rethink the current design and make a refactoring to satisfy the reason of change.

If you already know that there are three different reasons for change and that they map to those three classes, then you just applied the right dose of SRP. Again this comes with some liability: If you are wrong or future requirements change unexpectedly, you need to rethink the current design and make a refactoring to satisfy the reason of change.

The whole point is:

• Keep an eye on the drivers for change.
• Chose a flexible design, which can be refactored.
• Be ready to refactor code.

If you have this mindset, you will shape the code without much fear of speculative design / over engineering.

However there is always the factor time: For many changes you will not have the time you need. Refactoring costs time and effort. I often encountered situations where I knew I had to change the design, but due to time constraints I had to postpone it. This will happen and cannot be avoided. I found that it is easier to refactor under engineered code than over engineered code. YAGNI gives me a good guideline: If in doubt, keep the amount of classes and the application of design patterns to a minimum.

Answer 5

There are three reasons to invoke SRP as a reason to split a class:

• so that future changes in requirements can leave some classes unchanged
• so that a bug can be isolated more quickly
• to make the class smaller

There are three reasons to refuse to split a class:

• so that future changes in requirements won't cause you to make the the same change in multiple classes
• so that bug-finding won't involve tracing long paths of indirection
• to resist encapsulation-breaking techniques (properties, friends, whatever) that expose information or methods to everyone when they are only needed by one related class

When I look at your case and imagine changes, some of them will cause changes to multiple classes (eg add a field to ledger, you'll need to change all three) but many will not - for example adding sorting to the list of ledgers or changing the validation rules when adding a new ledger entry. Your colleague's reaction is probably because it's hard to know where to look for a bug. If something looks wrong on the list of ledgers, is it because it was added wrongly, or there's something wrong with the list? If there's a discrepancy between the summary and the details, which is wrong?

It's also possible your colleague thinks that the changes in requirements that mean you'll have to change all three classes are far more likely than that changes where you'd get away with changing only one. That could be a really useful conversation to have.

Answer 6

I think that if ledger was a table in db, I suggest put these thress task to one class(e.g DAO).If the ledger has more logic and was not a table in db, I suggest we should create more class to do these tasks(may be two or thress class) and provide a facade class to do simply for customer.

Answer 7

IMHO you shouldn't be using classes at all. There are no data abstractions here. The ledgers are a concrete data type. The methods for manipulating and displaying them are functions and procedures. There will certainly be many commonly used functions to be shared such as formatting a date. There is little place for any data to be abstracted and hidden in a class in the display and selection routines.

There is some case to hide state in a class for ledger editing.

Whether or not you're abusing the SRP, you're abusing something more fundamental: you're overusing abstraction. It's a typical problem, engendered by the mythical notion OO is a sane development paradigm.

Programming is 90% concrete: use of data abstraction should be rare and carefully engineered. Functional abstraction, on the other hand should be more common, since language details and choice of algorithms need to be separated from the semantics of the operation.

Answer 8

You only need one class, and the single responsibility of that class is to manage ledgers.