C Question

Electronics Computer Programming Q&A
psycho
Posts: 388
Joined: Thu Jan 10, 2008 8:13 pm
Location: Northwest Indiana
Contact:

C Question

Post by psycho »

I ran into something in C that I am not sure what is. I wouldn't even know how to google it...

Take the line :

if ( A != B || (C = D, E = F, G > H) ) {

How do the comma parts work?

Kevin
User avatar
philba
Posts: 2050
Joined: Tue Nov 30, 2004 1:01 am
Location: Seattle
Contact:

Post by philba »

(a,b,c) is a list. It is evaluated left to right and the entire statement takes the value of the rightmost value. (iirc) a=b is an expression that have the value of b.

In general, I think that kind of coding has no place in programming. It's obscure and can have odd side effects since some compilers can short circuit an if statement. for example

Code: Select all

if( a || (b=c))...
may never execute b=c if a is true.

phil
[/code]
psycho
Posts: 388
Joined: Thu Jan 10, 2008 8:13 pm
Location: Northwest Indiana
Contact:

Post by psycho »

Hmmm. Would you happen to know a site that explains the whole process of this?

Kevin
User avatar
haklesup
Posts: 3136
Joined: Thu Aug 01, 2002 1:01 am
Location: San Jose CA
Contact:

Post by haklesup »

Thats not a proper boolean expression. It is a syntax that is probably specific to the compiler it was written for. If you knew what kind of C and what version you could look up the syntax for the If statement and logic symbol conventions.

What you want to learn is a cross between logic and program coding. Logic design for hardware and that used in software is generally similar but the way it is entered into a program may be language specific. This statement is complex and defies translation into english. For example if A=B then the list of arguments but the ! and || have me confused too. But that's because I am a hardware guy.
User avatar
MrAl
Posts: 3862
Joined: Fri Jan 11, 2002 1:01 am
Location: NewJersey
Contact:

Post by MrAl »

Hi there,



Here's what it looks like to me, breaking it down one
part at a time in the order of execution...

A!=B
This executes first. There are two possible outcomes:

[1]
If the result is true then (C = D, E = F, G > H) does not
get executed due to 'or' short circuiting, and the statement
body enclosed in {} executes, and that's all that happens,
so C does not get set equal to D and E does not get set equal
to F and the test G>H never occurs. The entire statement is
complete, and C and E retain their previous values.


[2]
If the result of A!=B is false then next to execute is the
statement: (C = D, E = F, G > H).
This means C gets set equal to D, E gets set equal
to F, and then the test G>H occurs. If G>H is true then the
statement body enclosed in {} gets executed, but if G>H is false
then the statement body never gets executed.
The entire statement is then complete, and C and E has had
their values changed.


Overall...
Note sometimes C and E get their values changed and sometimes
not. Also sometimes the statement body gets executed and
sometimes not.

ADDED LATER:
I didnt carefully read all the replies before i started writing.
The test A!=B means "A not equal to B".
In other words,
if (A!=B)
means
"if A is not equal to B then"
This is similar to the test
if (A==B)
which of course means
"if A is equal to B then"
so the exclamation point is interpreted as "not".
LEDs vs Bulbs, LEDs are winning.
User avatar
philba
Posts: 2050
Joined: Tue Nov 30, 2004 1:01 am
Location: Seattle
Contact:

Post by philba »

haklesup wrote:Thats not a proper boolean expression.
It is valid C. stupid, confusing, error prone but valid. The thing about C is that the expression in the if statement evaluates to a value. It can have any type. As long as it evaluates to non-zero the "then" clause of the if statement is taken. That's why you will see things like

Code: Select all

char *s;
if(*s){
for checking to see if you are at the end of a string.

C is what is called an expression language. Statements are expressions. This is why

Code: Select all

a=b=c=d;
is valid. c=d is an expression that evaluates to d.

Code: Select all

a=(c=d,y=k);
is also valid.

This is why one of the most rookie mistakes people make in C is

Code: Select all

if(a=b) { // instead of if(a==b){
The compiler silently lets it through as an assignment and tests the value of b rather than checking to see if a is equal to b.
Engineer1138
Posts: 458
Joined: Thu Feb 05, 2004 1:01 am
Location: Minneapolis, MN
Contact:

Post by Engineer1138 »

I'll second what philba said. Even if it is "legal" C (don't have my reference here) the fact that it is difficult to interpret means that it should be avoided in professional software engineering unless there is a VERY GOOD reason to do it.

It's easy enough to make mistakes or have your code misunderstood when using the standard constructs. Incorporating such rarely-used syntax makes it all the more likely you'll make a mistake or someone will later misunderstand the intent.

Sorry if this seems harsh, but I spend enough time in code reviews as it is and stuff like this really bugs me.
User avatar
philba
Posts: 2050
Joined: Tue Nov 30, 2004 1:01 am
Location: Seattle
Contact:

Post by philba »

Engineer1138 wrote: Sorry if this seems harsh, but I spend enough time in code reviews as it is and stuff like this really bugs me.
I don't think it's harsh - life is too short for bad code.
User avatar
MrAl
Posts: 3862
Joined: Fri Jan 11, 2002 1:01 am
Location: NewJersey
Contact:

Post by MrAl »

Hi again,

I have to disagree to some extent here. Here's why...

Not only is the code "valid" and "legal", it's also fairly clever.

Let's take a look at what it would take to do it in a more
fashionable way:

Code: Select all

if (A!=B)
  {
     BODY();
  }
else
  {
     C=D;
     E=F;
     if (G>H)
       {
          BODY();
       }
  }

That ends up being a bit of a mess too. Note in particular that
what i have labeled as a procedure call "BODY()" has to be
repeated here *twice*. If that was instead several lines of code
then all that code would have to be repeated or else you would
have to do what i did above, make the lines into a function or
procedure and enter the call into the code twice (as above).
I do have to agree however that this second form is quicker to
understand.

To add to this, in the previous code some compilers will flag a
warning anyway that there is an assignment in a conditional
statement.

Now compare the above to the original code:

Code: Select all

if (A!=B||(C=D,E=F,G>H)){BODY()}
Which do you prefer now?
LEDs vs Bulbs, LEDs are winning.
User avatar
philba
Posts: 2050
Joined: Tue Nov 30, 2004 1:01 am
Location: Seattle
Contact:

Post by philba »

No contest, the expanded form. But I would never write code that way.

You don't know how a given C compiler will treat the expression in the parens with out explicit testing. This is bad software design because a change to the compiler or a different compiler can generate code that behaves differently. That code will work the way you say if you use the current X86 GCC compiler but there is no guarantee that other compilers will do the same. I know that GCC over the years has changed subtle things like this and broken a lot of code.

I believe that if you want something executed in the else clause of an if statement, put it in the else clause. Explicitness in software design is goodness. If you are trying to obscure the behavior of the code then I guess that's a fine way to do it.

In general, I think it is bad form to count on undocumented side effects in a programming language. Especially when you have no control over the compiler.
User avatar
philba
Posts: 2050
Joined: Tue Nov 30, 2004 1:01 am
Location: Seattle
Contact:

Post by philba »

dang it, tried to edit a typo, wound up with a reply. my bad.
psycho
Posts: 388
Joined: Thu Jan 10, 2008 8:13 pm
Location: Northwest Indiana
Contact:

Post by psycho »

Well, I guess that would make sense.

I knew what the != (not equal) and || (or) meant. Just not the comma mess. I can say I won't use that style.

Thanks to all who replied :)

Kevin
User avatar
MrAl
Posts: 3862
Joined: Fri Jan 11, 2002 1:01 am
Location: NewJersey
Contact:

Post by MrAl »

Hi again,

philba wrote:No contest, the expanded form. But I would never write code that way.

You don't know how a given C compiler will treat the expression in the parens with out explicit testing. This is bad software design because a change to the compiler or a different compiler can generate code that behaves differently. That code will work the way you say if you use the current X86 GCC compiler but there is no guarantee that other compilers will do the same. I know that GCC over the years has changed subtle things like this and broken a lot of code.

I believe that if you want something executed in the else clause of an if statement, put it in the else clause. Explicitness in software design is goodness. If you are trying to obscure the behavior of the code then I guess that's a fine way to do it.

In general, I think it is bad form to count on undocumented side effects in a programming language. Especially when you have no control over the compiler.
philba:

I guess i didnt get my point across. I used a simple statement
for "BODY()" but replace that with multiple lines of code and
you either have to repeat that code twice or compute a test
and then do another test to see if the first test passed or failed
before you can allow the multiple lines to execute.
Also, the multiple lines may include variables that are passed to
the main procedure and so if one were to write the body as a
function call, that function call would have to contain every argument
passed.
For the test/test code, it might look something like this:

long pass;
if (x==1){pass=1;} else {pass=0;}
if (pass) {BODY();}

Of course you may have a better idea, which you might care to share
here now?

I have to admit though that in my older age and with code i dont
look at for years and then one day pick back up again, i like the
most obvious way of coding...that is, what will be quickly apparent
when i pick it back up several years later. I dont want to have to
study it for several minutes just to figure out one routine.
Coders that do this every day however will not have any problem
with it.

You also have to realize that we have not yet mentioned speed of
execution, which deserves consideration sometimes.

Also, the original code depended highly on short circuiting, and
I dont know of any compiler/language developers that would be
silly enough to remove short circuiting from their compiler/language
either, since that's a feature that improves speed of execution
when dealing with several tests in a row and they put it in there
in the first place for this reason.

But of course we find ourselves again partly talking about the
controversial subject of what is collectively usually referred to as
"Readability", and as i have once long ago stated and will repeat here:

"Readability is in the mind of the beholder".
LEDs vs Bulbs, LEDs are winning.
User avatar
philba
Posts: 2050
Joined: Tue Nov 30, 2004 1:01 am
Location: Seattle
Contact:

Post by philba »

Let's review the bidding so far:

Code: Select all

if ( A != B || (C = D, E = F, G > H) ) { BODY():} 
The first thing that struck me about the original code is that it is impossible to determine the original author's intent. Since shortcutting is compiler dependent the intent is ambiguous. So, let's assume that the author knew that the compiler did shortcutting and the individual wanted to execute these statements when A!=B was zero:

Code: Select all

C=D; E=F;
Then I would refactor the sequence to be

Code: Select all

if(A==B) {
    C=D;
    D=F;
    }

if((A!=B) || (G<H)) {
    // body code
    }
While this does test A==B twice, it is far more obvious what it does. The execution of all the statements is unambiguous. You could place the second if in an else clause (with out the A!=B test, of course) but I think the slight overhead of the additional test is worth the increased clarity. In addition, a good optimizing compiler will probably recombine these two ifs and generate the same code.

Like I said earlier, it is completely unclear to me what the original author intended. If I picked up source with the original construct in it, I would have to go and figure out what the intent was, even in the unlikely event it was actually well commented. This is a huge waste of my time.

One other thing, while shortcutting may be in all compilers (it's definitely not and you can turn off optimization), there is nothing that defines the order of evaluation of parts of an expression at the same level of precedence. Some compilers may well execute the second part of the if expression first. Especially, if there was a calculation that involved G<H immediately above.

By the way, I've written compilers so I do know a fair amount on this topic.
Bigglez
Posts: 1282
Joined: Mon Oct 15, 2007 7:39 pm
Contact:

Post by Bigglez »

Greetings ????,
MrAl wrote:But of course we find ourselves again partly talking about the controversial subject of what is collectively usually referred to as
"Readability", and as i have once long ago stated and will repeat here:

"Readability is in the mind of the beholder".
I wasn't here for your earlier missive, perhaps it's just
as well. I'm not following the thread closely, but it
would appear that one side of the issue is making the
end product more useful and easier to repair or
modify through clarity. The other side is more
interested in intellectual weight lifting...

Comments Welcome!
Post Reply

Who is online

Users browsing this forum: No registered users and 13 guests