How to avoid multiple nested IFs

Question

I am currently trying to restructure my program to be more OO and to better implement known patterns etc.

I have quite many nested IF-statements and want to get rid of them. How can I go about this? My first approach was to get it done with exceptions, so e.g.

public static Boolean MyMethod(String param) {
 if (param == null)
  throw new NullReferenceException("param may not be null");

 if (param.Equals("none") || param.Equals("0") || param.Equals("zero"))
  throw new ArgumentNullException("param may not be zero");

 // Do some stuff with param
 // This is not executed if param is null, as the program stops a soon
 // as one of the above exceptions is thrown
}

The method is used in the main class of the application, e.g.

static void Main() {
 try {
  Boolean test = MyClass.MyMethod(null); // Will throw an exception
 } catch (Exception ex) {
  MessageBox.Show(ex.Message, "Error");
 }

I think this is quite nice, as it prevents the nested statements and nearly all of the methods actions are nicely arranged on one level.

As with IF-statements, the method would look like this

public Boolean MyMethod(String param) {
 if (param != null) {
  if (!param.Equals("none") && !param.Equals("0") && !param.Equals("zero")) {
   // Do some stuff with param
  } else {
   MessageBox.Show("param may not be zero", "Error");
 } else {
  MessageBox.Show("param may not be null", "Error");
 }
}

Which I find very, very ugly and hard to maintain.

Now, the question is; is this approach good? I know, that might be subjective, but how do you overcome nested IFs (1 or 2 levels are not that bad, but it gets worse after that...)

Answer 1

It really depends on their purpose. In your first sample, the if statements serves the purpose of enforcing a contract, making sure that the input to the method meets certain requirements. In those cases, my own code tend to look pretty much like your code.

In the case of using the if blocks to control the flow of a method (rather than enforcing a contract), it can sometimes be a bit harder. Sometimes I come across code like the following (extremely simplified) example:

private void SomeMethod()
{
    if (someCondition == true)
    {
        DoSomething();
        if (somethingElse == true)
        {
           DoSomethingMore();
        }
    }
    else
    {
        DoSomethingElse();
    }
}

In this case, it seems as if the method has several responsibilities, so in this case I would probably choose to split it into several methods:

private void SomeMethod()
{
    if (someCondition == true)
    {
        DoItThisWay();
    }
    else
    {
        DoSomethingElse();
    }
}

private void DoItThisWay()
{
    DoSomething();
    if (somethingElse == true)
    {
       DoSomethingMore();
    }
}

This makes each method much simpler with less nested code, and may also increase the readability, if the methods are given good names.

Answer 2

You way want to investigate C#4 code contracts.

A pattern often used is DDD specification pattern for abstracting out if statements, although in your case its probably not suitable.

Answer 3

It's really quite a broad question to answer as it would really depend on the functionality of the if statements.

Certainly where possible I try and replace nested if statements with a switch instead, but this is not always possible.

Checking for parameter validity is a good approach, but look at catching it higher up in the code i.e. have the throw statements, but not the catch in the class that throws it.

Answer 4

I think this will be useful for you:
http://sourcemaking.com/refactoring/replace-conditional-with-polymorphism

Answer 5

Your problem is known as the arrowhead anti-pattern.

There are pragmatic approaches, such as the Guard statements you've shown in your sample, to whole design patterns, avoiding if (and else) all together...

Lots of resources on how to solve them:

http://www.codinghorror.com/blog/2006/01/flattening-arrow-code.html

http://www.lostechies.com/blogs/chrismissal/archive/2009/05/27/anti-patterns-and-worst-practices-the-arrowhead-anti-pattern.aspx

http://elegantcode.com/2009/08/14/observations-on-the-if-statement/

Answer 6

Maybe AspectF could help you in this case :

public Boolean MyMethod(String param) {
 try {
  AspectF.Define
   .ErrorMsgIfNull(param, "must be not null")
   .ErrorMsgIfEquals(new string[] {"None", "Zero", "0"}, "may not be zero")
   //...
   // use your own "AspectFlets" you wrote
   //...
   .Do(() =>
    {
     // Do some stuff with param
     // This is not executed if param is null, as the program stops a soon
     // as one of the above exceptions is thrown
    });
}

If you have several conditions to meet (or to avoid) that would make a ugly block of nested if, this way to factorize code may help you to make things a little bit more descriptive.

Methods in the code above are only example that does not exists, but that can be easily implemented.