Avi Turner
Avi Turner

Reputation: 10456

Cleaner code: Re-factor multiple nested if statements

I have code that looks something like this:

if(condition1)
{
  //do some stuff
  if(condition2)
  {
   //do some other stuff
   if(condition3)
   {
       //do some more stuff
       if(condition4)
       { 
       //you probably got the point by now...
       }
   }
}

And I would like to re-factor it to code that looks better and is easier to follow.
So far the best I got Is:

do{    
    if(!condition1){break;}
    //do some stuff
    if(!condition2){break;}
    //do some other stuff
    if(!condition3){break;}
    //do some more stuff
    if(!condition4){break;}
    //you probably got the point by now...

}while(false);   

My question is:
Is there another better way I am missing?

I don't think it is relevant, but I am using C#...

Upvotes: 1

Views: 4128

Answers (2)

keenthinker
keenthinker

Reputation: 7830

Since you are using C#, the idea of @dseibert could be extended a little bit more and made flexible using delegates, in this case Func. You could create a List that holds Func's and add as many functions with the signature bool function(void) as you want and then evaluate the result of all of them using LINQ.

Three example functions to play with:

private bool isRed()
{
    System.Console.WriteLine("red");
    return true;
}

private bool isBlue()
{
    System.Console.WriteLine("blue");
    return false;
}

private bool isGreen()
{
    System.Console.WriteLine("green");
    return true;
}

List holding Funcs, that is filled with the test functions and initialized result:

var actions = new List<Func<bool>>();
actions.Add(() => isRed());
actions.Add(() => isGreen());
actions.Add(() => isBlue());
var result = true; // initial value

Evaluate all functions at once:

actions.ForEach(a => result &= a());
System.Console.WriteLine(result);

Now the only thing that you need to do is create a new method and add it to the list.

The downside of this solutions is that every method is always called even if the result is already false, but the code within the ForEach extension method could be optimized.

Upvotes: 2

dseibert
dseibert

Reputation: 1329

Possibly encapsulate the functionality you need for each boolean condition into a method and use the method instead of specifying condition1, condition2, condition3 etc.

private boolean isRed() {
//do some stuff
}

private boolean isBlue() {
//do some other stuff
}

private boolean isGreen() {
//do some more stuff
}

...

if(isRed() && isBlue() && isGreen()) {
//do some more stuff
}

Upvotes: 3

Related Questions