CODEWITHSUNDEEP
c#oopdesign-patterns
Pritam Karmakar
Pritam Karmakar

Reputation: 2801

Is my design following Single Responsibility Principle?

I want to build a Register class for a website. Here are my use cases --

  1. User will provide email id & password for registration
  2. If email id already exist then Register class will send an error message that this email already exist in our system
  3. If email doesn't exist in our syetem then Register class will send an activation email in user email id and show message to user that -- one activation email already sent to his/ser email

Here is the design that I thought about

Interface IRegister
{
 string RegisterUser(string email, string password);
}

public class Register:IRegister
{        
    public string RegisterUser(string email, string password)
    {
        //Call IsUserExist and SentActivationEmail method internally
    }

    private bool IsUserExist()
    {            
    }

    private void SendActivationEmail()
    {
    }
}

I don't want to mention IsUserExist and SendActivationEmail method in IRegister so that it remains simple. Now how I can enforce a developer who will implement the Register class that he/she should use IsUserExist and SendActivationEmail method and do all operations as mentioned in use case. And does this design violate SRP principle?

Upvotes: 1

Views: 278

Answers (2)

Mike Parkhill
Mike Parkhill

Reputation: 5551

If you want to enforce that developers use those methods then you should be declaring an abstract class with protected abstract methods rather than an interface. Then the definition enforces the methods, but the developer is free to implement them however they need.

public abstract class Register:IRegister
{        
    public string RegisterUser(string email, string password)
    {
        //Call IsUserExist and SentActivationEmail method internally
    }

    protected abstract bool IsUserExist();

    protected abstract void SendActivationEmail();
}

That being said, to keep with the SRP principle I would pull the email part of it out into a dependent IActivationEmailer interface. Emailing and registering are really two different actions and should be kept separate.

public interface IActivationEmailer {
    void SendActivationEmail();
}

public abstract class Register:IRegister
{        
    private IActivationEmailer m_emailer;
    protected Register(IActivationEmailer emailer){
       // store emailer to field
       m_emailer = emailer;
    }

    public string RegisterUser(string email, string password)
    {
        //Call IsUserExist and m_emailer.SentActivationEmail method internally
    }

    protected abstract bool IsUserExist();

}

Upvotes: 2

Andrés Fortier
Andrés Fortier

Reputation: 1693

I agree with Mikes answer. A slightly different approach would be to apply a template method and let subclasses define what they want to do on the registration (I'm not familiar with C# so please bear with me):

public abstract class Register:IRegister
{        
    public string RegisterUser(string email, string password)
    {
        if (this.IsUserExist())
        {
        //throw the error
        }
        else
        {
         this.performRegistration();
         this.notifyUSer();
        }
    }

    protected abstract bool IsUserExist();

    protected abstract notifyUSer();

    protected abstract performRegistration(){}
}

and then, as Mike pointed, you could define:

public interface IActivationEmailer {
    void SendActivationEmail();
}

public class CustomRegister
{        
    private IActivationEmailer m_emailer;

    public CustomRegister(IActivationEmailer emailer){
       // store emailer to field
       m_emailer = emailer;
    }

    protected abstract bool IsUserExist(){...}

    protected abstract notifyUSer() {this.m_emailer.SendActivationEmail();}

    protected abstract performRegistration(){...}
}

So basically the Register class is defining the steps to follow during a registration but it is leaving the subclasses to state how to implement those steps.

HTH

Upvotes: 0

Related Questions