Should I use early returns in C#?

Question

I've learned Visual Basic and was always taught to keep the flow of the program without interruptions, like Goto, Exit and Return. Using nested ifs instead of one return statement seems very natural to me.

Now that I'm partly migrating towards C#, I wonder what the best practice is for C-like languages. I've been working on a C# project for some time, and of course discover more code of ExampleB and it's hurting my mind somehow. But what is the best practice for this, what is more often used and are there any reasons against one of the styles?

public void ExampleA()
{
    if (a != b)
    {
        if (a != c)
        {
            bool foundIt;
            for (int i = 0; i < d.Count && !foundIt; i++)
            {
                if (element == f)
                    foundIt = true;
            }
        }
    }
}

---

public void ExampleB()
{
    if (a == b)
        return;

    if (a == c)
        return;

    foreach (object element in d)
    {
        if (element == f)
            break;
    }
}

Answer 1

I think it is actually the best practice for any language to not have conditionals nested too deeply. Otherwise, you have to remember where you are, for each line of code. Use your second example.

Answer 2

This has been asked here before lots of times in one form or another. Concensus seems to be that the second form is best, and it's certainly the one I use. Some old languages, like the original version of Pascal, required the first form, but all modern Pascals support the second.

Answer 3

Refactoring pressure increases with the amount of nesting. I too believe it is language-independent.

In this instance, go with the second (b) form.

Answer 4

You could refactor it into a second worker method, this has neither deep nesting nor multiple return statements:

public void ExampleE()
{
    if (a != b && a != c)
    {
        doLoopingWork(d, f);
    }
}

private void doLoopingWork (collection d, value f)
{
    foreach (object element in d)
    {
        if (element == f)
            break;
    }
}

Assuming that's a little too much, you can clarify this example by trying to combine the "escape conditions" that don't enter the loop, thus reducing the nesting. The following has slightly reduce nesting, but only one return statement. Combining your two conditions into one, may or may not increase clarity.

public void ExampleC()
{
    if (a != b && a != c)
    {
        foreach (object element in d)
        {
            if (element == f)
                break;
        }
    }
}

Alternatively, the nesting might be considered worse than multiple returns in which case:

public void ExampleD()
{
    if (a == b || a == c)
        return;

    foreach (object element in d)
    {
        if (element == f)
            break;
    }
}

Answer 5

• ExampleA represents the doctrine from structured programming, that one function should have only one return, and leads to unnecesary nested code and offers no benefit in return.

• ExampleB is like checking preconditions and refusing if a condition does not meet the requirements, it is easy to read to maintain and therefore my personal favorite

Answer 6

Single-return is a best practice in unmanaged code, but in managed environments, like the one C# is made for, you can use early returns. The explanation is that if you have do deallocate resources, a single-return makes it easier, but usually you don't need that in C#. In contrast, the early-return has more clarity and simplicity.

Answer 7

This topic is covered (along with many other VB/C# "best practices") in a book by Francesco Balena and Guiseppe Dimauro called "Practical Guidelines and Best Practices for Microsoft Visual Basic and Visual C# Developers" - available at Amazon.

I have found it to be invaluable as a guide in generating our department's standards for code production.

Answer 8

By looking at the two examples I quickly grasp what ExampleB does, but have to concentrate a bit more to get what ExampleA does.

SESE (Singe-Entry, Single-Exit) isn't true anyway with todays exception enabled languages. Go for code readibility.

As far as performance goes, branch prediction is really good nowadays on modern processors (except if you're working on some other hardware where you need to think about what's the most common case to help with branch prediction).

Answer 9

I'm clearly a dinosaur.

Multiple exit points in non-trivial routines are - to me (from what I was taught and what I've done) a bad thing from a readability and maintainability point of view.

Yes, where you've got examples as above where you hit the ideal of short routines where you can read and absorb it all in one go then yes, it works. But out here in the real world code - especially code you're trying to "maintain" - bug fix! - is frequently not like that - its big lumps of ugly code (that yes may scream out to be refactored) where you're banging your head of the desk trying to work out why its not hitting your breakpoint 'til you realise there's an all but invisible return a third of the way in.

Yes sometimes A makes for ugly code but equally B is sometimes used as a way for programmers to avoid thinking about the structure of the code they're writing because you can just bale out at any point.

And its for these reasons that this is a somewhat subjective question (you can make computer science arguments both ways - e.g. there are issues with proof of correctness for code with multiple exit points). At the end of the day I don't like it (though I can see the reasons why) and it contravenes my coding standards (bear in mind that a lot of things done in coding standards are notionally redundant but desirable in practise).

But as I said, clearly I'm a dinosaur.

Answer 10

Another comment: When you make changes to a deeply indented function, it can make SCM diffs annoyingly hard to read because you are constantly changing the indent level of code.