c# - Invert "if" statement to reduce nesting

ID : 10247

viewed : 14

Tags : c#resharperc#

Top 5 Answer for c# - Invert "if" statement to reduce nesting

vote vote

98

It is not only aesthetic, but it also reduces the maximum nesting level inside the method. This is generally regarded as a plus because it makes methods easier to understand (and indeed, many static analysis tools provide a measure of this as one of the indicators of code quality).

On the other hand, it also makes your method have multiple exit points, something that another group of people believes is a no-no.

Personally, I agree with ReSharper and the first group (in a language that has exceptions I find it silly to discuss "multiple exit points"; almost anything can throw, so there are numerous potential exit points in all methods).

Regarding performance: both versions should be equivalent (if not at the IL level, then certainly after the jitter is through with the code) in every language. Theoretically this depends on the compiler, but practically any widely used compiler of today is capable of handling much more advanced cases of code optimization than this.

vote vote

82

A return in the middle of the method is not necessarily bad. It might be better to return immediately if it makes the intent of the code clearer. For example:

double getPayAmount() {     double result;     if (_isDead) result = deadAmount();     else {         if (_isSeparated) result = separatedAmount();         else {             if (_isRetired) result = retiredAmount();             else result = normalPayAmount();         };     }      return result; }; 

In this case, if _isDead is true, we can immediately get out of the method. It might be better to structure it this way instead:

double getPayAmount() {     if (_isDead)      return deadAmount();     if (_isSeparated) return separatedAmount();     if (_isRetired)   return retiredAmount();      return normalPayAmount(); };    

I've picked this code from the refactoring catalog. This specific refactoring is called: Replace Nested Conditional with Guard Clauses.

vote vote

73

This is a bit of a religious argument, but I agree with ReSharper that you should prefer less nesting. I believe that this outweighs the negatives of having multiple return paths from a function.

The key reason for having less nesting is to improve code readability and maintainability. Remember that many other developers will need to read your code in the future, and code with less indentation is generally much easier to read.

Preconditions are a great example of where it is okay to return early at the start of the function. Why should the readability of the rest of the function be affected by the presence of a precondition check?

As for the negatives about returning multiple times from a method - debuggers are pretty powerful now, and it's very easy to find out exactly where and when a particular function is returning.

Having multiple returns in a function is not going to affect the maintainance programmer's job.

Poor code readability will.

vote vote

70

As others have mentioned, there shouldn't be a performance hit, but there are other considerations. Aside from those valid concerns, this also can open you up to gotchas in some circumstances. Suppose you were dealing with a double instead:

public void myfunction(double exampleParam){     if(exampleParam > 0){         //Body will *not* be executed if Double.IsNan(exampleParam)     } } 

Contrast that with the seemingly equivalent inversion:

public void myfunction(double exampleParam){     if(exampleParam <= 0)         return;     //Body *will* be executed if Double.IsNan(exampleParam) } 

So in certain circumstances what appears to be a a correctly inverted if might not be.

vote vote

58

The idea of only returning at the end of a function came back from the days before languages had support for exceptions. It enabled programs to rely on being able to put clean-up code at the end of a method, and then being sure it would be called and some other programmer wouldn't hide a return in the method that caused the cleanup code to be skipped. Skipped cleanup code could result in a memory or resource leak.

However, in a language that supports exceptions, it provides no such guarantees. In a language that supports exceptions, the execution of any statement or expression can cause a control flow that causes the method to end. This means clean-up must be done through using the finally or using keywords.

Anyway, I'm saying I think a lot of people quote the 'only return at the end of a method' guideline without understanding why it was ever a good thing to do, and that reducing nesting to improve readability is probably a better aim.

Top 3 video Explaining c# - Invert "if" statement to reduce nesting

Related QUESTION?