Nested expressions 'if' - 'else'

Question

I have code that is very dirty with if - else if it checks. The number of branching and nested branches is quite large (more than 20 if-else if and nested). This makes my code more difficult to read and will probably be a performance whistle. My application checks the many conditions that it receives from the user, and therefore the application must constantly check various situations, for example:

If the text of the text field is not 0, continue with the following ...

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

Then it checks if the user selects dates:

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

Here, if datepickers are not 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 only a small part of the checks. Some of them are functional, and some of them relate to input verification materials.

Is there a way to make it more readable and less workable?

Answer 1

This is not what pigs have in performance. Great blog post on how to fix these common problems, Compression of arrow code .

Answer 2

I see here some mix in checking. Try to transfer one field from others and check 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 will always tell the user what he did wrong, and your code is much simpler.

Additional Information from Jeff's Blog

Answer 3

One way is refactoring by encapsulating complex conditions as follows:

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

and then using these properties "facade condition"

Answer 4

Use the return 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; } } 

Answer 5

A little refactoring makes reading my eyes easier. I removed the extraneous brackets and combined several IF statements that really are 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 6

You can define your own predicates or common functions with meaningful names and encapsulate the logic in them.

Here is a sample code 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 to your code

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

Consider using common delegates such as Func<> and Delegate<> to write small blocks of your conditions using lambda expressions - it will save space and make your code more human-friendly.