Nested 'if'-'else' statements

Question

I have code that's very messy with the if - else if checks it is doing. The amount of branching and nested branching is quite big (over 20 if - else if and nested too). It's making my code harder to read and will probably be a performance hog. My application checks for a lot of conditions it gets from the user and so the application must check all the time for different situations, for example:

If the textbox text is not 0, continue with the next...

if ((StartInt != 0) && (EndInt != 0))   
{

And then here it checks to see whether the user have choosen dates:

if ((datePickerStart.SelectedDate == null) || (datePickerEnd.SelectedDate == null)) 
{
    MessageBox.Show("Please Choose Dates");
}

Here, if the datepickers aren't null it continues with code...

else if ((datePickerStart.SelectedDate != null) && (datePickerEnd.SelectedDate != null))
{
    // CONDITIONS FOR SAME STARTING DAY AND ENDING DAY.
    if (datePickerStart.SelectedDate == datePickerEnd.SelectedDate)
    {
        if (index1 == index2)
        {
            if (StartInt == EndInt)
            {
                if (radioButton1.IsChecked == true)
                {
                    printTime3();
                }
                else
                {
                    printTime();
                }
            }

This is just a small part of the checks being made. Some of them are functionals and some are for input validation stuff.

Is there any way to make it more readable and less of a performance hog?

Answer 1

It's not that of a performance hog. A great blog post on how to fix those common problems is Flattening Arrow Code.

Answer 2

I see here some mix in validation. Try to move one fields from others, and validate them separately, something like this:

if (StartInt == 0 || EndInt == 0)
{
    MessageBox.Show("Please Choose Ints");
    return;
}
if (datePickerStart.SelectedDate == null || datePickerEnd.SelectedDate == null)
{
    MessageBox.Show("Please Choose Dates");
    return;
}

In this approach you'll always say to the user what he did wrong, and your code is much simpler.

More information from Jeff's blog

Answer 3

One way is to refactor by encapsulating complex conditions in the following way:

public bool DateRangeSpecified
{
  get 
  {
    return (datePickerStart.SelectedDate != null) 
           && 
           (datePickerEnd.SelectedDate != null)
           && StartInt != 0 && EndInt != 0; 
  }
}

and then using these "condition facade" properties

Answer 4

Some slight refactoring makes it easier to read to my eyes. I removed extraneous brackets and consolidated multiple IF statements that are really just AND logic.

if (StartInt == 0 || EndInt == 0)    
    return;
if (datePickerStart.SelectedDate == null || datePickerEnd.SelectedDate == null)
{
    MessageBox.Show("Please Choose Dates");  
    return;           
}
if (datePickerStart.SelectedDate != null 
    && datePickerEnd.SelectedDate != null
    && datePickerStart.SelectedDate == datePickerEnd.SelectedDate
    && index1 == index2
    && StartInt == EndInt)
{
    if (radioButton1.IsChecked == true)
        printTime3();
    else
        printTime();
}

Answer 5

You can define your own predicates or generic functions with meaningful names and encapsulate you logic into those.

Here is a code example for some predicates:

public Predicate<DateTime> CheckIfThisYear = a => a.Year == DateTime.Now.Year;
public Func<DateTime, int, bool> CheckIfWithinLastNDays = (a, b) => (DateTime.Now - a).Days < b;

Now you can easily write in your code

if (CheckIfThisYear(offer) && CheckIfWithinLastNDays(paymentdate,30)) ProcessOrder();

Consider using generic delegates, like Func<> and Delegate<> for writing small blocks of your conditions using lambda-expressions -- it will both save the space and makes your code much more human-readable.

Answer 6

Use the return statement to stop the execution of a block.

For instance,

void Test()
{
    if (StartInt==0 || EndInt==0)
    {
        return;
    }

    if (datePickerStart.SelectedDate == null || datePickerEnd.SelectedDate == null)
    {
        MessageBox.Show("Please Choose Dates");
        return;
    }
}