Why does it suggest parentheses for the code snippet?

L

lovecreatesbea...

Isn't code at line 2 correct?

a.c:2: warning: suggest parentheses around && within ||

int isleap(int year){
if (year % 4 == 0 && year % 100 != 0 || year % 400 == 0) /*line
2*/
return 1;
return 0;
}
 
B

Ben Pfaff

Isn't code at line 2 correct?

a.c:2: warning: suggest parentheses around && within ||

int isleap(int year){
if (year % 4 == 0 && year % 100 != 0 || year % 400 == 0) /*line
2*/
return 1;
return 0;
}

No. This is in the FAQ.

20.32: Will 2000 be a leap year? Is (year % 4 == 0) an accurate test
for leap years?

A: Yes and no, respectively. The full expression for the present
Gregorian calendar is

year % 4 == 0 && (year % 100 != 0 || year % 400 == 0)

See a good astronomical almanac or other reference for details.
(To forestall an eternal debate: references which claim the
existence of a 4000-year rule are wrong.) See also questions
13.14 and 13.14b.
 
M

mark_bluemel

Isn't code at line 2 correct?

Yeah. It's correct, but it's not ugly.
a.c:2: warning: suggest parentheses around && within ||

Your compiler has good taste :) Which one is it?
int isleap(int year){
if (year % 4 == 0 && year % 100 != 0 || year % 400 == 0) /*line
2*/
return 1;
return 0;

}

Bonus points for "return(!(year % 4) && (year % 100) || !(year %
400));" surely?
 
W

Walter Roberson

Isn't code at line 2 correct?
a.c:2: warning: suggest parentheses around && within ||
int isleap(int year){
if (year % 4 == 0 && year % 100 != 0 || year % 400 == 0) /*line
2*/
return 1;
return 0;
}

&& binds more tightly than ||, so the test is equivilent to

(((year % 4 == 0) && (year % 100 != 0)) || (year % 400 == 0))

However, it is relatively common for people to mistakenly think of
&& and || as having the same precedences, and so it is common
for people to read the operators from left to right. It doesn't
make a difference in this particular case, but if you had written,

if (year % 400 == 0 || year % 4 == 0 && year % 100 != 0)

then people would tend to (mistakenly) read this as

if ((year % 400 == 0 || year % 4 == 0) && year % 100 != 0)

when really it is

if (year % 400 == 0 || (year % 4 == 0 && year % 100 != 0))

The warning is altering you to the possibility of the common misreading.
 
C

christian.bau

No. This is in the FAQ.

20.32: Will 2000 be a leap year? Is (year % 4 == 0) an accurate test
for leap years?

A: Yes and no, respectively. The full expression for the present
Gregorian calendar is

year % 4 == 0 && (year % 100 != 0 || year % 400 == 0)

The expressions are different, but always produce the same result.
 
B

Ben Pfaff

christian.bau said:
The expressions are different, but always produce the same result.

For the record, that's why I canceled my article quoted above
soon after posting it.
 
O

Old Wolf

Bonus points for "return(!(year % 4) && (year % 100) || !(year %
400));" surely?

Not really: your code would still trigger the warning in the OP,
and you have superfluous brackets around the expression being
returned.
 
R

Richard Heathfield

Ben Pfaff said:
For the record, that's why I canceled my article quoted above
soon after posting it.

"Anyone who thinks programming dates is easy to get right the first time
probably hasn't done much of it." - Peter van der Linden
 
D

David Wade

Isn't code at line 2 correct?

Its not obvious from a first glance if it is or is not correct. Whilst the
precedance of arithmetic operators is easily remembered, when you mix
arithmetic and comparative operators the order of evaluation is not clear.
Inserting parenthesis makes this clear.
 
D

David Wade

Walter Roberson said:
&& binds more tightly than ||, so the test is equivilent to

(((year % 4 == 0) && (year % 100 != 0)) || (year % 400 == 0))

However, it is relatively common for people to mistakenly think of
&& and || as having the same precedences, and so it is common
for people to read the operators from left to right. It doesn't
make a difference in this particular case, but if you had written,

if (year % 400 == 0 || year % 4 == 0 && year % 100 != 0)

then people would tend to (mistakenly) read this as

if ((year % 400 == 0 || year % 4 == 0) && year % 100 != 0)

when really it is

if (year % 400 == 0 || (year % 4 == 0 && year % 100 != 0))

The warning is altering you to the possibility of the common misreading.

Along the same lines GCC suggests parenthesis around constructs of the form

if( x=mysub(a,b,c) ) {

Any thoughts on why? It seems obvious to me that this is assigning the
return from mysub to x and then testing it...
 
M

Mark L Pappin

David Wade said:
Along the same lines GCC suggests parenthesis around constructs of the form

if( x=mysub(a,b,c) ) {

Any thoughts on why? It seems obvious to me that this is assigning the
return from mysub to x and then testing it...

For the same reason: while it's valid code, it differs from
if( x==mysub(a,b,c) ) {
by just one character and the two have very different behaviour. The
presence of parens around the assignment is the heuristic GCC uses to
determine whether to warn you about the possibility that your code
contained a common typo.

mlp
 
C

CBFalconer

David said:
.... snip ...

Along the same lines GCC suggests parenthesis around constructs
of the form

if( x=mysub(a,b,c) ) {

Any thoughts on why? It seems obvious to me that this is assigning
the return from mysub to x and then testing it...

Because the original intent might have been:

if (x == mysub(a, b, c)) {

and a typo has deleted the second '='. This is a fairly common
error. By adding the extra set of parenthesis the test is on a
single value, i.e. the parenthized expression. This sort of typo
is also the reason for prefering:

if (CONST == var)
rather than:
if (var == CONST)

because dropping one of the '=' chars will trigger an error in the
first case, but not in the second. Unlike the extra parens case,
this will catch the error on any compiler.

--
<http://www.cs.auckland.ac.nz/~pgut001/pubs/vista_cost.txt>
<http://www.securityfocus.com/columnists/423>

"A man who is right every time is not likely to do very much."
-- Francis Crick, co-discover of DNA
"There is nothing more amazing than stupidity in action."
-- Thomas Matthews
 
K

Keith Thompson

CBFalconer said:
This sort of typo
is also the reason for prefering:

if (CONST == var)
rather than:
if (var == CONST)

because dropping one of the '=' chars will trigger an error in the
first case, but not in the second. Unlike the extra parens case,
this will catch the error on any compiler.

Yes. On the other hand, some of us find the "CONST == var" form so
ugly that it's not worth the increase in safety. De gustibus yadda
yadda.
 
D

David Wade

Keith Thompson said:
Yes. On the other hand, some of us find the "CONST == var" form so
ugly that it's not worth the increase in safety. De gustibus yadda
yadda.

I found the extra parenthisis round my expression ugly. Some one who was
reviewing the code suggested changing it to say

if (NULL != ( f=myfunc(a,b,c) ) )

which I found very ugly, and unintuitive. The suggestion was that by putting
a comparision in I was making it obviusly a conditional. My view was that I
was free to use conditionals as and when appropriate ....
 
R

Richard Bos

Besides, like setting free()d pointers to null, it's a trap. One day you
_will_ accidentally write if (var1 = var2) (sorry, presumably you'll
write if (var2 = var1)), and you'll have dishabited yourself from
spotting such errors. You will proceed to make yourself look silly in
front of your colleagues who haven't... if you're lucky.
I found the extra parenthisis round my expression ugly. Some one who was
reviewing the code suggested changing it to say

if (NULL != ( f=myfunc(a,b,c) ) )

which I found very ugly, and unintuitive.

Urg. Why not write

if ( ( NULL != ( f=myfunc(a,b,c) ) ) == TRUE )

Richard
 
Y

Yevgen Muntyan

Richard said:
Besides, like setting free()d pointers to null, it's a trap. One day you
_will_ accidentally write if (var1 = var2) (sorry, presumably you'll
write if (var2 = var1)), and you'll have dishabited yourself from
spotting such errors. You will proceed to make yourself look silly in
front of your colleagues who haven't... if you're lucky.


Urg. Why not write

if ( ( NULL != ( f=myfunc(a,b,c) ) ) == TRUE )

Some people don't like doing "if (ptr)", they insist that
if (ptr != NULL)
is the right way, and if you have this, you combine it with
if (0 == foo)
and there you go:
if (NULL != (f = myfunc(a,b,c)))
Crazy stuff.

Yevgen
 
G

Guest

Richard said:
Urg. Why not write

if ( ( NULL != ( f=myfunc(a,b,c) ) ) == TRUE )

I know you're not serious, but because (NULL != ( f=myfunc(a,b,c) ) )
is already easily readable as a boolean expression, arguably unlike
( f=myfunc(a,b,c) ).
 
R

Richard Bos

=?utf-8?B?SGFyYWxkIHZhbiBExLNr?= said:
I know you're not serious, but because (NULL != ( f=myfunc(a,b,c) ) )
is already easily readable as a boolean expression, arguably unlike
( f=myfunc(a,b,c) ).

No, I fear not. You are right that I'm not serious, but that's because I
find my proposed alternative no more unreadable than the one which was
suggested to David, and rather less readable than the simple, idiomatic
if (f=myfunc(a, b, c))

Richard
 

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

Forum statistics

Threads
473,764
Messages
2,569,566
Members
45,041
Latest member
RomeoFarnh

Latest Threads

Top