early return/continue vs mega-nesting

A question of layout. First, a couple of case studies.

Casey's code is usually layed out something like this:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
BOOL myFunc()
{
	BOOL returnValue = YES;

	if(something1)
	{
		if(something2 && !something3)
		{
			for(int x=0; x<something4; ++x)
			{
				if(something5)
				{
					returnValue = NO;
					break;
				}
			}
		}
		else
		{
			returnValue = NO;
		}
	}

	return returnValue;
}


There are two features I'm interested in here: firstly, it only exits at the end of the function, and secondly, there are many levels of nesting.

My code would have multiple exit points but have as few levels of nesting as what -- to me at least -- makes the code most readable:


 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
BOOL myFunc()
{
	if(!something1)
		return NO;
	if(!something2)		*
		return NO;
	if(something3)		*
		return NO;

	for(int x=0; x<something4; ++x)
	{
		if(!something5)
			continue;

		return NO;
	}

	return YES;
}

* if something2 and something3 are logically related then I would combine them as (!something2 || something3)


You also need to picture these functions filled out with all the in-between code, which alters the trade-offs somewhat: in Casey's version the closing braces are often off screen, and in my version the multiple exit points (whether returns, continues, or breaks) can get lost in the code jungle.

--

Casey's approach doesn't slow Casey down at all. But I wondered how everyone else approached the tradeoff, and whether there are any mines lurking in the deep waters of either layout?

Edited by Rowan Crawford on
I tend to use the same multi-return minimal nested approach as you are suggesting.

I think it's just because I had looking at code that is more than three indents deeps.
Early returns are problematic if the function acquires resources that it has to release again if an error occurs later on. And since I rarely know in advance if I'll ever add some resource acquisition to a function retroactively, I tend to gravitate to nesting.

As always, there are exceptions. If the function I'm writing only has work to do if some simple to test preconditions are met, I check those and return early as appropriate. This usually happens within the first few lines of a function, so it's rather obvious and easy to see.
The trade off with "early out" code is that it becomes harder to figure out how much the current code you are looking at is contingent on other things. By that I mean, when you are looking at some code and see that it is highly indented, you know that there are a number of conditions that must be fulfilled for it to execute and you know that without having to look above (typically off the screen) to see what they are. With the early out model, the only way you know is to start at the beginning of the function and scan down to the point of interest.

There a number of things about Casey's coding style that go against my typical habits but that is one of things that makes the series interesting. He usually has good justifications for why he does the things that he does and provides food for thought. I am very interested to see what the final production code looks like.
Yes, as Nimbal said, the primary reason that you don't want to do early returns is because it is difficult to know if all paths do all the things required of the function. If you decide later that you want the function to always do some additional thing (like setting some member of the thing that's passed in to "true" or "false", or something like this), then with a single-return-point function it tends to be easy to make error-free updates of this form. With a multi-return-point function, it is harder, because you may forget about one of the return points.

But, if you're confident that you don't make that kind of error very often, then it's probably fine to just use early returns if you prefer them! Much like "const", it has nothing to do with the behavior of the code, it's just about preventing errors. So if you find that it doesn't actually prevent you from making more errors, then it's not useful and is unnecessary.

- Casey
Thanks for the responses. Some things mentioned there that I definitely need to watch out for, to see if they're a cause of bugs for me.

Thanks.
Generally I prefer early exits over deeply nested structures. It makes it much easier to mentally follow what the function is doing. If I hit a return statement I know I am done immediately. If it is written with lots of nesting and only a single return I have to follow along for the rest of the function before I know if I really am done.


If you decide later that you want the function to always do some additional thing (like setting some member of the thing that's passed in to "true" or "false", or something like this), then with a single-return-point function it tends to be easy to make error-free updates of this form. With a multi-return-point function, it is harder, because you may forget about one of the return points.

I disagree. It makes it easier to do the same thing before returning, no matter what. But in my experience it is much more common to add some more code only for some conditions. With early returns you can grep for the 'return' keyword and immediately see all the places where you might want to add something. Finding the right place to add an 'else' in a deeply nested structure takes much more effort.
How do you people feel about guard statements?
What is guard statement?
Maybe it was called different.
But what I mean is checking the parameters in the top of a function and return there if the input is invalid.


 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
void Foo(int X)
{
   // guard statement, check input parameters
   if (X < 0)
   {
     return;
   }
  
   // rest of code, branch from here

}
cmuratori
If you decide later that you want the function to always do some additional thing (like setting some member of the thing that's passed in to "true" or "false", or something like this), then with a single-return-point function it tends to be easy to make error-free updates of this form. With a multi-return-point function, it is harder, because you may forget about one of the return points.


Im not saying that this is any better, it's all dependent on personal preference, but lets not forget about `goto`

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
BOOL myFunc()
{
	if(!something1)
	    goto EARLY_OUT;
	if(!something2)
	    goto EARLY_OUT;
	if(something3)
	    goto EARLY_OUT;

	for(int x=0; x<something4; ++x)
	{
		if(!something5)
			continue;

	    goto EARLY_OUT;
	}

	return YES;

	EARLY_OUT:
	printf("Doing some additional thing\n");
        return NO;

}
clankill3r
Maybe it was called different.
But what I mean is checking the parameters in the top of a function and return there if the input is invalid.

That is called early-return.
This is what 5umaleth suggests in his second code fragment in first post of this topic.
I like them and write code like that.