I am trying to learn the Single Responsibility Principle (SRP) but it is being quite difficult as I am having a huge difficult to figure out when and what I should remove from one class and where I should put/organize it.
I was googling around for some materials and code examples, but most materials I found, instead of making it easier to understand, made it hard to understand.
For example if I have a list of Users and from that List I have a class Called Control that does lots of things like Send a greeting and goodbye message when a user comes in/out, verify weather the user should be able to enter or not and kick him, receive user commands and messages, etc.
From the example you don't need much to understand I am already doing too much into one class but yet I am not clear enough on how to split and reorganize it afterwards.
If I understand the SRP, I would have a class for joining the channel, for the greeting and goodbye, a class for user verification, a class for reading the commands, right ?
But where and how would I use the kick for example ?
I have the verification class so I am sure I would have all sort of user verification in there including weather or not a user should be kicked.
So the kick function would be inside the channel join class and be called if the verification fails ?
For example:
public void UserJoin(User user)
{
if (verify.CanJoin(user))
{
messages.Greeting(user);
}
else
{
this.kick(user);
}
}
Would appreciate if you guys could lend me a hand here with easy to understand C# materials that are online and free or by showing me how I would be splitting the quoted example and if possible some sample codes, advice, etc.
Let’s start with what does Single Responsibility Principle (SRP) actually mean:
This effectively means every object (class) should have a single responsibility, if a class has more than one responsibility these responsibilities become coupled and cannot be executed independently, i.e. changes in one can affect or even break the other in a particular implementation.
A definite must read for this is the source itself (pdf chapter from "Agile Software Development, Principles, Patterns, and Practices"): The Single Responsibility Principle
Having said that, you should design your classes so they ideally only do one thing and do one thing well.
First think about what “entities” you have, in your example I can see User
and Channel
and the medium between them through which they communicate (“messages"). These entities have certain relationships with each other:
This also leads naturally do the following list of functionalities:
SRP is an important concept but should hardly stand by itself – equally important for your design is the Dependency Inversion Principle (DIP). To incorporate that into the design remember that your particular implementations of the User
, Message
and Channel
entities should depend on an abstraction or interface rather than a particular concrete implementation. For this reason we start with designing interfaces not concrete classes:
public interface ICredentials {}
public interface IMessage
{
//properties
string Text {get;set;}
DateTime TimeStamp { get; set; }
IChannel Channel { get; set; }
}
public interface IChannel
{
//properties
ReadOnlyCollection<IUser> Users {get;}
ReadOnlyCollection<IMessage> MessageHistory { get; }
//abilities
bool Add(IUser user);
void Remove(IUser user);
void BroadcastMessage(IMessage message);
void UnicastMessage(IMessage message);
}
public interface IUser
{
string Name {get;}
ICredentials Credentials { get; }
bool Add(IChannel channel);
void Remove(IChannel channel);
void ReceiveMessage(IMessage message);
void SendMessage(IMessage message);
}
What this list doesn’t tell us is for what reason these functionalities are executed. We are better off putting the responsibility of “why” (user management and control) in a separate entity – this way the User
and Channel
entities do not have to change should the “why” change. We can leverage the strategy pattern and DI here and can have any concrete implementation of IChannel
depend on a IUserControl
entity that gives us the "why".
public interface IUserControl
{
bool ShouldUserBeKicked(IUser user, IChannel channel);
bool MayUserJoin(IUser user, IChannel channel);
}
public class Channel : IChannel
{
private IUserControl _userControl;
public Channel(IUserControl userControl)
{
_userControl = userControl;
}
public bool Add(IUser user)
{
if (!_userControl.MayUserJoin(user, this))
return false;
//..
}
//..
}
You see that in the above design SRP is not even close to perfect, i.e. an IChannel
is still dependent on the abstractions IUser
and IMessage
.
In the end one should strive for a flexible, loosely coupled design but there are always tradeoffs to be made and grey areas also depending on where you expect your application to change.
SRP taken to the extreme in my opinion leads to very flexible but also fragmented and complex code that might not be as readily understandable as simpler but somewhat more tightly coupled code.
In fact if two responsibilities are always expected to change at the same time you arguably should not separate them into different classes as this would lead, to quote Martin, to a "smell of Needless Complexity". The same is the case for responsibilities that never change - the behavior is invariant, and there is no need to split it.
The main idea here is that you should make a judgment call where you see responsibilities/behavior possibly change independently in the future, which behavior is co-dependent on each other and will always change at the same time ("tied at the hip") and which behavior will never change in the first place.