I want to refactor the following code to avoid if...else so that I don't have to change the method every time a new survey type comes in (Open/closed principle). Following is the piece of code I am considering to refactor:
if (surveyType == SurveySubType.Anonymous)
{
    DoSomething(param1, param2, param3);
}
else if (surveyType == SurveySubType.Invitational)
{
    DoSomething(param1);
}
else if (surveyType == SurveySubType.ReturnLater)
{    
    DoSomething(param1);
}
To solve the problem, I added the following classes:
    public abstract class BaseSurvey
{
            public string BuildSurveyTitle()
            {
             ...doing something here
            }
    public abstract void DoSomething(int? param1,int?  param2,int?  param3);
}
public class InvitationalSurvey: BaseSurvey
{
    public override void DoSomething(int? param1,int?  param2,int?  param3)
    {
    //I don't need param2 and param3 here
    }
}
public class ReturnLaterSurvey: BaseSurvey
{
    public override void DoSomething(int? param1,int?  param2,int?  param3)
    {
    //I don't need param2 and param3 here
    }
}
public class AnonymousSurvey: BaseSurvey
{
    public override void DoSomething(int? param1,int?  param2,int?  param3)
    {
    //I need param2 and param3 here
    //do something
    }
}
And this is what my code ends up:
var survey = SurveyFactory.Create();
survey.DoSomething(param1,param2,param3);
My question is what would be a nice to avoid passing param2 and param3 to InvitationalSurvey and ReturnLaterSurvey classes?
The experts in clean code advise not to use if/else since it's creating an unreadable code. They suggest rather using IF and not to wait till the end of a method without real need.
Avoid if-else When Assigning Value to a Variable While it might not seem bad, you could easily end up inverting or tweaking the conditional logic to give a whole new meaning. The branching logic also brings a little code duplication, as we're assigning the variable num in multiple blocks, which is not necessary.
Some alternatives to the if-else statement in C++ include loops, the switch statement, and structuring your program to not require branching.
If param2 and param3 are concrete requirements of AnonymousSurvey, they shouldn't be part of the interface, but of the concrete class:
public abstract class BaseSurvey
{
    public abstract void DoSomething(param1);
}
public class InvitationalSurvey: BaseSurvey
{
    public void DoSomething(param1)
    {
    }
}
public class ReturnLaterSurvey: BaseSurvey
{
    public void DoSomething(param1)
    {
    }
}
public class AnonymousSurvey: BaseSurvey
{
    private readonly object param2;
    private readonly object param3
    public AnonymousSurvey(param2, param3)
    {
        this.param2 = param2;
        this.param3 = param3;
    }
    public void DoSomething(param1)
    {
        // use this.param2 and this.param3 here
    }
}
                        If you love us? You can donate to us via Paypal or buy me a coffee so we can maintain and grow! Thank you!
Donate Us With