Following on from the previous article in the series, another programming habit I see frequently is following the structured programming rule of having only one exit point from a function/method.
I've been prompted to write this article (which I was previously considering but thought might be seen too much a matter of taste rather than anything more definitive than that) by Dave Thomas's talk at SPA 2006. He explained the phenomina of Cargo Cults - which I won't fully describe here - but in a nutshell, it's doing something because it used to work but not considering whether it's still appropriate.
Dave mentioned structured programming - in particular the GOTO statement as a Cargo Cult. (I couldn't find much of a write-up of his talk - I might write more myself some time ...).
I think that the "single exit point" rule of structured programming is a Cargo Cult (but I don't think that the absence of GOTO statements in languages is).
Consider the following function (and pretend the ternary operator isn't available):
void foo(boolean condition) {
int returnValue;
if(condition){
returnValue = 5;
}else{
returnValue = 6;
}
return returnValue;
}
I've seen lots of code like this over the years. I'm sure it's because people have been taught at some point (and I think it's at University) that a function should only have one exit point, and so that's what they do.
Compare the code above with this version:
void foo(boolean condition) {
if(condition){
return 5;
}else{
return 6;
}
}
It's slightly shorter, and I think, easier to understand.
In the "single exit point" version, the declaration of "returnValue" is unnecessary fluff that I have to read, that is less direct and clear than just doing the returns when you need them. Unless I read the whole function (in this case mercifully short) I don't know whether "returnValue" gets used for other things during the function.
Longer functions that use the "single exit point" rule can get really convoluted and difficult to understand. Having a function return as soon as it can improves the clarity because it improves the "locality" of the code. It's much the same philosophy as behind a previous article in the series - declaring variables as locally as possible.
At a return statement you know what the code is going to do next, you don't need to read through the rest of the function to work out that none of it's relevant for that path (well, there is an exception to that - a finally clause, but that's another article).
I've seen code that has become really convoluted as a result of trying to follow the "single exit point" rule and I just can't see any benefit.
What I'd like is for people to think about whether having a "single exit point" is a good thing rather than just believing the accepted wisdom and taking it as true. It's become such a strong "Cargo Cult" that it'll take many years until it disappears. To me it seems as unchallengable as the idea that water goes down a plug hole clockwise in one hemisphere and anti-clockwise in the other.
Posted by ivan at March 31, 2006 7:51 AMI am working at a place that has just brought in a new coding standard that enforces functions having only one exit point.
I tried to convince them not to have that rule, using the arguments you gave above. They just didn't seem to get it, they shot me down by pointing out that if you have to release resources before exit you have to do it at each return statement.
Of course it doesn't help that we are a c only shop.
To add insult to injury the also stopped me declaring variables as local as possible.
So you could say cargo cult programming is alive and well where I work.
Posted by: Lewis at March 31, 2006 9:52 AMI suggest some improvements on your sample code provided. Check out my comment at http://www.extragroup.de/weblog/hmk/archives/002997.html
Posted by: Hans Martin Kern at March 31, 2006 1:21 PMHi Hans,
Maybe my example was a bit too simple. Consider if the code had been returning the value of some function call (that you only want executed once) rather than literal 5 or 6. What does that do to the way you'd implement it?
Ivan
Posted by: ivan at March 31, 2006 1:43 PMThere is a trade-off here, and that is in the ability to debug easily the code. Assume first that your method returns a the result of invoking another method (e.g., return doSomethingElse();). With a declared variable it is much simple to step through your method and determine what the return value will be. In the latter case, both alt and F8 will cringe at the propsect of being pressed unnecessarily one more time simply to execute the doSomethingElse() method. In the interest of speeding up the entire design-build-test cycle, I prefer the latter, but not necessarily for the single point of exit benefit.
-Faiser
Posted by: Faiser at March 31, 2006 1:49 PMIvan, I wonder if exposure to functional programming has anything to do with prefering this style?
More on that thought at
http://peripateticaxiom.blogspot.com/2006/03/functional-style-and-multiple-returns.html
Hi Keith,
An interesting idea, but I don't think my preference is based on exposure to functional programming, I think it's just my attempt to write what I think will be simplest to read, and that has some parallels with a functional style in this case.
The way in which "single exit point" is a cargo cult is the large number of people who follow the "rule" without thinking. That was meant to be the message behind the article.
Ivan
Posted by: ivan at April 1, 2006 9:28 AMOne of the syntax/idioms I liked when reading about Eiffel was the "result" pseudo-variable used in functions. I have been using that in C, C++, Java, etc. and like it -- it's less confusing than uniquely-named variable used in function, and no harder than scanning the code for "return" statements.
( imagine good indentation being used...)
Bar foo( boolean condition ) {
Bar result = defaultValue;
Bar y = someStuff();
if ( condition() ) {
Bar x = complicatedStuff();
result = wowo() + x - y;
}
else if ( someOtherCondition() ) {
return = lala() - y;
}
return result;
}
Ivan,
I think I swould still go for the single-exit approach. Check my comments at http://www.extragroup.de/weblog/hmk/archives/002999.html.
Faiser has an interesting twist on the topic regarding debugging. That's another take on the problem I would second.
Posted by: Hans Martin Kern at April 2, 2006 6:39 PMKeith Ray, I don't now know what possessed me to call the variable "returnValue" rather than "result". When using a variable in this way I usually would call it "result" (honest) - good call.
However, I just don't see what the benefit of having a single exit point is supposed to be for those people who still follow this "rule". Basically, I see the single exit versions of the code and think "so what?". Of course you can have a single exit point, but why do you think that's a desirable property? The code from Keith Ray could be written:
Bar foo(boolean condition) {
Bar y = someStuff();
if (condition()) {
Bar x = complicatedStuff();
return wowo() + x - y;
} else if (someOtherCondition()) {
return lala() - y;
}
return defaultValue;
}
Why would that be worse? To me, it looks both simpler and shorter (very slightly).
Posted by: ivan at April 2, 2006 9:39 PMComing late to the party, I just put the ternary back in. I want to *return* something, dammit.
http://stevef.truemesh.com/archives/000612.html
Posted by: Steve Freeman at April 2, 2006 10:18 PM