Recommended style

[Kelvin Moss]

Hi all,

Which is the recommended style in production code for the following -

....
fp = fopen("xyz.txt" , "r");
if (fp == NULL) {
....
}

OR
fp = fopen("xyz.txt" , "r");
if (!fp) {
....
}

Thanks.

[vir]

Mike Wahler wrote:

"Emmanuel Delahaye" <em***@YOURBRAn oos.fr> wrote in message
news:mn******** *************** @YOURBRAnoos.fr ...

Victor Nazarov wrote on 01/01/05 :

I usually use this:

{
fp = fopen (fn, fmode);
if (!fp) {
error ();
goto fail;
}
rc = foo (fp);
fail:
if (!fp)
fclose (fp);
}

Isn't it a little bit complicated for the purpose ? Or is it a troll ?

Probably a troll. Note the OP's "name".

What's wrong with the name?

This style is to be used for a functions allocating several resources...
Think of

int foo (int len)
{
FILE *f1p = NULL, *f2p = NULL;
char *s = NULL;
int res = -1;

f1p = fopen (fn, fmode);
if (!f1p) {
error ();
goto fail;
}
f2p = fopen (fn, fmode);
if (!f2p) {
error ();
goto fail;
}
s = malloc (len);
if (!s) {
error ();
goto fail;
}
/*
...
Some valuable actions
...
*/
res = 0
fail:
free (s);
if (!f2p)
fclose (f2p);
if (!f1p)
fclose (f1p);
return res;
}

Cascaded function calls seems to be less readable then one function.

--
vir

[Mike Wahler]

"Herbert Rosenau" <os****@pc-rosenau.de> wrote in message
news:wm******** *************** ****@URANUS1.DV-ROSENAU.DE...

On Sun, 2 Jan 2005 08:46:10 UTC, Michael Mair
<Mi**********@i nvalid.invalid> wrote:

if (!x) or in long if (x != 0)

if (!x) is equivalent to if (x == 0)

No, it's not, it is highly different.

Really?

ISO 9899:1999

6.5.3.3 Unary arithmetic operators

5 The result of the logical negation operator ! is 0
if the value of its operand compares unequal to 0,
1 if the value of its operand compares equal to 0.
The result has type int. The expression !E is
equivalent to (0==E).
-Mike

[Allin Cottrell]

Lawrence Kirby quoted:

if((fp=fopen (filename,"mode "))==NULL)

and commented

That's a clear, concise C idiom, but please locate your space bar.
LK again quoted:
if ( NULL == ( fp = fopen( filename, mode ) ) ) {

and commented:
Whereas that's just contorted. The point of space is to help the eye see
the natural grouping. Put space around everything and you're no better of
than not using spaces at all, perhaps worse.

Agreed, absolutely. If I were writing the first of these examples I
would write

if ((fp = fopen(filename, "mode")) == NULL)

and if I were writing the second,

if (NULL == (fp = fopen(filename, mode))) {

BUT... if I were writing for comprehensibili ty I would eschew both
and write:

fp = fopen(filename, "mode");
if (fp == NULL) {
/* respond to error condition */

There's just too great an opportunity for error in the "cutesy"
versions of this code, which roll the basic assignment into the
detection of an error condition. The immature programmer thinks,
"Look how smart I am, how many things I can do at once!". The
mature programmer thinks, "Let's do one thing at a time, in such
a way that it's quite transparent what we're doing; and let's
get it RIGHT."

IMO.

Allin Cottrell

[Michael Wojcik]

In article <cr***********@ news.comcor-tv.ru>, vir <vv*****@mail.r u> writes:

This style is to be used for a functions allocating several resources...
Think of

int foo (int len)
{
FILE *f1p = NULL, *f2p = NULL;
char *s = NULL;
int res = -1;

f1p = fopen (fn, fmode);
if (!f1p) {
error ();
goto fail;
}
...
fail:
free (s);
if (!f2p)
fclose (f2p);
if (!f1p)
fclose (f1p);
return res;
}
I also use this single-point-of-return style (though I employ it
slightly differently) in many cases, and consider it the most
prominent excuse for employing goto, as those who remember our
most recent "goto" flame war know. However, note that
if (!f2p)
fclose (f2p);
if (!f1p)
fclose (f1p);

should be

if (f2p)
fclose (f2p);
if (f1p)
fclose (f1p);

since obviously you want to call fclose only if the FILE* variable
is NOT null. And that, I suppose, demonstrates that correctness
still trumps style.

(Personally, I wish the committee had defined behavior for null
arguments for fclose and some of the other functions, as they did
for free, if only to get rid of those guard if's. But we have the
language we have.)

--
Michael Wojcik mi************@ microfocus.com

When most of what you do is a bit of a fraud, the word "profession "
starts to look like the Berlin Wall. -- Tawada Yoko (t. M. Mitsutani)

[Eric Sosman]

Victor Nazarov wrote:

I usually use this:

{
fp = fopen (fn, fmode);
if (!fp) {
error ();
goto fail;
}
rc = foo (fp);
fail:
if (!fp)
fclose (fp);
}

Stylish, indeed! It verges upon the elegant! There
is, however, one tiny drawback: It's wrong.

"Other than that, Mrs. Lincoln, what did you think
of the play?"

--
Er*********@sun .com

[Ben Pfaff]

Eric Sosman <er*********@su n.com> writes:

Victor Nazarov wrote:

I usually use this:

{
fp = fopen (fn, fmode);
if (!fp) {
error ();
goto fail;
}
rc = foo (fp);
fail:
if (!fp)
fclose (fp);
}

Stylish, indeed! It verges upon the elegant! There
is, however, one tiny drawback: It's wrong.

It can be made correct by deleting one character.
--
"I should killfile you where you stand, worthless human." --Kaz

[Eric Sosman]

Ben Pfaff wrote:

Eric Sosman <er*********@su n.com> writes:

Victor Nazarov wrote:

I usually use this:

{
fp = fopen (fn, fmode);
if (!fp) {
error ();
goto fail;
}
rc = foo (fp);
fail:
if (!fp)
fclose (fp);
}

Stylish, indeed! It verges upon the elegant! There
is, however, one tiny drawback: It's wrong.

It can be made correct by deleting one character.

Or by repeating it, if "deleting a character" is
too gruesome in the context of the play (a remark your
good taste, er, deleted ...)

This sort of mistake is, I think, a good reason
to prefer `x == 0' (or `0 == x' if a backward-thinking
person you are, young Skywalker) over `!x' when `x'
isn't "obviously boolean." I particularly dislike

if (!strcmp(passwo rd, "drowssap") )

.... which tests for equality but appears on a hasty
reading to do the opposite.

--
Er*********@sun .com

[vir]

Michael Wojcik wrote:

In article <cr***********@ news.comcor-tv.ru>, vir <vv*****@mail.r u> writes:

This style is to be used for a functions allocating several resources...
Think of

int foo (int len)
{
FILE *f1p = NULL, *f2p = NULL;
char *s = NULL;
int res = -1;

f1p = fopen (fn, fmode);
if (!f1p) {
error ();
goto fail;
}
...
fail:
free (s);
if (!f2p)
fclose (f2p);
if (!f1p)
fclose (f1p);
return res;
}

I also use this single-point-of-return style (though I employ it
slightly differently) in many cases, and consider it the most
prominent excuse for employing goto, as those who remember our
most recent "goto" flame war know.

I havn't read the thread about goto. But if I get read of goto statement
I'll need an exception mechanism in C...
However, note that

if (!f2p)
fclose (f2p);
if (!f1p)
fclose (f1p);

should be

if (f2p)
fclose (f2p);
if (f1p)
fclose (f1p);

since obviously you want to call fclose only if the FILE* variable
is NOT null.

Yes, sorry for this mistake.
And that, I suppose, demonstrates that correctness
still trumps style.

(Personally, I wish the committee had defined behavior for null
arguments for fclose and some of the other functions, as they did
for free, if only to get rid of those guard if's. But we have the
language we have.)

Agread. In my own code I allways write destructors accepting invalid
value (NULL in most cases).

--
vir

[pete]

Andrey Tarasevich wrote:

Kelvin Moss wrote:

Which is the recommended style in production code for the following -

...
fp = fopen("xyz.txt" , "r");
if (fp == NULL) {
...
}

OR
fp = fopen("xyz.txt" , "r");
if (!fp) {
...
}
...

It is a matter of a personal preference and/or coding standard.

Personally, I leave out explicit comparison for values with pronounced
'boolean' semantics only. All other values require explicit comparison
in my book. That would mean that I'd choose the first variant.

Me too.

[Michael Wojcik]

In article <cr**********@n ews1brm.Central .Sun.COM>, Eric Sosman <er*********@su n.com> writes:

Victor Nazarov wrote:

if (!fp)
fclose (fp);

This sort of mistake is, I think, a good reason
to prefer `x == 0' (or `0 == x' if a backward-thinking
person you are, young Skywalker) over `!x' when `x'
isn't "obviously boolean."

I'm not convinced that would significantly reduce this sort of error.
I strongly doubt it would for me (though I can't recall having made
this particular mistake; doesn't mean I haven't, but it hasn't happened
often or recently enough for me to remember). I don't find the
comparison with an explicit zero any more readable than the logical-
negation one, or see any other reason why it might discourage error.

But, of course, YMMV.
I particularly dislike

if (!strcmp(passwo rd, "drowssap") )

... which tests for equality but appears on a hasty
reading to do the opposite.

With this, on the other hand, I agree. Inverted strcmp sense errors
are one of the more common sort I find in others' code. I don't
remember making one myself since I adopted Peter van der Linden's
macro, which I think does improve readability significantly. For
example:

#define MySTRCMP(s1, op, s2) (strcmp(s1, s2) op 0)
if (MySTRCMP(passw ord, ==, "drowssap") )

Inlining the operator brings string comparisons syntactically closer
to their numeric cousins.

--
Michael Wojcik mi************@ microfocus.com

An intense imaginative activity accompanied by a psychological and moral
passivity is bound eventually to result in a curbing of the growth to
maturity and in consequent artistic repetitiveness and stultification.
-- D. S. Savage