Learning Single Responsibility Principle with C#
Solution 1:
Let’s start with what does Single Responsibility Principle (SRP) actually mean:
A class should have only one reason to change.
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:
- A user has a number of channels that he has joined
- A channel has a number of users
This also leads naturally do the following list of functionalities:
- A user can request to join a channel.
- A user can send a message to a channel he has joined
- A user can leave a channel
- A channel can deny or allow a user’s request to join
- A channel can kick a user
- A channel can broadcast a message to all users in the channel
- A channel can send a greeting message to individual users in the channel
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.
Solution 2:
I had a very easy time learning this principle. It was presented to me in three small, bite-sized parts:
- Do one thing
- Do that thing only
- Do that thing well
Code that fulfills those criteria fulfills the Single-Responsibility Principle.
In your above code,
public void UserJoin(User user)
{
if (verify.CanJoin(user))
{
messages.Greeting(user);
}
else
{
this.kick(user);
}
}
UserJoin does not fulfill the SRP; it is doing two things namely, Greeting the user if they can join, or rejecting them if they cannot. It might be better to reorganize the method:
public void UserJoin(User user)
{
user.CanJoin
? GreetUser(user)
: RejectUser(user);
}
public void Greetuser(User user)
{
messages.Greeting(user);
}
public void RejectUser(User user)
{
messages.Reject(user);
this.kick(user);
}
Functionally, this is no different from the code originally posted. However, this code is more maintainable; what if a new business rule came down that, because of recent cybersecurity attacks, you want to record the rejected user's IP address? You would simply modify method RejectUser. What if you wanted to show additional messages upon user login? Just update method GreetUser.
SRP in my experience makes for maintainable code. And maintainable code tends to go a long ways toward fulfilling the other parts of SOLID.