Is this C program doing what it is supposed to do ?

[ImpalerCore]

 Not really. I just see a stated opinion, which is what I'm curious to
hear the reasoning behind.


 I've been writing C for a living since 1998, so I know about the
syntax. I just fail to see how it is in any way safer, clearer or in
any other better than this:

 if (ch == '\n') {
   at_beginning = 1;
 } else {
   at_beginning = 0;
 }


 Showing off doesn't score high with me. And that is the only reason
for coding in that cramped style.

 at_beginning = ch == '\n';
 at_beginning = ch = '\n';

The one comment I'll offer is that the advantage of using a boolean
variable explicitly is to assign semantic meaning to a (possibly
complicated) assembly of boolean logic. Stuffing it directly in an
'if' expression provides the maintenance programmer less context. If
the whole purpose of the 'if/else' expression is to assign a boolean
value to 1 or 0 anyways, in my opinion, it's much more verbose than it
needs to be.

Getting back to the whole 'a = b == c' style debate, I think that the
expression is fine, but I will typically go to the point of making
'==' expression in parentheses a la 'a = (b == c)'. I typically don't
add parentheses with other conditions like &&, ||, <, >, etc., unless
it's part of a larger boolean expression, i.e. I have no problems with
'a = b < c'. Out of curiosity, would you write an 'if' expression for
'a = b < c'?

On a different note, I range between dislike and abhor most 'if
( variable = function() )' even if some of them have idiomatic
histories.

 Both of the lines are equally plausible, when seen by themselves. So
to figure out the intent, you need to look elsewhere in the source. In
this case it isn't that hard, but when you are looking at 400k lines
of code written over a decade by a multitude of developers, each with
their own idea about how "clever" they are, constructs like this
become a major risk.

I don't think many will debate that having a company coding style
guidelines for projects of that size is in general a good idea. Of
course there's a big difference between saying it and actually doing
it.

Best regards,
John D.

 

[Keith Thompson]

No it started as

some-condtion= foo==bar;

That's how it started *in this thread*. I was discussing the
hypothetical situation that Anders Wegge Keller appeared to be
concerned about, that a perfectly clear and straightforward (IMHO)
line of code like

some_condition = (foo == bar);

would evolve into something big and ugly, and presumably that

if (foo == bar) {
some_condition = 1;
}
else {
some_condition = 0;
}

would be immune to such problems.

I think that everyone who's expressed an opinion agrees that the
parentheses are helpful.

 

[Keith Thompson]

If that holds true for you, then you are one lucky guy

With the risk of making this sound like a rant, My world consists of
an event driven control system, distributed over anything from 15 to
45 processes, 3 to 5 interface cards with their own CPU, talking to a
bunch of distributed nodes, with yet another level of code. That mess
controls various sorting machines that generate events by means of
upwards of 50 operators doing stuff in a random order.

I didn't say that *all* logic is simple. Surely the complexity you
describe doesn't show up in every single line of C code you write.

Some logic is complex. Some logic is simple. In the cases where it's
simple, we can write simple code that expresses it clearly.

And in my opinion, this:

some_condition = (foo == bar);

(assuming meaningful indetifiers) clearly and simply expresses the
intent.

[...]
I had to change the names to protect the guilty parties. I tried to
convey the general spirit of them, though.

Ok, here's how I might write the above (assuming "moon_phase =" for
"moon_phase ==" is the only typo):

some_condition = ( input == 0x42 ||
( moon_phase == M_WAXING && input == 042 ) ) &&
a_hideously_long_name_caused_by_a_fad_in_2003 != flabbergast;

Or I might consider putting the "&&" on a line of its own, for emphasis.
And I'd define some sort of symbolic names for the magic numbers 0x42
and 042 (did you really intend to use one hex constant and one octal
constant?).

Or I might declare separate objects for some of the subexpressions,
assuming they can be given meaningful names, but it's possible to go too
far with that kind of thing. It's hard to say without having any
understanding of the problem domain.

So how would you write it? Show us some actual code so we can compare
and decide which is actually more readable.

 

[Keith Thompson]

We are talking about two entirely different things here. I'm dead set
against the pattern of making conditional statements, be it
assignments or other statements, without a clearly visible if () { }
[else { }] construct. If I wanted to write perl, I'd do it in that
language.

Then you must have a great deal of difficulty reading C code written by
other people.

You object to using logically boolean expressions (expressions whose
results are best thought of as representing a true or false value)
in contexts that don't require such expressions (contexts other than
if(), while(), "&&", "||", etc.). "some_condition == (foo = bar);"
is one example of what you object to, but obviously not the only one.

Is that a good summary of your position?

I see no sensible basis for your objection, and I doubt that further
discussion will change either of our minds.

 

[Anders Wegge Keller]

We are talking about two entirely different things here. I'm dead set
against the pattern of making conditional statements, be it
assignments or other statements, without a clearly visible if () { }
[else { }] construct. If I wanted to write perl, I'd do it in that
language.

Then you must have a great deal of difficulty reading C code written
by other people.

Not really. I just don't like it.

You object to using logically boolean expressions (expressions whose
results are best thought of as representing a true or false value)
in contexts that don't require such expressions (contexts other than
if(), while(), "&&", "||", etc.). "some_condition == (foo = bar);"
is one example of what you object to, but obviously not the only
one.

Is that a good summary of your position?

Yes, with the addition of the ternary operator, which may or may not
make me a hypocrite in your eyes. In some cases I end up with stuff
like this, if the event space is small, and only used in one place.

logfunc ("Event %s(%d)",
(event == ENUM_this) ? "THIS" :
(event == ENUM_that) ? "THAT" : "UNKNOWN",
event);

For larger event collections or stuff that are logged in more than
one place, I make a function that return a const char pointer instead.

I see no sensible basis for your objection, and I doubt that further
discussion will change either of our minds.

Probably not.

 

[Anders Wegge Keller]

...

I didn't say that *all* logic is simple. Surely the complexity you
describe doesn't show up in every single line of C code you write.

You are right, to the extent that I will concede the existance of
logic that hasn't yet become complex.

Some logic is complex. Some logic is simple. In the cases where it's
simple, we can write simple code that expresses it clearly.

And in my opinion, this:

some_condition = (foo == bar);

(assuming meaningful indetifiers) clearly and simply expresses the
intent.

The problem is that it hardly ever stays that way. At least there
aren't that many places, where it's predictable to any reasonable
degree of certainity, which is why I avoid stuff like that on
principle.

Ok, here's how I might write the above (assuming "moon_phase =" for
"moon_phase ==" is the only typo):

some_condition = ( input == 0x42 ||
( moon_phase == M_WAXING && input == 042 ) ) &&

[Snipped to avoid breaking the line]

It still has the wrong flow. (action - condition), instead of the
other way around, which is more natural to me. Furthermore, this will
only work for assignments, meaning that when some day the action
taking place further down in the flow is refactored into a function of
its own, the conditionals have to be rewritten using if anyway. Either

if (conditions) {
function();
}

Or

condition_var = (conditions);
if (condition_var) {
function();
}

I prefer similar looking code in those two different situations, over
the chance to elide 6-8 characters and a linefeed or two.

Or I might consider putting the "&&" on a line of its own, for emphasis.
And I'd define some sort of symbolic names for the magic numbers 0x42
and 042 (did you really intend to use one hex constant and one octal
constant?).

We have some creep from the low-level code banging the actual
SCC's. For historical reasons[1], a lot of those constants have been
magic numbers, and that practice spread a bit into related functions.

Or I might declare separate objects for some of the subexpressions,
assuming they can be given meaningful names, but it's possible to go
too far with that kind of thing. It's hard to say without having
any understanding of the problem domain.

The domain is probably not that important. We have some events that
trigger some actions, if or if not some other condition is
fullfilled. Sometimes the events have to come in a particular order,
and we have multiple external conditions that have to be fulfilled, or
alters the meaning of some events.

Mostly it's like implementing the entire ftp protocol, without either
a state machine or goto.

So how would you write it? Show us some actual code so we can compare
and decide which is actually more readable.

if (side == UNITDESC_OUTER) {
if (shmptr->dd[dest].side != INNER) {
shmptr->dd[dest].side = OUTER;
} else {
shmptr->dd[dest].side = SIDE_BOTH;
}
}


if (DownTab->elm.tics_left) {
DownTab->elm.tics_left-- ;

if (DownTab->elm.tics_left == 0) {
DownTab->elm.state = DOWN_TIMEOUT ;
DownTab->elm.seen = FALSE ;

if (down_inform (i) == FALSE)
DownTab->elm.tics_left = 1 ;
}
}

This isn't overly complex, but I chose these two bits, because I can
post them without changes.


1. Really just an excuse for not fixing 20 year old code that still works.

 

[Keith Thompson]

Yes, with the addition of the ternary operator, which may or may not
make me a hypocrite in your eyes.

[...]

That was covered by the "etc.", along with "!", the second expression
in "for(;", and probably a few other things I haven't taken the
time to think of.

No, I don't think it makes you a hypocrite; your position seems to be
internally consistent. But it's good to know what your position is,
however strongly I disagree with it.

 

[Keith Thompson]

Ok, here's how I might write the above (assuming "moon_phase =" for
"moon_phase ==" is the only typo):

some_condition = ( input == 0x42 ||
( moon_phase == M_WAXING && input == 042 ) ) &&

[Snipped to avoid breaking the line]

It still has the wrong flow. (action - condition), instead of the
other way around, which is more natural to me. Furthermore, this will
only work for assignments, meaning that when some day the action
taking place further down in the flow is refactored into a function of
its own, the conditionals have to be rewritten using if anyway. Either

if (conditions) {
function();
}

Or

condition_var = (conditions);
if (condition_var) {
function();
}

I prefer similar looking code in those two different situations, over
the chance to elide 6-8 characters and a linefeed or two.

To be clear, my preference isn't about saving a few characters; I'm a
pretty fast typist, and I don't mind some extra typing for the sake of
clarity.

Or I might consider putting the "&&" on a line of its own, for emphasis.
And I'd define some sort of symbolic names for the magic numbers 0x42
and 042 (did you really intend to use one hex constant and one octal
constant?).

We have some creep from the low-level code banging the actual
SCC's. For historical reasons[1], a lot of those constants have been
magic numbers, and that practice spread a bit into related functions.

Or I might declare separate objects for some of the subexpressions,
assuming they can be given meaningful names, but it's possible to go
too far with that kind of thing. It's hard to say without having
any understanding of the problem domain.

The domain is probably not that important. We have some events that
trigger some actions, if or if not some other condition is
fullfilled. Sometimes the events have to come in a particular order,
and we have multiple external conditions that have to be fulfilled, or
alters the meaning of some events.

I mentioned the domain because it can provide some guidance for
assigning meaningful names to parts of the expression. Given:

if (foo == bar || something_else()) {
...
}

I can't tell how it might be broken into simpler part, but if I know
that "foo == bar" means that the boiler is at an acceptable pressure, the
clarity of the code might be improved by changing it to:

boiler_ok = (foo == bar);
if (boiler_ok || something_else()) {
...
}

or, if you insist:

if (foo == bar) {
boiler_ok = 1;
}
else {
boiler_ok = 0;
}
if (boiler_ok || something_else()) {
...
}

Mostly it's like implementing the entire ftp protocol, without either
a state machine or goto.

So how would you write it? Show us some actual code so we can compare
and decide which is actually more readable.

if (side == UNITDESC_OUTER) {
if (shmptr->dd[dest].side != INNER) {
shmptr->dd[dest].side = OUTER;
} else {
shmptr->dd[dest].side = SIDE_BOTH;
}
}


if (DownTab->elm.tics_left) {
DownTab->elm.tics_left-- ;

if (DownTab->elm.tics_left == 0) {
DownTab->elm.state = DOWN_TIMEOUT ;
DownTab->elm.seen = FALSE ;

if (down_inform (i) == FALSE)
DownTab->elm.tics_left = 1 ;
}
}

This isn't overly complex, but I chose these two bits, because I can
post them without changes.


1. Really just an excuse for not fixing 20 year old code that still
works.

Is that supposed to be based on the code we were discussing, the big
expression referring to moon_phase? I don't see the resemblance, nor do
I see much that's relevant to what we've been discussing.

My main quibble with this new code is that I'd write:
if (! down_inform (i))
rather than
if (down_inform (i) == FALSE)

(And I'd put braces around the controlled statement, and omit the white
space in front of the semicolons, but that's even less relevant.)

But what I was asking was how you'd re-write the big expression
referring to moon_phase, so we could compare your version and mine.
Of course you're under no obligation to do so, since we've pretty
much established our respective positions.

 

[Tim Rentsch]

This is CRAP the studies show otherwise.

Ah, so desu ka? Exactly which studies are those? I'm looking
forward to reading them.

 

[Tim Rentsch]

On 02/ 7/11 10:56 AM, Ben Bacarisse wrote:
<snip>
The line (justifiably) deemed appalling was:

at_beginning = ch == '\n';

[snip]

I tend to bracket any expression used as an rvalue, or where I'd have
to remember precedence rules!.

Here's a simple mnemonic to help you remember:

In C, assignment has lower precedence than _every other
operator_ except the comma operator.

Try taping that on your monitor or bathroom mirror for six months
and see if it helps you remember. If not you might consider
taking a memory class or consulting a neurologist.

The next lesson will feature a mnemonic to help remember that
assignment is right associative.

 

[Ben Bacarisse]

In this context I fail to see why this is different from you saing
it's safe and beautifull.

You snipped that part where I said it was safe. As for beautiful, I
require more of a bit code than this example has before I'd call it
beautiful. I don't mind if you do, but there's no point in arguing
about it.

Becuse I do not have the possibility of exhaustive testing. I don't
even have unit tests. What I have is a simulation of the machines our
software controls, and that's it. In this case, input hardly ever was
34, meaning that this lived for a long time.

Obviously that's a problem but writing it as an if does not remove the
typo. If you can't test you code you have real difficulties, but they
are not eliminated by your preferred form. In fact, because the if is
longer it may well present more typo opportunities.

Anyway, this is not the point! I don't mind if you insist on
writing a if to set a logical variable (though I see it as no safer
to do so -- it just introduces more opportunities for errors). The
original code set the variable in two places (conditionally) and it
is that trend that I was warning against. If modifications continue
to add further places where it is set, the logic will deteriorate
even further.

We are talking about two entirely different things here. I'm dead set
against the pattern of making conditional statements, be it
assignments or other statements, without a clearly visible if () { }
[else { }] construct. If I wanted to write perl, I'd do it in that
language.

Why do you associate this pattern with Perl? It's available in almost
every imperative language that I can think of (old FORTRAN is an
exception).

Anyway, I suspect we've reached then end of this exchange. I am sure we
understand each others position and I don't think either of us will come
up with some killer argument that has so far not been made.

 

[Ben Bacarisse]

if ( ( i = sscanf( &buf[0], Format,
&tmpstr[0][0], &tmpstr[1][0], &tmpstr[2][0]))
== NUM_Fields ) {

[snip]

That's *very* different from the original example.

The pattern is the same. Written by someone, who 17 years ago was
fresh out of school, knowing that a = (b == c) would remove the need
for if.

In what way is the pattern the same? As I said in a direct reply to
your post, your example has the form (i = f(...)) == CONST not
i = (f(...) == CONST). Is the pattern simply "expressions I don't like"?

If you object to the assignment in the conditional test, then I agree
that it seems to be unwarranted. I'd write the assignment first and then
the test:

i = sscanf(buf, Format, tmpstr[0], tmpstr[1], tmpstr[2]);
if (i == NUM_Fields) {

I will make an exception for a loop. In a loop, removing an assignment
from the condition often requires duplicating it and that presents
enough opportunities for future errors that I am happy to have a more
complex loop condition.

<snip>

 

[Hans Vlems]

[...]

On 6 feb, 03:04, Keith Thompson <[email protected]> wrote:

[[...] [snip]
An expression like:
   if (c==684)
       b=1;
   else
       b=0;
which came up in onother track of this post, obviously has its
supporters here.
In Algol or Pascal,
   IF (C=684) THEN B:=TRUE ELSE B:=FALSE;
is fairly ugly,
   B:=C=684;
is "better".

Yes, and I find the C equivalent better for exacly the same reasons.

I guessed as much and it is more a style issue than a performance
argument.
With Algol in mind you'd never use an IF statement to assign a value
to a boolean variable.

My point is that we needn't be entirely limited by the semantics of the
language we're using.  C90 doesn't have a boolean type.  C99 does, but
the equality and relational operators still yield results of type int,
and conditions are still defined in terms of comparison to zero.
Yes.

Nevertheless, we can write clear code that treats boolean expressions as
boolean expressions.  We don't have to get lost in an analysis of how
each expression is evaluated in whatever context it appears in.  With
carefully chosen idioms, we can write code that expresses its intent as
clearly as possible, without adding unnecessary verbosity just because
there's no explicit boolean type, or just because we don't happen to be
using it.

We can write:

    at_beginning = (ch == '\n');
    ...
    if (at_beginning) {
        ...
    }

and the code works *exactly* the same way as it would if "=="
yielded a bool result and if() took a bool condition.

The extra verbosity of

    if (ch == '\n') {
        at_beginning = 1;
    }
    else {
        at_beginning = 0;
    }

gains us exactly *nothing*.

Agreed.
Hans

 

[Keith Thompson]

Agreed.
Hans

Ok, now I'm confused. You've been arguing against the shorter form and
in favor of the longer form, but now you say you agree that the extra
verbosity gains us exactly nothing? Have you changed your mind?

 

[Mickey Mouse]

Hasn’t this urinating contest finished yet?

 

[Tim Rentsch]

It isn't but the original line was

flag = foo == bar;

Any C programmer who doesn't immediately understand the
effect of this statement (either with or without the
redundant parentheses) upon having read it needs to go
back and re-take Programming 101. The right way to
deal with "programmers" who did poorly in Programming 101
is not to cater to their lack of competence but to
encourage them to switch to another profession.

 

[Tim Rentsch]

Well it is compared to

at_beginning = (ch == '\n');

it takes the reader longer to parse the first, is it an assignment to
at_beginning or to at_beginning and ch?

at_beginning = ch = '\n';

That's a specious argument because the parentheses are
redundant in both cases.

 

[Keith Thompson]

That's a specious argument because the parentheses are
redundant in both cases.

IMHO it's not *entirely* specious.

Certainly the parentheses are redundant, but (for some reason I'm not
sure I can entirely explain) they seem more appropriate for the "=="
case than for the "=" case.

Chained assignments are usually written without parentheses:

var1 = var2 = some_value;

and if I see one *with* parentheses:

var1 = (var2 = some_value);

I know it means exactly the same thing, but I'm going to wonder why the
parentheses are there (hmm, maybe the second "=" was a typo for "=="?).

On the other hand, if I see:

at_beginning = ch == '\n';

it *might* occur to me that the "==" could be a typo for "=" -- but
in this case the meaningful names argue rather strongly against that.
I do find that superfluous parentheses:

at_beginning = (ch == '\n');

make it a bit easier to read (*for me*). I have a mild preference
for the version with the parentheses, and a strong preference for
either form over the verbose if/else version.

I suppose one could argue that adding the parentheses is catering
to ignorant readers just as much as using the verbose if/else form.
I would disagree with such an argument, but I'm not sure I can
articulate why.

More generally, C code has at least two audiences, the compiler and
human readers. I like to cater to the needs of the latter (for example,
by indenting my code, something the compiler couldn't care less about),
but I also like to assume that any human readers know the language
reasonably well. We're unlikely to reach universal agreement on where
to draw that line.

 

[Willem]

Tim Rentsch wrote:
)> Well it is compared to
)>
)> at_beginning = (ch == '\n');
)>
)> it takes the reader longer to parse the first, is it an assignment to
)> at_beginning or to at_beginning and ch?
)>
)> at_beginning = ch = '\n';
)
) That's a specious argument because the parentheses are
) redundant in both cases.

The standard idiom when assigning one value to multiple variables is:

a = b = c = 1;

So when a random programmer sees such a statement, the first thing he
will think is: 'that looks like a multiple-assignment statement'.
To differentiate the assignment of the boolean result of a comparison,
it would seem prudent to add the parentheses.

a = (b == c);


SaSW, Willem
--
Disclaimer: I am in no way responsible for any of the statements
made in the above text. For all I know I might be
drugged or something..
No I'm not paranoid. You all think I'm paranoid, don't you !
#EOT

 

[ImpalerCore]

That's a specious argument because the parentheses are
redundant in both cases.

Depends on your programming style.

I'm more likely to look at [at_beginning = ch = '\n';] as double
assignment, but [at_beginning = (ch = '\n');] as a syntax error since
using parentheses in that assignment doesn't mesh with my personal
style preferences. On that point, I avoid statements that include
multiple assignments altogether as a style issue simply to reduce the
opportunity for encountering statements with semantic ambiguities.
It's also the main reason I don't like '=' in 'if' expressions.

If this was someone else's code, I'd have no style frame of reference
to know what semantics the original author intended (not without
reading other code written by that author), and it's particularly
problematic in code that is maintained by several authors with
different styles. The problem is not with the legality of the
statement itself, the problem is the perception of the semantics of
that statement by different people, especially when you're trying to
track down a bug.

Best regards,
John D.