Which one is preferable: nested or output sub / function?

Question

I recently worked on legacy vb.net code, and during a peer-to-peer code review, it was recommended not to use Exit Sub / Function, but instead to put all the functionality into IF statements.

When I initially began to develop, I did it so instinctively (Nest IF), it not only seemed more logical, but it seemed less confusing to him.

However, at some point I was working with a team that saw nested IF as “evil,” and therefore the “Exit” premise / functions that I was told were preferable. I am sure that they have prepared MS material for best practice to support this.

So this question is for experienced developers, which path is really preferable? If you give an answer, can you also indicate your sources or just mention that this preference is preferable to your team / company / personal staff and an explanation of the reasons.

Thanks in advance.

EDIT as requested: Sample code

Sub Output:

Private Sub DoSomeWork() if not conditionMetFromAnotherFunction() then exit Sub end if 'Method work starts here End Sub 

Nested IFs:

 Private Sub DoSomeWork() if conditionMetFromAnotherFunction() then 'Method work starts here end if End Sub 

Answer 1

As David noted in his comment, nested if statements can add to the complexity of your code.

Imagine the following (simplified) code:

 Private Sub DoSomeWork() if conditionMetFromAnotherFunction() then if conditionDependantUponPreviousCondition then ' Do the work end if end if End Sub 

Or the following

 Private Sub DoSomeWork() if not conditionMetFromAnotherFunction() return else if not conditionDependantUponPreviousCondition return end if ' If we're here, everything all good ' Do the work... End Sub 

If your conditions become more complex, returning makes it much easier to understand that under certain conditions your code does nothing and makes your code more readable.

Otherwise, you must read the entire function and mentally parse the nested if nothing is done.

Answer 2

If you do not exit your functions earlier, you will reach the point where your code looks like this:

No one can tell me that this is a better style than returning earlier from a function.

Answer 3

During peer-to-peer code review, it was recommended not to use Exit Sub / Function, but instead to put all the functionality into IF statements.

This is terrible advice. It is so simple. Ignore it. In fact, on the contrary, it is usually true, especially in situations where an indent is required, or when you check your parameters for validity and can exit earlier: the code in your question is a good example of this. Use the early exit here.

There is no “official” source for this (what would be official?), But its quite a lot of consensus among good programmers, with a very small minority that opposes this. For more information, see the discussion on programmers .

However, Id advises using Return instead of Exit {Sub|Function} .

Answer 4

Like everyone else, "it depends." Anyone can be unpleasant if used in the wrong context.

For example, what is the conditionMetFromAnotherFunction() check? If he checks some required precondition for DoSomeWork() , I would even go so far as to exclude, and not just calmly exit the function. ArgumentException would be useful if it checked the validity of the argument passed to the function, for example. A quiet performance does not seem right if something in the system was really wrong.

For nested conventions, it's definitely messy. Keep in mind the rule of thumb that a function must "do one thing." Verifying that the condition is one thing. Therefore, in this case, the 'Method work starts here should be nothing more than a call to another method that actually does this work. It should not be many lines of code wrapped in one big conditional. And function names should accurately reflect what they do. So this will be DoWorkIfConditional (in a far-fetched example), and the other method will be DoWork .

This is normal for precondition checking functions before doing work. If the preconditions are not met, I would choose an exception. But it depends on the actual application logic, which is not really passed in this example.

Answer 5

I suggest reading the answers here for a good overview on this topic.

To summarize: Single Entry / Single Exit, developed in languages ​​where you can have multiple entry points for a function, and you can also return to different positions in the code. It was misinterpreted as allowing only one point in the code where you can return from .

The "best practice" of using only one return / exit statement in a routine comes from languages ​​with explicit heap control, where subprogram resources are freed at the end of the routine, so a control flow is required to go through there. This does not apply to languages ​​based on .NET or the JVM.

In general, code is usually read when you are allowed to use multiple returns.

Answer 6

IMO is nested if it is a very fast route to a terrible spaghetti code. Generally speaking, if you deeply embed your code, then you are trying to do a lot of work in your method and, most likely, will benefit from the reorganization into smaller parts.

Having said that sometimes this cannot be avoided, so no answer is suitable for everyone.