Goto still considered helpful

  • Thread starter James Dow Allen
  • Start date
T

Tor Rustad

CBFalconer wrote:

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

How do you measure 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);

FYI, compression of the source, does not lower the intrinsic complexity
of the code.


IMO, you made it too simple.
 
T

Tor Rustad

CBFalconer wrote:

[...]
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. :)

This is good-weather code, and has nothing to do with "safe" code in
production.

Just on my first read, I think you need to consider:

- fopen failure
- getc failure
- putc failure

far more carefully.
 
K

Keith Thompson

Tor Rustad said:
CBFalconer wrote:
[...]
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. :)

This is good-weather code, and has nothing to do with "safe" code in
production.

Just on my first read, I think you need to consider:

- fopen failure
- getc failure
- putc failure

far more carefully.

I believe it already handles fopen failure correctly.

I agree about getc and putc.
 
T

Tor Rustad

Keith said:
Tor Rustad said:
CBFalconer wrote:
[...]
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. :)
This is good-weather code, and has nothing to do with "safe" code in
production.

Just on my first read, I think you need to consider:

- fopen failure
- getc failure
- putc failure

far more carefully.

I believe it already handles fopen failure correctly.

Hmm... I really hate C code like this:

return sf && df;

OK, I see. 'sf' will be NULL on the first fopen failure, and 'df' is not
evaluated. <g>
 
S

santosh

Keith said:
Tor Rustad said:
CBFalconer wrote:
[...]
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. :)

This is good-weather code, and has nothing to do with "safe" code in
production.

Just on my first read, I think you need to consider:

- fopen failure
- getc failure
- putc failure

far more carefully.

I believe it already handles fopen failure correctly.

I agree about getc and putc.

The code snippet wont compile.
 
C

CBFalconer

Keith said:
Tor Rustad said:
CBFalconer wrote:
[...]
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. :)

This is good-weather code, and has nothing to do with "safe" code in
production.

Just on my first read, I think you need to consider:

- fopen failure
- getc failure
- putc failure

far more carefully.

I believe it already handles fopen failure correctly.

I agree about getc and putc.

It also handles getc failure. putc failure is not handled. The
handling technique is simply to abort the function execution.
fopen failure is signalled, the others are not.

putc can be handled by "if (EOF != putc(ch, df)) {
/* error action */
break;
}"

and similar testing after the while loop can give details about a
getc failure. Most systems don't have any use for all this, and
function exit suffices.
 
C

CBFalconer

santosh said:
Keith said:
Tor Rustad said:
CBFalconer wrote:
[...]

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

This is good-weather code, and has nothing to do with "safe"
code in production.

Just on my first read, I think you need to consider:

- fopen failure
- getc failure
- putc failure

far more carefully.

I believe it already handles fopen failure correctly.

I agree about getc and putc.

The code snippet wont compile.

Yes, there are a couple of missing ')'s. Penalty for typing into
the newsreader. The above errors (apart from putc) already force
function termination.
 
R

Richard

CBFalconer said:
I am not concerned with debuggery. I haven't used one for two
years. I am concerned with readability, and to me good readability

Then you should probably not be commenting. Maintainable and debuggable
code is a very important asset in the real word. The fact that you are
not concerned with debugging means your views are biased and, frankly,
amateurish.
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.

Having multiple statements on one line rarely, if ever, makes the code
more readable. I suspect from your comments you have little, if any,
experience of working on large distributed code bases shared by
tens or even hundreds of programmers.
I think most people will, on rereading my original post (quoted
above), agree that the code is clear. Each line performs a

No. Most certainly not. It is easy to determine, but "at a glance" it is
far from clear.
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.

Wrong. It has been proven, time and time again to work and benefit the
majority. I have never, ever worked on a project where multiple
statements on a line are considered good practice. Ever.

Yes, people can disagree all they like. But one thing experience has
proven to me is that there will always be people who will argue the toss
if only to appear a little "maverick" or different.

Multiple lines in C are bad. They are hard to read. They make spotting
the flow difficult. And they make debugging harder than it need be.

Altering code for debugging is heresy and is frowned upon in any real SW
shop. It is a source of bugs and a nightmare for version
tracking. Recompile with some flags on, maybe, but to modify the lines
just to make it easier to debug? Amateur at best and incredibly naive.
 
K

Keith Thompson

CBFalconer said:
Keith Thompson wrote: [...]
CBFalconer wrote:
[...]
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. :)
[snip]
I believe it already handles fopen failure correctly.

I agree about getc and putc.

It also handles getc failure. putc failure is not handled. The
handling technique is simply to abort the function execution.
fopen failure is signalled, the others are not.

putc can be handled by "if (EOF != putc(ch, df)) {
/* error action */
break;
}"

and similar testing after the while loop can give details about a
getc failure. Most systems don't have any use for all this, and
function exit suffices.

It doesn't distinguish between an error in getc() and reaching
end-of-file.

putc() can fail if, for example, the target file system is full. This
is an important condition to check. Such a failure can also show up
in fclose(), which should also be checked.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,774
Messages
2,569,596
Members
45,140
Latest member
SweetcalmCBDreview
Top