Goto still considered helpful

[James Dow Allen]

On reading the entirety of James's article,
Richard concludes that James has completely failed to
convince Richard that James is an idiot. Would
James care to try again?

Sorry to take so long to get back to you
on this. Often my stupid-seemingest posts
are ones I spend effort preparing, but I've
been busy working on a book. (Nothing to do
with C language, all will be happy to note.)

I thought of replying to one of your posts showing
your gcc command line with

Didn't you forget an option?
-Wwarn_about_excessive_number_of_long-named_warning_options

but was afraid that might be judged as simply poor
humor rather than stupidity.

Here's my submission:

Firstly, I would just like to point out that it's rarely
a good idea to use goto (in fact, I've never come across
a good reason to use it in "real" code).

Let me refer Honourable Members to a
reply I gave some years ago in this ng:
http://groups.google.com/group/comp.lang.misc/msg/90274cfe18bb3117?dmode=source

Tim Rentsch posted a "structured alternative" to
my Favorite-Goto Bridge Solving Program. It and
the "unstructured alternative" are shown here:
http://james.fabpedigree.com/gotoalt.htm
I do hope readers will indicate which version
is "more readable."

As noted on the webpage, in order to get rid
of the 'goto' and its associated label, Tim's
version
replaces
* four "natural" for-loops (which work
together smoothly *because* of the goto)
with
* two for_ever loops nested inside a third
for_ever loop, as well as two break's, four
if's, two else's, and four inside-loop state
changing fragments (all required because the
"natural" for-loop headers were expunged
to get rid of goto);
replaces
* a single return at end of function
with
* a single return inside an inner loop;
replaces
* a single initialization
with
* two instances of this same initialization.

(I don't claim that Inside-loop returning or replicated
initialization is a fault of the same magnitude as goto,
just that the need for them in Tim's version suggests
that my version (with goto) better captures the program's
natural structure.)

James Dow Allen

 

[Army1987]

Tim Rentsch posted a "structured alternative" to
my Favorite-Goto Bridge Solving Program. It and
the "unstructured alternative" are shown here:
http://james.fabpedigree.com/gotoalt.htm
I do hope readers will indicate which version
is "more readable."

goto is only useful to break out of several statements at once.
Most often there is a better alternative. I fail to see a good
alternative in the code on that page, but maybe that's just
because I'm not in good shape today. (Curiously, that's very
similar to the only time I felt the need to use goto in my code.)

 

[Malcolm McLean]

goto is only useful to break out of several statements at once.
Most often there is a better alternative. I fail to see a good
alternative in the code on that page, but maybe that's just
because I'm not in good shape today. (Curiously, that's very
similar to the only time I felt the need to use goto in my code.)

I use it heavily as a poor man's exception handler.

typedef struct
{
int ***ptr;
} FOO;

FOO *foo(int x, y , z)
{
FOO *answer'
int i, ii;

answer = malloc(sizeof(answer));
if(!answer)
goto error_exit;
answer->ptr = malloc(z * sizeof(int **));
if(!answer->ptr)
goto error_exit;
for(i=0;i<z;i++)
answer->ptr = 0;
for(i=0;i<z;i++)
{
answer->ptr = malloc(y * sizeof(int *));
if(!answer->ptr)
goto error_exit;
for(ii=0;ii<y;ii++)
answer->ptr[ii] = 0;
for(ii=0;ii<y;ii++)
{
answer->ptr[ii] = malloc(x * sizeof(int));
if(!answer->ptr[ii])
goto error_exit;
}
}
return answer;
error_exit:
killfoo(answer);
return 0;
}

The advantage is that the code is boiler plate. Anything that builds
structures can follow this pattern - a function or two functions construct
it, another function destroys it, on a memory failure we jump to a lable
called error_exit, claean up, and return NULL.

It is still an unnacceptable amount of code merely to create a 3d array,
which in Java would be one line, plus a real exception handler somewhere in
the call stack, probably just one for the entire program terminating cleanly
on allocation failure.

 

[Kenneth Brody]

I use it heavily as a poor man's exception handler.

[... snip code example ...]

I, too, prefer such handling over using nesting conditional on
conditional to make sure that the errors/exceptions drop through
to the end of the function. And I especially prefer it over the
use of multiple returns.

Yes, it can lead to horrible spaghetti code (I know I've seen
plenty of that), but I like to think that I can judiciously use
"goto" when needed.

--
+-------------------------+--------------------+-----------------------+
| Kenneth J. Brody | www.hvcomputer.com | #include |
| kenbrody/at\spamcop.net | www.fptech.com | <std_disclaimer.h> |
+-------------------------+--------------------+-----------------------+
Don't e-mail me at: <mailto:[email protected]>

 

[Peter Pichler]

I, too, prefer such handling over using nesting conditional on
conditional to make sure that the errors/exceptions drop through
to the end of the function.

Don't knock nested ifs. Not only they are easier to read (personal
opinion, of course), but they also give you the opportunity to introduce
a new scope to declare your variables as locally as possible as a free
bonus, leading potentially to safer code.

Consider:

int fcopy (char const * dst, char const * src)
{
FILE * const sf = fopen(src, "r");
if (sf)
{
FILE * const df = fopen(dst, "w");
if (df)
{
/* "meat" of the intended file copy function
omitted for clarity */
fclose(df);
}
fclose(sf);
}
return /* value indicating success or failure */ 0;
}

versus:

int fcopy (char const * dst, char const * src)
{
FILE * sf /*= NULL? yuck!*/;
FILE * df /*= NULL? yuck!*/;

sf = fopen(src, "r");
if (!sf)
goto bail;

df = fopen(dst, "w");
if (!df)
{
fclose(sf);
goto bail;
}

/* "meat" of the intended file copy function
omitted for clarity */

fclose(df);
fclose(sf);

bail:
return /* value indicating success or failure */ 0;
}

Which one is more readable?
I usually start each nested conditional with a skeleton:

x = fopen/malloc/whatever(...);
if (x)
{
/* meat */
fclose/free/unwhatever(x);
}

and only when that is ready, I start adding the "meat" (which can be
another skeleton, for example).

All variables are declared as locally as possible, which also adds the
benefit that they can be initialized at the same time and often made
const (first step towards safety). Moreover, local variables *force* you
to free whatever resource they represent before leaving the scope
(second step towards safety).

(Sorry about the long article; I wrote it in a hurry and could not come
up with a shorter one.)

 

[CBFalconer]

Don't knock nested ifs. Not only they are easier to read (personal
opinion, of course), but they also give you the opportunity to
introduce a new scope to declare your variables as locally as
possible as a free bonus, leading potentially to safer code.

Consider:

int fcopy (char const * dst, char const * src)
{
FILE * const sf = fopen(src, "r");
if (sf)
{
FILE * const df = fopen(dst, "w");
if (df)
{
/* "meat" of the intended file copy function
omitted for clarity */
fclose(df);
}
fclose(sf);
}
return /* value indicating success or failure */ 0;
}

versus:

int fcopy (char const * dst, char const * src)
{
FILE * sf /*= NULL? yuck!*/;
FILE * df /*= NULL? yuck!*/;

sf = fopen(src, "r");
if (!sf)
goto bail;

df = fopen(dst, "w");
if (!df)
{
fclose(sf);
goto bail;
}

/* "meat" of the intended file copy function
omitted for clarity */

fclose(df);
fclose(sf);

bail:
return /* value indicating success or failure */ 0;
}

Which one is more readable?
I usually start each nested conditional with a skeleton:

You don't need all that confusing local declaration of variables,
etc. Consider:

int fcopy(char const *dst, char const *src) {
FILE *sf, *df;
int ch

if (sf = fopen(src, "r") { /* text files */
if (df = fopen(dst, "w") {
while (EOF != (ch = getc(sf))) putc(ch, df);
fclose(df);
}
fclose(sf);
}
return sf && df; /* non-zero for success */
}

which I consider more readable and safer than your version.

(Yes, I use goto where appropriate)

 

[inmatarian]

Don't knock nested ifs. Not only they are easier to read (personal
opinion, of course), but they also give you the opportunity to introduce
a new scope to declare your variables as locally as possible as a free
bonus, leading potentially to safer code.

Consider:

int fcopy (char const * dst, char const * src)
{
FILE * const sf = fopen(src, "r");
if (sf)
{
FILE * const df = fopen(dst, "w");
if (df)
{
/* "meat" of the intended file copy function
omitted for clarity */
fclose(df);
}
fclose(sf);
}
return /* value indicating success or failure */ 0;
}

versus:

int fcopy (char const * dst, char const * src)
{
FILE * sf /*= NULL? yuck!*/;
FILE * df /*= NULL? yuck!*/;

sf = fopen(src, "r");
if (!sf)
goto bail;

df = fopen(dst, "w");
if (!df)
{
fclose(sf);
goto bail;
}

/* "meat" of the intended file copy function
omitted for clarity */

fclose(df);
fclose(sf);

bail:
return /* value indicating success or failure */ 0;
}

Which one is more readable?
I usually start each nested conditional with a skeleton:

x = fopen/malloc/whatever(...);
if (x)
{
/* meat */
fclose/free/unwhatever(x);
}

and only when that is ready, I start adding the "meat" (which can be
another skeleton, for example).

All variables are declared as locally as possible, which also adds the
benefit that they can be initialized at the same time and often made
const (first step towards safety). Moreover, local variables *force* you
to free whatever resource they represent before leaving the scope
(second step towards safety).

(Sorry about the long article; I wrote it in a hurry and could not come
up with a shorter one.)

Is there a problem with the Immediately Crash principle? I could write
your function like so:

int fcopy (char const * dst, char const * src)
{
FILE * sf /*= NULL? yuck!*/;
FILE * df /*= NULL? yuck!*/;

sf = fopen(src, "r");
if (!sf)
return /* value indicating failure */ 1;

df = fopen(dst, "w");
if (!df)
{
fclose(sf);
return /* value indicating failure */ 1;
}

/* "meat" of the intended file copy function
omitted for clarity */

fclose(df);
fclose(sf);
return /* value indicating success */ 0;
}

I think a good lesson from all of this would be to recognize the need
for a goto statement as a sign your function is doing too much, instead
of the Do One Thing Well principle.

 

[Kenneth Brody]

Don't knock nested ifs. Not only they are easier to read (personal
opinion, of course), but they also give you the opportunity to introduce
a new scope to declare your variables as locally as possible as a free
bonus, leading potentially to safer code.

Consider:

int fcopy (char const * dst, char const * src)
{

[... clean nested-if version ...]

}

versus:

int fcopy (char const * dst, char const * src)
{ [... ugly goto version ...]
}

Which one is more readable?

[...]

Well, in simple cases as you've given, I would agree that the
nested-if approach is "better". However, I have cases which get
a lot more complex, and the judicious use of goto in those cases
makes for more readable, and "better" (IMO) code. Of course, one
could also argue that in the more complex cases, you could make
the code "even more betterer" by breaking it into several
functions.

Where the line is drawn is, of course, a matter of personal taste.

(Sorry about the long article; I wrote it in a hurry and could not come
up with a shorter one.)

No problem. I hope my trimming hasn't taken the spirit out of
your post.

(Hey, when did we get so polite and civilized around here?)

--
+-------------------------+--------------------+-----------------------+
| Kenneth J. Brody | www.hvcomputer.com | #include |
| kenbrody/at\spamcop.net | www.fptech.com | <std_disclaimer.h> |
+-------------------------+--------------------+-----------------------+
Don't e-mail me at: <mailto:[email protected]>

 

[Kenneth Brody]

[... nested-if versus goto ...]

Is there a problem with the Immediately Crash principle? I could write
your function like so:

int fcopy (char const * dst, char const * src)
{
FILE * sf /*= NULL? yuck!*/;
FILE * df /*= NULL? yuck!*/;

sf = fopen(src, "r");
if (!sf)
return /* value indicating failure */ 1;

df = fopen(dst, "w");
if (!df)
{
fclose(sf);
return /* value indicating failure */ 1;
}

/* "meat" of the intended file copy function
omitted for clarity */

fclose(df);
fclose(sf);
return /* value indicating success */ 0;
}

I think a good lesson from all of this would be to recognize the need
for a goto statement as a sign your function is doing too much, instead
of the Do One Thing Well principle.

Well, now you get into the "multiple returns" scenario, which is
met with as much disdain (if not more) than the use of goto.

Imagine extending your function to one which also did several
malloc()s for buffers, with each failure point now needing to do
cleanup of those things which succeeded before it. (ie: if the
first malloc fails, fclose the two streams; if the second malloc
fails, fclose the streams and free the first malloc; if the read
of the header fails, fclose the streams and free both buffers; and
so on.)

--
+-------------------------+--------------------+-----------------------+
| Kenneth J. Brody | www.hvcomputer.com | #include |
| kenbrody/at\spamcop.net | www.fptech.com | <std_disclaimer.h> |
+-------------------------+--------------------+-----------------------+
Don't e-mail me at: <mailto:[email protected]>

 

[Richard Heathfield]

Kenneth Brody said:

(Hey, when did we get so polite and civilized around here?)

Quite a few clcers have always (or mostly always) been polite and civilised
- top of the pile is Chris Torek, of course, but there are lots of others
who qualify. They also tend to be the ones who know about C.

The impolite, uncivilised ones are those generally referred to as trolls,
although that's probably unfair on at least one of them. But they needn't
bother you, since they're not interested in discussing C; they are far
more concerned with complaining about the fact that we don't discuss
anything else!

 

[Ed Jensen]

[SNIP: Examples]

Which one is more readable?

I actually found your version a little tricky to read. I find
functions more readable if all the clean up happens in one place.

It took me a few seconds to make sure your files were properly closed
no matter what series of events happened.

Using your approach, things would get more tricky pretty quickly if
more resources were added to the function, such as dynamically
allocated read and write buffers.

Also, your approach unnecessarily duplicates code. Even your simple
example contains multiple calls to close the very same file.

I like the following pattern, which keeps the resource clean up in one
place, and the pattern scales well if more resources are added to the
function in the future:

int fcopy(char const *dst, char const *src)
{
FILE *sf = NULL;
FILE *df = NULL;

sf = fopen(src, "r");
if (!sf)
goto done;

df = fopen(dst, "w");
if (!df)
goto done;

/* copy happens here */

done:
if (sf)
fclose(sf);
if (df)
fclose(df);

return /* value indicating success or failure */ 0;
}

 

[inmatarian]

Well, now you get into the "multiple returns" scenario, which is
met with as much disdain (if not more) than the use of goto.

Imagine extending your function to one which also did several
malloc()s for buffers, with each failure point now needing to do
cleanup of those things which succeeded before it. (ie: if the
first malloc fails, fclose the two streams; if the second malloc
fails, fclose the streams and free the first malloc; if the read
of the header fails, fclose the streams and free both buffers; and
so on.)

Granted, although you would have a big group of fcloses and frees in any
of the forms of this particular function. I'm not saying either way is
particularly better, and I'm sure gotos have their place, but I'd
recommend deferrence to Eric S Raymond's rules, or whoever your favorite
pioneer is, they all said basically the same thing. In this case, I'm
citing two rules:

Rule of Repair: When you must fail, fail noisily and as soon as possible.

Rule of Modularity: Write simple parts connected by clean interfaces.

Having three returns in a file copy function isn't all that bad, though
I'd agree that when you start pushing it on multiple returns, that
probably fails Rule of Modularity, in that the function is doing too much.

 

[Peter Pichler]

done:
if (sf)
fclose(sf);
if (df)
fclose(df);

As you wish. I do not like this approach because it requires all
declarations at the top of the function, which IMO is almost as bad
as making them global. In addition, it requires them initialised
to "harmless" values which can hide bugs later on. Of course, YMMV.
De gustibus non disputandum est.

 

[CBFalconer]

inmatarian wrote:

[... nested-if versus goto ...]

Is there a problem with the Immediately Crash principle? I could
write your function like so:

int fcopy (char const * dst, char const * src)
{
FILE * sf /*= NULL? yuck!*/;
FILE * df /*= NULL? yuck!*/;

sf = fopen(src, "r");
if (!sf)
return /* value indicating failure */ 1;

df = fopen(dst, "w");
if (!df)
{
fclose(sf);
return /* value indicating failure */ 1;
}

/* "meat" of the intended file copy function
omitted for clarity */

fclose(df);
fclose(sf);
return /* value indicating success */ 0;
}

I think a good lesson from all of this would be to recognize the
need for a goto statement as a sign your function is doing too
much, instead of the Do One Thing Well principle.

Well, now you get into the "multiple returns" scenario, which is
met with as much disdain (if not more) than the use of goto.

So look at the following, with a single return, and no complexity:

int fcopy(const char *dst, const char *src) {
FILE *sf, *df;
int err, ch;

if (!(sf = fopen(src, "r"))) err = 1;
else if (!(df = fopen(dst, "w"))) err = 2;
else {
while (EOF != (ch = getc(sf))) putc(ch, df);
err = 0;
fclose(df);
}
if (sf) fclose(sf);
return err;
}

 

[¬a\\/b]

In data Mon, 15 Oct 2007 17:09:19 -0000, Ed Jensen scrisse:

my

I actually found your version a little tricky to read. I find
functions more readable if all the clean up happens in one place.

It took me a few seconds to make sure your files were properly closed
no matter what series of events happened.
hahahahha

Using your approach, things would get more tricky pretty quickly if
more resources were added to the function, such as dynamically
allocated read and write buffers.

so not is complete

Also, your approach unnecessarily duplicates code. Even your simple
example contains multiple calls to close the very same file.

I like the following pattern, which keeps the resource clean up in one
place, and the pattern scales well if more resources are added to the
function in the future:

int fcopy(char const *dst, char const *src)
{
FILE *sf = NULL;
FILE *df = NULL;

sf = fopen(src, "r");
if (!sf)
goto done;

df = fopen(dst, "w");
if (!df)
goto done;

/* copy happens here */

done:
if (sf)
fclose(sf);
if (df)
fclose(df);

return /* value indicating success or failure */ 0;
}

in my almost imagenary language it is 2 lines
int fcopy(char* dst, char* src)
{istream sf(dst); ostream df(src); return (int)(sf>>df);}

the goto should be use to implemet all the rest
while, for, and in general loops are worse than gotos

all in programming area is only use well goto and jumps

 

[Ben Pfaff]

int fcopy(char const *dst, char const *src) {
FILE *sf, *df;
int ch

if (sf = fopen(src, "r") { /* text files */
if (df = fopen(dst, "w") {
while (EOF != (ch = getc(sf))) putc(ch, df);
fclose(df);
}
fclose(sf);
}
return sf && df; /* non-zero for success */

I think that this is a bad idea: it is a lot like checking the
value of a pointer after it has been freed (often fclose will
actually call free on the stream, in fact). I'm sure it works in
practice on almost every implementation though.

 

[Keith Thompson]

I think that this is a bad idea: it is a lot like checking the
value of a pointer after it has been freed (often fclose will
actually call free on the stream, in fact). I'm sure it works in
practice on almost every implementation though.

If the first fopen() fails, no value is assigned to df, and accessing
it invokes undefined behavior.

In addition, I think you're write; though fclose() cannot change the
value of of its argument, it can cause it to become indeterminate.
(In practice, it will probably appear to be be null or non-null after
fclose() if it was, respectively, null or non-null before fclose().)

 

[CBFalconer]

I think that this is a bad idea: it is a lot like checking the
value of a pointer after it has been freed (often fclose will
actually call free on the stream, in fact). I'm sure it works in
practice on almost every implementation though.

A point. The alternative is a local 'err' variable. I was mainly
interested in proper error exiting without grotesque code
structure.

 

[CBFalconer]

If the first fopen() fails, no value is assigned to df, and
accessing it invokes undefined behavior.

Note the order of access in the return statement, and use of the &&
operator. So df is not accessed in that case.

 

[Keith Thompson]

In addition, I think you're write;

[...]

Did I really write "write" instead of "right"? Sigh.