Can the following complicated branching structure be simplified?

Question

I was wondering if there is any way to simplify the following complicated branching structure in C#? Note that "do C;" appears twice.

      if (condition1)
      {    
           if (condition2)
            {
                do A;
                if (condition3 on result of doing A)
                {
                    do B;
                }
                else
                {
                    do C;
                }                    
            }
            else 
            {
                do C;
            }
      }
      else
      {    
            do D;
       }
        do E;    

Answer 1

You can simplify it so that C() only appears once.

Your code is equivalent to

    if (condition1)
    {
        if (condition2)
        {
            if (A())
            {
                B();
            }
            else
            {
                C();
            }
        }
        else
        {
            C();
        }
    }
    else
    {
        D();
    }

    E();

Where the A() method returns a bool and is equivalent to your condition3 on result of doing A).

Looking at this simplification, note that you can replace:

    if (condition2)
    {
        if (A())
        {
            B();
        }
        else
        {
            C();
        }
    }
    else
    {
        C();
    }

With:

if (condition2 && A())
{
    B();
}
else
{
    C();
}

So the final code would look like this:

if (condition1)
{
    if (condition2 && A())
    {
        B();
    }
    else
    {
        C();
    }
}
else
{
    D();
}

E();

It's not a huge improvement, but it now mentions C() only once.

You could also write it as a switch but personally I don't think this is an improvement (I know that this is a subjective matter):

switch (condition1)
{
    case true when condition2 && A():
        B();
        break;

    case true:
        C();
        break;

    default:
        D();
        break;
}

E();

NOTE: It's critical to put condition2 && A() and not A() && condition2, of course.

Answer 2

Usually the problem in this scenary it's the ciclomatic complexity, the code it's hard to read. One way to try to simplify the code is using return.

try
{
    if (condition1)
    {
        if (condition2)
        {
            if (A())
            {
                B();
                return;
            }
        }

        C();
        return;
    }

    D();
}
finally
{
    E();
}

Sometimes this give you a better code, more readable. But this is not the case. The final code is more or less the same and has lots of indentation levels. I include only because sometimes may be an option.

Another way it's divide and conquer. Create a method to reduce nesting:

if (condition1)
{
    if (condition2)
    {
        OnBothConditions();
    }
    else
    {
        C();
    }
}
else
{
    D();
}

E();

And delegate in other method:

private void OnBothConditions()
{
    if (A())
    {
        B();
    }
    else
    {
        C();
    }
}

You also can create this method:

private void OnCondition1()
{
    if (condition2)
    {
        OnBothConditions();
    }
    else
    {
        C();
    }
}

And change your code:

if (condition1)
{
    OnCondition1();
}
else
{
    D();
}

E();

There are other options, like Matthew Watson one, that are right and more compacted, but I prefer code more readable than more compacted because you must maintain that code and in 6 or 10 months probably will be difficult to understand this kind of "optimizations".

Answer 3

I see now after writing it down that this answer is to some extent a mixture of that of Matthew Watson and that of Victor, but I will provide it anyway.

First of all, I agree with Victor that getting rid of all the nesting is an important improvement. I also agree with them that splitting your code into methods can be very helpful in that regard.

Secondly, I agree with Matthew that if the result of do A can be used directly to define the result of condition3, it may also be used directly rather than calling it before evaluating condition3.

If condition3 can be extracted to a method that takes the result of do A as an input parameter, you could avoid nesting entirely by creating one method per nesting level. As an example:

public static void Main()
{
    DoYourThing();
    do E;
}

DoYourThing()
{
    if (<condition1>)
    {
        DoYourNextThing();
    }
    else
    {
        do D;
    }
}

DoYourNextThing()
{
    if (<condition2> && Condition3(A()))
    {
        do B;
    }
    else
    {
        do C;
    }
}

private bool Condition3(object a)
{
    //...
}

If condition3 cannot be extracted to a method that takes the result of do A as an input parameter, DoYourNextThing() may need to be implemented differently. One possibility is:

DoYourNextThing()
{
    if (ShouldDoB())
    {
        do B;
    }
    else
    {
        do C;
    }
}

ShouldDoB()
{
    if (!condition2)
    {
        return false;
    }
    
    var a = do A;
    
    return condition3; // based on a
}

---

As a side note, I'd say that using descriptive variable names and method names is generally more important than refactoring away nested loops; whereas doing both (in a way that enhances comprehension and readability) is gold.

That being said, I trust that OP is using generic names simply to present a clear and easy example of the logic they are trying to achieve.