Avoiding If Else conditions

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?

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. — ScoPi, Mar 27 '14 at 04:54
Looking at this I'm so happy that Delphi has `case` switches. Aren't there such an alternative in C#? — Kromster, Mar 27 '14 at 09:30
`C#` does have `case` statements, the [`switch`](http://msdn.microsoft.com/en-us/library/06tc147t.aspx) statement which is also mentioned in [this question](http://stackoverflow.com/questions/15136134/c-sharp-how-to-use-enum-with-switch). 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. — talegna, Mar 27 '14 at 11:16
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? — jnovacho, Mar 27 '14 at 12:31
This question appears to be off-topic because it is about [CodeReview](http://codereview.stackexchange.com). — Jeroen Vannevel, Mar 29 '14 at 20:51
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. — Greg Trevellick, Oct 22 '16 at 23:10

score 18 · Answer 1 · edited Mar 27 '14 at 08:54

18

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

edited Mar 27 '14 at 08:54

Mehmet Ataş

11,081
6
51
78

answered Mar 27 '14 at 07:39

Mark Seemann

225,310
48
427
736

Should DoSomething not be extracted as an Interface and then you could use dependency injection? – Atomic Star Mar 27 '14 at 13:41
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. – Mark Seemann Mar 27 '14 at 14:54
@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? – fahmi Mar 27 '14 at 22:35
When do you create the `InvitationalSurvey`, `ReturnLaterSurvey`, `AnonymousSurvey` objects? Who owns the `param2` and `param3` values? – Mark Seemann Mar 27 '14 at 22:48
@MarkSeemann As I showed above, a factory method creates the objects. param2 and param3 is relevent to AnonymousSurvey class only. – fahmi Mar 27 '14 at 23:49
When are `param2` and `param3` available? Who creates them? – Mark Seemann Mar 28 '14 at 06:18

score 3 · Answer 2 · answered Mar 27 '14 at 05:01

3

why not adding an overload

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

answered Mar 27 '14 at 05:01

Tal

700
4
11

score 2 · Answer 3 · answered Mar 27 '14 at 05:05

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();

Then again, you need to introduce if...else type checking to determine which factory method to call. — fahmi, Mar 27 '14 at 22:38
@fahmi - no way around that. But it does localize it in the creational code. — tvanfosson, Mar 28 '14 at 02:56

score 2 · Answer 4 · edited Jun 20 '20 at 09:12

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

score 1 · Answer 5 · answered Mar 27 '14 at 04:57

1

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

answered Mar 27 '14 at 04:57

Elliott Frisch

198,278
20
158
249

Dawood ibn Kareem · Answer 6 · 2014-03-27T05:42:27.167

1

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();
    }
}

edited Mar 27 '14 at 05:42

answered Mar 27 '14 at 04:57

Dawood ibn Kareem

77,785
15
98
110

what kind of propaganda lies behind your profile picture? – nurettin Mar 27 '14 at 08:28
I think @nurettin is suggesting that you are possibly pushing a political message via your avatar – Matt Wilko Mar 27 '14 at 13:05
1

@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. – nurettin Mar 27 '14 at 13:12
@nurettin 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? – Dawood ibn Kareem Mar 27 '14 at 18:19
@DavidWallace I agree, stackoverflow is no place for your propaganda. We have reddit for that. – nurettin Mar 28 '14 at 12:22

Avoiding If Else conditions

6 Answers6

Live Demo