Damn Skippy wrote:
instead of using 'break' in the pseudocode below, i would prefer using
'goto'.
A break (or a continue) is just a goto in disguise. In some ways they're
worse - at least a goto has a label associated with it which you can
give a meaningful name. I never use either, except, of course, break in
a switch statment.
Suggestions appreciated.
Since you asked...
So you must decide what is readable
That's what I'm after. In most code, I'll cheerfully accept a very small
performance impact in exchange for even a very small gain in readability.
============ inefficient, inelegant, recommended ===========
I'd never recommend this....
for ( .. ) {
if ( a != b ) {
good=false;
complain();
break;
}
}
if (good) {
process();
}
cleanup_regardless:
cleanup();
============ better? saves one test and one stack variable ===========
Yes, if modified as described further down in my response.
for ( .. ) {
if ( a != b ) {
complain();
goto cleanup_regardless;
}
}
process();
cleanup_regardless:
cleanup();
============ ===========
What both of the above are missing is an explicit statement of what gets
you out of the "for" loop, so the next person to work on this code has
no help in understanding what this VERY IMPORTANT condition is that
suddenly changes flow control. Assuming that "a != b" means that the
processor is in overload, I'd write it as something like:
olvdState = STABLE;
for ( ..; .. && ovldState == STABLE; .. ) {
if ( a != b ) {
ovldState = OVERLOADED;
complain();
}
}
if (ovldState = STABLE) {
process();
}
cleanup();
If you had written your goto-version like this:
for ( ..; ..; .. ) {
if ( a != b ) {
complain();
goto overloaded;
}
}
process();
overloaded:
cleanup();
then it's pretty close to the version I wrote without gotos in that it
explicitly tells the reader WHY you're leaving the loop (as opposed to
just what you're going to do after you leave the loop, which is already
clear from the "cleanup()" function name) and yes, I think it's better
than your original version that uses a break.
The main reason I prefer the version that uses a variable is that it
forces the original code author to explicitly state the VERY IMPORTANT
loop exit conditions so the next reader isn't fooled into thinking they
know exactly why the loop terminates until they're in the middle of the
loop and suddenly come across an unexpected, probably unnamed, loop exit.
Sure it's a little more work for the original author to come up with
good names for the variable and it's values, but that's better than the
next guy having to potentially take much longer to have to figure out
that abstraction, possibly under customer pressure to fix or enhance
that and/or other code.
People using gotos typically (but, I assume, not always) create their
labels based on WHAT they're doing at that labelled point rather than
WHY they're doing it. Not being a goto officianado, I can't decide if
that's the right thing to do with a goto or not - you'd end up with more
labels if you use the "WHY" (e.g. since there could be many reasons for
going to a "cleanup" routine) but IMHO you'd have more readable code.
of course, i hope i never see gotos in something like:
if ( (fp = fopen(szFilename)) != NULL ) {
if ( fread (&buf, 1, len, fp) == len ) {
if ( !strncmp ( buf, 4, "JFIF") ) {
process_jfif();
}
}
}
Then don't look at the URL posted elsethread (shudder...) ;-).
Ed.