Learning Single Responsibility Principle with C#

问题:

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.

回答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.

回答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.

回答3:

My recommendation is to start with the basics: what things do you have? You mentioned multiple things like Message, User, Channel, etc. Beyond the simple things, you also have behaviors that belong to your things. A few examples of behaviors:

• a message can be sent
• a channel can accept a user (or you might say a user can join a channel)
• a channel can kick a user
• and so on...

Note that this is just one way to look at it. You can abstract any one of these behaviors until abstraction means nothing and everything! But, a layer of abstraction usually doesn't hurt.

From here, there are two common schools of thought in OOP: complete encapsulation and single responsibility. The former would lead you to encapsulate all related behavior within its owning object (resulting in inflexible design), whereas the latter would advise against it (resulting in loose coupling and flexibility).

I would go on but it's late and I need to get some sleep... I'm making this a community post, so someone can finish what I've started and improve what I've got so far...

Happy learning!