Goto still considered helpful

[Flash Gordon]

Keith Thompson wrote, On 19/10/07 07:29:

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

If the first fopen fails sf will be a null pointer and short circuit
evaluation ensures that df is not accessed, so it is OK.

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().)

Agreed.

 

[James Dow Allen]

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.

This thread has generated several posts on a
simple goto which can be replaced simply,
but only one comment about the more interesting
goto. Come on, don't be shy!

Counting previous posts, I've received about 12 responses
on this code. Perhaps Army1987's unusually tepid
response counts as favorable. I'm grateful to Tim,
the *only* one who's ever taken the trouble to produce
a goto-free version. The other responses I've
received divide equally into two categories.

Half the responses, paraphrased, were:

replace the common code with a function call [or
some other response proving that respondent was
too lazy or incompetent to grok the program at all] The other half had the form:
fixing your ugly code would be a minute's child
play but I'm too busy now [presumably busy
constructing a 99th diatribe about ignoramuses
who write "void main()" ?]

Of course what I'd like to see is:

A special convocation of C experts declared today
that Jamie's GOTO is the most readable way to express
a bridge solving program. Cornelia Dijkstra made
a special appearance; she regretted that her grandfather
couldn't come to apologize to Mr. Allen in person.
Ms. Dijkstra has submitted a retraction to the ACM
Bulletin on her grandfather's behalf.

But I'd be willing to live with (and indeed could then
promise to never mention GOTO in this newsgroup again)
a response like:

After careful objective scrutiny, 4 out of 5 c.l.c
experts agreed that Mr. Rentsch's submission was
more readable than Mr. Allen's. In the words of
long-time c.l.c guru Eric B. Keithfield:

What I frankly find irritating are responses like

fixing your ugly code would be a minute's child
play but I'm too busy now....
...
... [wait a few months, then post in another thread]
I've never come across
a good reason to use goto in "real" code).

In addition, I think you're write; ...
Did I really write "write" instead of "right"? Sigh.

Lately I write "their" for "there", "are" for "or"
and even "haven't" for "having". Is there a name for
this condition? I'm also losing my sense of smell.

How old are you, Keith?

James Dow Allen

 

[Richard]

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;
}

To any nOOB out there, please never, ever follow the style above. It is
tantamount to blasphemy to put more than one statement on a line in C
with a condition.

e.g

if (sf) fclose(sf);

SHOULD be in any decent code base

if (sf)
fclose(sf);

Why? So that in the case of stepping through in a debugger you can see
if the fclose was executed or not. There is a group in c.l.c that think
using a debugger is "twice as hard as writing the code in the first
place" and apparently they never, ever use debuggers. I think they are
dreaming. While there may be an element of truth in doing it right the
first time, it bears no scrutiny in the real world where other people
have to debug other peoples code all the time a long time after it was
written. Be structuring your code to be debugger friendly, you will earn
the thanks of many, many people in the years to come.

 

[Keith Thompson]

Lately I write "their" for "there", "are" for "or"
and even "haven't" for "having". Is there a name for
this condition? I'm also losing my sense of smell.

How old are you, Keith?

Older than I've ever been.

 

[Keith Thompson]

Keith Thompson wrote, On 19/10/07 07:29:

If the first fopen fails sf will be a null pointer and short circuit
evaluation ensures that df is not accessed, so it is OK.

[...]

You're right.

 

[Richard Bos]

To any nOOB out there, please never, ever follow the style above. It is
tantamount to blasphemy to put more than one statement on a line in C
with a condition.

Erm... that may be _your_ opinion, but it's hardly law.

SHOULD be in any decent code base

if (sf)
fclose(sf);

Depends entirely on the circumstances. If you have six of them in a row,
all similar, the first one is much neater than the second.

Why? So that in the case of stepping through in a debugger you can see
if the fclose was executed or not.

Get a real debugger, then. Programming tools are there to make writing
code easier, not to put demands on my style.

Richard

 

[Richard Bos]

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.

Beg to differ. Beg to differ very greatly, in fact. Declarations
scattered through the code hailshot-wise are a great way of ensuring
that you'll have to hunt for one sooner or later.

In addition, it requires them initialised to "harmless" values
which can hide bugs later on.

No, it doesn't. For example, in the code from which the above is quoted,
the initialisations of sf and df are unnecessary: fopen() will always
return either a valid FILE *, or a null pointer, never garbage.

Richard

 

[Richard]

Erm... that may be _your_ opinion, but it's hardly law.

No, like any style its not law. But after years of programming in
various sectors and languages it is, IMO, better to make the code
debugger friendly for those that follow.

Depends entirely on the circumstances. If you have six of them in a row,
all similar, the first one is much neater than the second.

Neatness is nothing to do with it. Although neatness can also play a
part in readability and maintainability. This is not one of those
moments though. Its about being debugger friendly. Nothing more. (I also
find it MUCH easier to read the code too, but thats another issue).

Get a real debugger, then. Programming tools are there to make writing
code easier, not to put demands on my style.

What is a "real debugger" and which "real debugger" do you recommend?

All debuggers I have used will step by line as the default and its far
easier when the outcome of the conditional is on another line - you don't
have to *think* about "stepping in" or any some such.

I would question the merits of any programmer who thinks

if(f)func();

on one line is better for code maintenance than

if(f)
func();

Richard

richard.

 

[Richard]

Beg to differ. Beg to differ very greatly, in fact. Declarations
scattered through the code hailshot-wise are a great way of ensuring
that you'll have to hunt for one sooner or later.

Not with any half decent editor. Besides, locally declared variables
don't need to be hunted for. They are there where you need
them. Declaring all variables in one go at the top is bad for
maintenance and leads to confusion IMO. Declare the variable at the last
possible moment. It also keeps the locals display in your debugger
cleaner and only showing what is pertinent to the current context.

 

[Richard Bos]

I would question the merits of any programmer who thinks

if(f)func();

on one line is better for code maintenance than

if(f)
func();

Of course you would. They're both evil. It should be either

if (f) func();

or

if (f)
func();

depending on context.

Any style that does not allow for "depending on context" is for the
robots, not for humans like me.

Richard

 

[Richard Bos]

Not with any half decent editor.

Ah, yes, I forgot. Desk checks have gone out of fashion. Because they
required that you were, you know, competent. Can't be having that.

Besides, locally declared variables don't need to be hunted for.
They are there where you need them.

Your confidence in your fellow programmer is endearing, but ill
deserved.

Richard

 

[Richard]

Ah, yes, I forgot. Desk checks have gone out of fashion. Because they
required that you were, you know, competent. Can't be having that.


Your confidence in your fellow programmer is endearing, but ill
deserved.

Sorry? I am advocating things to improve things. I am not showing
confidence in anything.

 

[Richard]

Of course you would. They're both evil. It should be either

if (f) func();

or

if (f)
func();

depending on context.

What context dictates making the code debugger unfriendly? And one
liners DO do just that.

Any style that does not allow for "depending on context" is for the
robots, not for humans like me.

Any human that doesn't see the need for a consistent, conforming code
base which adhere to coding rules which are engineered to improve the
accountability and maintainability of a professional code base has no
business contributing to that code base.

 

[Ben Bacarisse]

You're right.

....but it is still a syntax error!

 

[Richard Harter]

To any nOOB out there, please never, ever follow the style above. It is
tantamount to blasphemy to put more than one statement on a line in C
with a condition.

e.g


SHOULD be in any decent code base

if (sf)
fclose(sf);

Well, no, it shouldn't. If you are going to put the statement on
a separate line put it in braces, e.g.,

if (sf) {
fclose(sf);
};

Not having them there is asking for a surprise - it looks like
the statement is in a block but it isn't. Some take the view
that one should alway uses braces for the if body - the single
line if is a little maintenance trap waiting to happen. I, and
perhaps many others, don't share your concern for being "debugger
friendly" but that's another matter.

Why? So that in the case of stepping through in a debugger you can see
if the fclose was executed or not. There is a group in c.l.c that think
using a debugger is "twice as hard as writing the code in the first
place" and apparently they never, ever use debuggers. I think they are
dreaming. While there may be an element of truth in doing it right the
first time, it bears no scrutiny in the real world where other people
have to debug other peoples code all the time a long time after it was
written. Be structuring your code to be debugger friendly, you will earn
the thanks of many, many people in the years to come.

Richard Harter, (e-mail address removed)
http://home.tiac.net/~cri, http://www.varinoma.com
In the fields of Hell where the grass grows high
Are the graves of dreams allowed to die

 

[Richard]

Well, no, it shouldn't. If you are going to put the statement on
a separate line put it in braces, e.g.,

if (sf) {
fclose(sf);
};

I agree with you and do do that personally, but it wasn't so
important to the point I was making about the debugger steps.

Although I wouldn't include the last semi colon .....

 

[Joachim Schmitz]

Lately I write "their" for "there", "are" for "or"
and even "haven't" for "having". Is there a name for
this condition? I'm also losing my sense of smell.

Alzheimers, or some other for of dementia? Esp. loosing sense of smell is an
early symptom, and I'm not joking here...

Bye, Jojo

 

[CBFalconer]

Well, no, it shouldn't. If you are going to put the statement on
a separate line put it in braces, e.g.,

if (sf) {
fclose(sf);
};

Not having them there is asking for a surprise - it looks like
the statement is in a block but it isn't. Some take the view
that one should alway uses braces for the if body - the single
line if is a little maintenance trap waiting to happen. I, and
perhaps many others, don't share your concern for being "debugger
friendly" but that's another matter.

I am not concerned with debuggery. I haven't used one for two
years. I am concerned with readability, and to me good readability
requires minimizing vertical creep and expansion. Thus I will
normally use the 'one line' condition and single action statement
construct. Also, should I really need to delve into the debugged
action of such a statement, I can easily alter the source and
recompile.

I think most people will, on rereading my original post (quoted
above), agree that the code is clear. Each line performs a
function, possibly combined with a test. The alternative paths are
clear. I consider it an excellent style example. Richards
overconcern with fixed and pointless formatting dicta will simply
cause confusion.

BTW, my formatting style is nearly achieved by the following
control file (indent.pro) for the indent utility, GNU version
2.2.9. It is a single line.

-kr -l66 -i3 -bad -di16 -lc66 -nce -ncs -cbi0 -bbo -pmt -psl -ts1
-cdw -ppi 3

 

[Peter Pichler]

In addition, [this style] requires [all variables] initialised to "harmless" values
which can hide bugs later on.

No, it doesn't. For example, in the code from which the above is quoted,
the initialisations of sf and df are unnecessary: fopen() will always
return either a valid FILE *, or a null pointer, never garbage.

The context was lost in snipping. It went something like this:

type1 var1;
type2 var2; /* you need, "= safe_value;" here */

initialize(var1); /* what if this fails? */
if (wrong(var1))
goto end;
initialize(var2);
if (wrong(var2))
goto end;

do_something(var1, var2);

end:
if (need_deinitialize(var1))
deinitialize(var1);
if (need_deinitialize(var2)) /* BANG! (maybe) */
deinitialize(var2);

Now, if initializing var1 fails, var2 remains unitialized and your
cleanup section is a potential trouble. Therefore you need to initialize
var2. With more than two variables, you need to initialize all of them
before jumping to end.

Declaring variables as local as possible avoids this problem.

(I am not fighting a holy war, only explaining.)

 

[Peter Pichler]

Alzheimers, or some other for of dementia? Esp. loosing sense of smell is an
early symptom, and I'm not joking here...

Alzheimer? Isn't that the German guy who follows me around and hides things?