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