What's your best example of a single responsibility violation?
I'm looking for some good code examples that violate the Single Responsibility Principle. Don't show me any examples from Uncle Bob's books or websites as they are plastered all over the internet, for example:
interface Modem
{
public void dial(String pno);
public void hangup();
public void send(char c);
public char recv();
}
source to share
The granularity of your OO design is a matter of taste and may not be acceptable to others. Thus, I would not look for examples of single responsibility violations in some business logic classes when discussing whether there is too much or little to do.
In my opinion the best examples (with the worst side effects) are breaking the parsing of your application. F.ex :.
- Executing business logic at the data access layer (the only responsibility should be to ensure continuity of access to the application)
- Accessing business services from (through) the domain model (whose sole responsibility is to preserve most of the state of the application)
- Execution of complex business logic at the presentation layer (responsible for presenting data and entering user data)
source to share
Here's some code from a PHP project that I had to execute:
class Session
{
function startSession()
{
// send HTTP session cookies
}
function activateUserAccount()
{
// query the DB and toggle some flag in a column
}
function generateRandomId()
{}
function editAccount()
{
// issue some SQL UPDATE query to update an user account
}
function login()
{
// perform authentication logic
}
function checkAccessRights()
{
// read cookies, perform authorization
}
}
I believe this class is very important.
source to share
In fact, in most of the OO languages I've used, the Object
top-level class is a good example. For example, in Ruby, a class Object
(or rather mix2 Kernel
, which is mixed with Object
) has 45 public instance methods. Now some of them are aliases, but there still has to be at least 20 and they are from all sides. I can easily define at least 5 responsibilities.
Now I'm not going to pick Ruby. This is my favorite programming language. That's why I used it as an example: because it is the language I am most familiar with. And I'm sure what I've written about Ruby is 100% applicable to at least Java and .NET.
source to share
The key about SRP is defining responsibilities so that your implementation does exactly that thing. It is like making a rule (giving a class a name and responsibility) and then trying to follow it.
So, if you don't do this, you are either not defining the rule correctly or are inconsistent in implementing it (or both, which might be the most common case).
I usually find classes that don't give a half-like attempt at determining which main responsibility or good name is the best offenses. Then you just need to read the whole class to try and determine if there are any clearly defined responsibilities at all.
source to share
This is a qualitative question for defining the "responsibilities" of a class.
Just looking at a given class code can by no means give us an idea of how well it handles this reaction.
It is very important, at least in my experience, to take into account how a requirement changes a class will propagate to other modules (or how changes from other classes will propagate to that class).
Since @schmeedy gives a good explanation of "breaking the system layering", which I think is just one way to analyze the "area of responsibility".
I tried to take the discussion a little further. My attempts are to quantify "responsibility".
Some discussions on my blog: http://design-principle-pattern.blogspot.in/2013/12/single-responsibility-principle.html
source to share
#import <Foundation/Foundation.h>
#import <CoreGraphics/CoreGraphics.h>
@interface Spreadsheet : NSObject
- (void)loadFromURL:(NSURL *)url;
- (void)saveToURL:(NSURL *)url;
- (void)drawTo:(CGRect*)targetArea withContext:(CGContextRef *)context;
@end
I would say the above example violates SRP.
At first glance, it's clear that the class is responsible for one thing: tables. It differs from other entities in the area of document management problems such as word processing.
However, consider the reasons for changing the spreadsheet object.
There may be a change in the model underlying the spreadsheet. This affects how the code is loaded and saved, but will not affect how the table is drawn. Thus, the responsibility for loading / saving does not depend on the responsibility for the drawing. Our class has two responsibilities.
So, if we think about all reasonably possible reasons for changing the class and see that only specific methods in the class will be affected, we will see an opportunity to decouple the responsibility for getting a more focused class.
The revised class will be:
@interface SpreadsheetEncoder
- (NSData *)encodedSpreadsheet:(Spreadsheet *)spreadsheet;
- (Spreadsheet *)spreadsheetFromEncodedData:(NSData *)data;
@end
@interface Spreadsheet2 : NSObject
- (NSData *)data;
- (instancetype)initSpreadsheetFromData:(NSData *)data;
- (void)drawTo:(CGRect*)targetArea withContext:(CGContextRef *)context;
@end
As the product evolves, we can ask ourselves "what might change" again, and then reorganize the classes to only be responsible for one problem. SRP refers only to the problem area and our understanding of it at a given time.
SRP, in my opinion, boils down to the task "what can change?" and "what will be affected". When "what might change" only displays one thing that is affected, you have classes that implement the SRP principle.
source to share