Avoiding If Else conditions [closed]

Question

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?

Perfoliate

The best I can think of off the top of my head is to add a similar class structure for the arguments... In other words, make something like a BaseSurveyArguments base class and extend it as needed. Not very elegant though. – Ceremony

Looking at this I'm so happy that Delphi has case switches. Aren't there such an alternative in C#? – Strata

C# does have case statements, the switch statement which is also mentioned in this question. There's probably some clever way of using delegates or entity references to make it even sleeker though I haven't experienced any yet myself. – Taxexempt

I too think that switch together with default arguments is a solution. In Python you can assign default value to all params and the provide list of updated parameters. Other concern is, if there should be only one DoSomething method. Why not ProcessLateSurvey, ProcessAnonSurvey etc. mehtods? – Lam

This question appears to be off-topic because it is about CodeReview. – Euton

Just change the 'if' to a 'switch' - yes, when a new condition crops up you have to change the 'switch', but just a very simple copy/paste of an existing switch to call a new method for the new scenario. This is entirely SOLID, because everything is still single responsibilty, separation of concerns, and more importantly is easy to understand and therefore a highly maintainable solution. The other answers to this post are massively over-complicated things just for sake of being SOLID at the price of having easy-to-maintain-code. – Mallissa

Answer 1

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
    }
}

Shannashannah

Should DoSomething not be extracted as an Interface and then you could use dependency injection? – Haden

You could do that. That would be my preferred design approach, but as given, BaseSurvey is a pure abstract type, so from a client's perspective, it's compositionally equivalent to an interface. – Shannashannah

@MarkSeemann Could you give me example of how the client will call DoSomething? Need to do a type check for AnonymousSurvey for passing param2 and param3 right? – Perfoliate

When do you create the InvitationalSurvey, ReturnLaterSurvey, AnonymousSurvey objects? Who owns the param2 and param3 values? – Shannashannah

@MarkSeemann As I showed above, a factory method creates the objects. param2 and param3 is relevent to AnonymousSurvey class only. – Perfoliate

When are param2 and param3 available? Who creates them? – Shannashannah

Answer 2

why not adding an overload

doSometing(Param1){
 doSomething(Param1, null, null)
}

Demodulator

Answer 3

It would help to know what the parameter types are. If they are all the same, then you could, in C# at least, use the params keyword and send as many parameters as needed. If not, then you might want to pass a parameter dictionary, then leave it up to the implementing class to cast the object to the correct type.

public abstract class BaseSurvey
{
    public abstract void DoSomething(params string[] parameters);
}

public abstract class BaseSurvey
{
    public abstract void DoSomething(Dictionary<string,object> parameters);
}

Perhaps a better way would be to incorporate the parameters into the factory method call and have the factory set the values on the correct type when it is created, then you can call the method without any parameters.

var survey = surveyFactory.CreateAnonymousSurvey(param1, param2, param3);
survey.DoSomething();

and

var survey = surveyFactory.CreateReturnLaterSurvey(param1);
survey.DoSomething();

Hoff

Then again, you need to introduce if...else type checking to determine which factory method to call. – Perfoliate

@fahmi - no way around that. But it does localize it in the creational code. – Hoff

Answer 4

It seems to be a case of Overloading, but it is already suggested. So as an alternative, why don't you do just like this, which means assigning a default value to an argument makes it optional. Have a look at the below example.

I demonstrated an integer type you can change the type and make your default value which you suits best fit.

Live Demo

using System;

public class Test
{
    public static void Main()
    {
        // your code goes here
        InvitationalSurvey iservey = new InvitationalSurvey();
        iservey.DoSomething(1, 1, 1);
        iservey.DoSomething(1);
    }
}

public abstract class BaseSurvey
{
     
}
public class InvitationalSurvey: BaseSurvey
{
    public void DoSomething(int param1, int param2 = 0, int param3 = 0)
    {
    //I don't need param2 and param3 here
    Console.WriteLine(string.Format("{0},{1},{2}",param1, param2, param3));
    }
}

Petulia

Answer 5

Your posted code is not c# or java. Regardless, it sounds like you want an Option type.

Ation

Answer 6

You could have another abstract class extending BaseSurvey, which InvitationalSurvey and ReturnLaterSurvey both extend. This abstract class could implement DoSomething(param1,param2,param3) by calling its own abstract method DoSomething(param1), which InvitationalSurvey and ReturnLaterSurvey could extend intead of DoSomething(param1,param2,param3)

public abstract class BaseSurvey
{
    public abstract void DoSomething(param1, param2, param3);
}

public abstract class SpecialSurvey : BaseSurvey
{
    public abstract void DoSomething(param1);

    public void DoSomething(param1, param2, param3)
    {
        DoSomething(param1);
    }
}

public class InvitationalSurvey: SpecialSurvey
{
    public void DoSomething(param1)
    {
         ReallyDoSomething();
    }
}

Yulan

what kind of propaganda lies behind your profile picture? – Ozzy

I think @Ozzy is suggesting that you are possibly pushing a political message via your avatar – Forestay

@DavidWallace If you want to direct a question to a person on the site, you can use the @ symbol so they get an alert. Assuming that was directed at me, I am obviously suggesting that you are using a propaganda picture and asking what it means. – Ozzy

@Ozzy I think you know perfectly well what it means, and you are just trying to start a fight. I don't think Stack Overflow is the right forum for that, do you? – Yulan

@DavidWallace I agree, stackoverflow is no place for your propaganda. We have reddit for that. – Ozzy