Coding Tip: Have A Single Exit Point

Having one exit point (return) from a function is a good thing. Here is an example of a single exit point:

int MyArray::indexOfElement(int elementToFind){
	int foundIndex = ELEMENT_NOT_FOUND;
 
	for(int i = 0; i < m_numberOfElements; ++i){
		if(this->elementAtIndex(i) == elementToFind){
			foundIndex = i;
			break;
		}
	}
 
	return foundIndex;
}

Having multiple exit points can be bad. Here is an example of multiple exit points:

int MyArray::indexOfElement(int elementToFind){
	for(int i = 0; i < m_numberOfElements; ++i){
		if(this->elementAtIndex(i) == elementToFind){
			return i;
		}
	}
 
	return ELEMENT_NOT_FOUND;
}

Why are multiple exit points bad? Because I say so. They are also bad for these two reasons:

1. It makes code fragile, and therefor increases the chance of introducing bugs.
2. It interrupts the logical flow of the function, and therefor makes it harder to understand.

Before I make my argument against multiple exit points, I will admit that the examples provided are rather trivial. A six line function with two exit points isn’t particularly dangerous or unreadable; especially considering that it’s very common to see an early return in functions that search a collection of objects. However, the problems increase exponentially with the number of exit points and the size of the function. When you’re making changes to a 70 line function with a generous sprinkling of return statements, the problems are quite obvious.

There is some controversy surrounding returning early versus having a single exit point, and both sides have decent arguments. However, it is my belief that robustness is more important than insignificant performance gains, and for any non-trivial function returning once will improve maintainability.

Fragility

Here is an example of a modification to the above function where having two exit points causes a bunch of problems, demonstrating the fragility of the function:

int MyArray::indexOfElement(int elementToFind){
	FooBar* fb = new FooBar;
	fb->openSomeFile();
	fb->startSomeWorkerThread();
 
	for(int i = 0; i < m_numberOfElements; ++i){
		if(this->elementAtIndex(i) == elementToFind){
			//this return causes:
			//  1. a memory leak
			//  2. a permanently open file
			//  3. a rogue worker thread that could disrupt the application later
			return i;
		}
	}
 
	fb->closeSomeFile();
	fb->waitForWorkerThreadToFinish();
	delete fb;
	return ELEMENT_NOT_FOUND;
}

You could duplicate the clean up code, but duplication will make the function even more fragile. Any change to the clean up code must be done in multiple places, and if you forget one, you get problems all over again. As a rule of thumb, you should never duplicate code. Here is an example with the duplicated clean up code:

int MyArray::indexOfElement(int elementToFind){
	FooBar* fb = new FooBar;
	fb->openSomeFile();
	fb->startSomeWorkerThread();
 
	for(int i = 0; i < m_numberOfElements; ++i){
		if(this->elementAtIndex(i) == elementToFind){
			fb->closeSomeFile(); //yuck
			fb->waitForWorkerThreadToFinish(); //yuck
			delete fb; //and yuck
			return i;
		}
	}
 
	fb->closeSomeFile();
	fb->waitForWorkerThreadToFinish();
	delete fb;
	return ELEMENT_NOT_FOUND;
}

Maintainability

Have you ever:

1. modified a function only to find that it behaves exactly the same as before because of an early return statement?
2. introduced a bug like in the code example above and spent forever finding it because the early return statement only happens under certain conditions?

If so, then you’ve experienced how extra return statements can make it difficult to read and understand a section of code. If you accidentally miss the extra return statements, then you’re likely to introduce bugs. If you do catch the return statements, then you have to constantly ask yourself "will this line be executed, or has it already returned?" The larger the function and number of return statements, the greater the potential for error.

The Exceptions

Having said how great single exit points are, I will mention that there are certain situations where multiple exit points are safe and improve readability. I think it is good to return early in function guards like so:

void MyArray::insertElementAtIndex(int element, int index){
	if(index < 0 || index > m_numberOfElements)
		return;
 
	//code for inserting element goes here
}

This is only because the return effectively stops the whole function from executing. Adding a FooBar object is perfectly safe, as you can see here:

void MyArray::insertElementAtIndex(int element, int index){
	if(index < 0 || index > m_numberOfElements)
		return;
 
	FooBar* fb = new FooBar;
 
	//code for inserting element goes here
 
	delete fb;
}

Returning early can also be used instead of deeply nested ‘if’ statements. In such a situation, returning early can be the lesser of two evils.

This post was inspired by Wil Shipley’s post about why he likes returning early. I love your work, Wil, but I have to disagree with you on this one. Those goto statements give me a bad feeling.

This entry was posted on Tuesday, June 30th, 2009 at 10:19 am and is filed under Coding Tips. You can follow any responses to this entry through the RSS 2.0 feed. You can leave a response, or trackback from your own site.

If you enjoyed this post, why not subscribe?