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.

 

[Alex]

how about

if((fp=fopen(filename,"mode"))==NULL)
{
//failed
}

 

[Artie Gold]

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.

I think in a case like this, either one would be just fine; it's just
the case that the former is more explicit, while the latter employs a
well-known C idiom.

You might want to keep it consistent in a project -- but either choice
is sufficiently transparent. (I prefer the second, but that's pure opinion.)

HTH,
--ag

 

[The Dragon]

how about

if((fp=fopen(filename,"mode"))==NULL)
{
//failed
}

or, conversely:

if ( NULL == ( fp = fopen( filename, mode ) ) ) {
/* Failed */
}

Constants going on the left makes the C compiler bitch about =
instead of == which is a common programmer error that generally
takes a long time to find during runtime debugging ;}

Same with below:

fp = fopen( filename, mode );
if ( NULL == fp ) {
/* failed */
}

if ( !fp ) doesn't require constants on the left, since ! is a
unary logic operator.

-- The Dragon

 

[Martin Ambuhl]

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) {
...
}

Anyone who tells you that either of these is "recommended style" except
with reference to your local coding standards is blowing smoke.

For what it's worth, I prefer
if (!(fp = fopen("xyz.txt","r"))) {
/* handle error */
}

 

[infobahn]

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) {
....
}

It depends who you ask.

If your employer/client/whatever doesn't impose a house style, decide
which you think is more logical, and use it.

Some people (including myself) argue that a pointer is not a boolean
variable and should not be treated like one. Other people argue
that !fp is just a shorthand for == 0, and so there's no harm in
using if(!fp), and if(fp == NULL) is only for people who love typing.

I hate typing, but I would rather type a few more characters if the
result is code which I find easier to maintain.

Personally, I use this:

fp = fopen(fn, fmode);
if(fp != NULL)
{
rc = foo(fp);
fclose(fp);
}
else
{
deal with the error
}

but there is no "right" answer to this question.

 

[Emmanuel Delahaye]

Kelvin Moss wrote on 30/12/04 :

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.

The first one. No option!

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
The C-library: http://www.dinkumware.com/refxc.html

"Clearly your code does not meet the original spec."
"You are sentenced to 30 lashes with a wet noodle."
-- Jerry Coffin in a.l.c.c++

 

[Emmanuel Delahaye]

Alex wrote on 30/12/04 :

how about

if((fp=fopen(filename,"mode"))==NULL)
{
//failed
}

Certainely not. Ugly, hard to read, hard to maintain.

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
The C-library: http://www.dinkumware.com/refxc.html

"C is a sharp tool"

 

[Emmanuel Delahaye]

infobahn wrote on 30/12/04 :

Personally, I use this:

fp = fopen(fn, fmode);
if(fp != NULL)
{
rc = foo(fp);
fclose(fp);
}
else
{
deal with the error
}

Agreed.

--
Emmanuel
The C-FAQ: http://www.eskimo.com/~scs/C-faq/faq.html
The C-library: http://www.dinkumware.com/refxc.html

"Clearly your code does not meet the original spec."
"You are sentenced to 30 lashes with a wet noodle."
-- Jerry Coffin in a.l.c.c++

 

[Kelvin Moss]

Kelvin Moss wrote:

Some people (including myself) argue that a pointer is not a boolean
variable and should not be treated like one. Other people argue
that !fp is just a shorthand for == 0, and so there's no harm in
using if(!fp), and if(fp == NULL) is only for people who love typing.

I too belong to the same school of thought. In fact in my whole code I
have used this syntax.

I hate typing, but I would rather type a few more characters if the
result is code which I find easier to maintain.

Personally, I use this:

fp = fopen(fn, fmode);
if(fp != NULL)
{
rc = foo(fp);
fclose(fp);
}
else
{
deal with the error
}

but there is no "right" answer to this question.

I too feel that doing this makes the code more readable.

Thanks.

 

[Keith Thompson]

or, conversely:

if ( NULL == ( fp = fopen( filename, mode ) ) ) {
/* Failed */
}

Constants going on the left makes the C compiler bitch about =
instead of == which is a common programmer error that generally
takes a long time to find during runtime debugging ;}

Same with below:

fp = fopen( filename, mode );
if ( NULL == fp ) {
/* failed */
}

We've discussed this at length here recently. Opinions vary
considerably on this. A lot of people (including myself) find things
like "if (4 == x)" just plain ugly for insufficient benefit.

 

[Lawrence Kirby]

That's a clear, concise C idiom, but please locate your space bar.

or, conversely:

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

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.

/* Failed */
}
}
Constants going on the left makes the C compiler bitch about = instead
of == which is a common programmer error that generally takes a long
time to find during runtime debugging ;}

Any C compiler must generate a diagnostic for

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

Same with below:

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

And many C compilers can be configured to issue a warning about

if (fp = NULL) {

Despite the theoretical benefits of this code that's readable is usually
the easiest code to debug.

Lawrence

 

[Joona I Palaste]

That's a clear, concise C idiom, but please locate your space bar.

I realise I'm treading on religious issues here, but I have a strong
rule about spaces before the first opening paranthesis. "Strong" as in
"written in stone".
The rule consists of two parts:
1) If the statement starts with a keyword (such as "if", "for", "while"
etc.) then include *ONE* space.
2) If the statement starts with a function or macro name then include
*NO* spaces whatsoever.
This rule applies to both C and Java.

I am amazed how many people either break one of the above parts, or even
worse, switch them around.

 

[Alan Balmer]

I realise I'm treading on religious issues here, but I have a strong
rule about spaces before the first opening paranthesis. "Strong" as in
"written in stone".
The rule consists of two parts:
1) If the statement starts with a keyword (such as "if", "for", "while"
etc.) then include *ONE* space.
2) If the statement starts with a function or macro name then include
*NO* spaces whatsoever.
This rule applies to both C and Java.

I am amazed how many people either break one of the above parts, or even
worse, switch them around.

Why would you be surprised that someone would violate *your* rule? I
didn't even know about it until now. (And, to tell the truth, still
don't feel any particular obligation to conform to it.)

 

[Joona I Palaste]

Why would you be surprised that someone would violate *your* rule? I
didn't even know about it until now. (And, to tell the truth, still
don't feel any particular obligation to conform to it.)

Bad choice of words. With "this rule" I really meant "a similar rule". I
don't expect people to know I have written this rule in stone. I am
reasonably sure most people have come up with a similar rule on their
own, or if not, work the same way anyway. After all, keywords are like
normal English words, and you add a space after English words. OTOH,
functions are not English words, they're more like mathematical
functions, and I don't think people leave spaces between mathematical
functions and an opening paranthesis.

 

[Andrey Tarasevich]

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.

For example

if (isalpha(c))
/* function 'isalpha' has boolean semantic even though it returns
an 'int' */
...

but

if (strstr(lpsz1, lpsz2) != NULL)
...

if (strcmp(lpsz1, lpsz2) == 0)
/* Never just '!strcmp(lpsz1, lpsz2)' */
...

 

[xarax]

Bad choice of words. With "this rule" I really meant "a similar rule". I
don't expect people to know I have written this rule in stone. I am
reasonably sure most people have come up with a similar rule on their
own, or if not, work the same way anyway. After all, keywords are like
normal English words, and you add a space after English words. OTOH,
functions are not English words, they're more like mathematical
functions, and I don't think people leave spaces between mathematical
functions and an opening paranthesis.

My rules is no space before '('. Always put block braces '{}'
each on their own line. Put "while" on its own line. Indent
4 spaces per nesting level, always converting tabs to spaces.
A bunch of other rules, that don't mean much to anyone else...

Different strokes for different folks. Just be consistent,
and hopefully use rules that can be taught to a beautifier
so you can easily convert someone elses format to your own.

 

[E. Robert Tisdale]

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

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

or

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

I might write:

FILE* fp = fopen("xyz.txt", "r");
if (NULL != fp) {
// read from xyz.txt
}
else { // (NULL == fp)
fprintf(stderr, "Couldn't open file xyz.txt!\n");
}

I prefer to deal with the normal, usual or expected condition first
and handle the error (exception) condition in the else part.

Others may prefer to handle error conditions first
and deal with the normal condition in the else part.

It is probably *not* a good idea to mix the two "styles"
in nested tests because it makes it very difficult
for other programmers to follow the logic.
Again, the rule is, "Be consistent."
Choose a style and stick with it.

 

[Bonj]

Neither:
fp = fopen("xyz.txt", "r");
if (fp == NULL) {
...
}
is nearly right, but that's java style. Since your question is titled
'Recommended style', the correct style is
fp = fopen("xyz.txt", "r");
if (fp == NULL)
{
...
}

is correct, but if ... is only one line, then I prefer just
fp = fopen("xyz.txt", "r")
if(fp == NULL)
...

or better still
if(fopen("xyz.txt", "r") == NULL) ...


They both will work, but if(!fp) is less explicit, as it's not actually a
boolean value you're testing, it's a returned handle, and you're testing
whether it's NULL or not.

 

[Bonj]

Nah, that's just naff. That just says "I'm someone who often forgets to put
== instead of = and therefore I always stick to putting the operand on the
left".