Re: [Qemu-devel] [7234] Use a more natural order

[lsorense@csclub.uwaterloo.ca (Lennart Sorensen)]

On Thu, Apr 23, 2009 at 11:46:25PM +0100, Jamie Lokier wrote:
> Lennart Sorensen wrote:
> > On Thu, Apr 23, 2009 at 10:31:05PM +0300, Blue Swirl wrote:
> > > I don't think any code style document can cover all possible cases.
> > > But another approach can be used: you could try to find a precedent
> > > case where this style has been used in QEMU.
> > 
> > # grep -r '([0-9] =' .
> > ./net.c:    if (0 == errno && '\0' == *last_char &&
> > ./hw/gus.c:    if (0 == ((mode >> 4) & 1)) {
> > ./hw/dma.c:            if ((0 == (d->mask & mask)) && (0 != (d->status & (mask << 4)))) {
> > ./hw/sb16.c:        if (0 == s->needed_bytes) {
> > ./hw/sb16.c:            if (0 == s->needed_bytes) {
> > ./hw/sb16.c:        if (0 == s->dma_auto) {
> > ./hw/sb16.c:        if (0 == s->dma_auto) {
> > 
> > That was just one quick search.  Looks like whoever wrote a bunch of
> > the audio hardware emulation liked less buggy code.
> 
> No, it means they believe that way around makes them less likely to
> write bugs.  They may be right for themselves.  It doesn't mean the
> code is less buggy, and it doesn't mean someone else following the
> same style would write less buggy code because of the style.
> 
> Same for the redundant parentheses around every comparison next to &&.
> Really, there is no need: if (condition && condition) is so common
> that there's no need to clarify the grouping.  Unlike say "x & 1 << 2"
> where it's a good idea to add parentheses.
> 
> > Well to me software development is a kind of engineering and hence using
> > anything but the safest practice that is at all practical makes no sense.
> 
> But what you call safest isn't safest for everyone.  A lot depends on
> what people are familiar with and trained to spot deviations from.
> 
> > That means:
> > Constants before variables in all comparisons.
> > Braces are never optional for blocks.
> 
> I disagree with both, but especially the first.  You call it safe
> practice.  I call it likely to cause more bugs because it's unfamiliar
> compared with common practice.  And because it's the opposite to the
> way we say it mathematically or in English.

Not everyone on the planet speaks english.  And common practice
(fortuantely) changes over time.

> It's opposite to the "Object Verb Subject" grammar that English
> speakers think in: asking "is zero equal to x?"  is really weird
> English, because that is asking a question _about zero_, which is not
> right: zero is not meant to be something about which there is any question.
> It's a constant!

Again, that may be true in english, but not many other languages.

> That cognitive dissonance adds a translation step between thinking (in
> English) and reading/writing, which means more chance of not seeing
> other bugs - limited brainpower and all.
> 
> "if (0 < x)" is even worse: it's begging to be accidentally written
> with ">".  More importantly, if it _is_ accidentally written with ">",
> the mistake is less likely to be noticed than if it were written the
> other way around.  But if you write "if (0 == x)" and "if (x > 0)"
> just because of "=" C syntax, that's an inconsistent mess.

Well preferably someone would invent a time machine, and go back and
smack whoever thought using = for assignment in C was a good idea.  :=
or something else that isn't just one character away from a completely
different operation would have been much better.

> Do you also write things like "for (x = 0; NUMBER > x; x++)"?  No, of
> course not.

Well to some extent, when doing < and >, you are not a single character
typo away from assigment.  It is == versus = that is the problem.

> Sure, it was ok practice back in the days when compilers would let you
> write "if (x = 0)", but those days are long gone.  Now all compilers
> warn, and you can ask them to make it an error.  A compiler check is
> much safer than depending on writing style, especially a weird one
> that's inconsistent with other writing in the same code, and
> inconsistent with English thinking.

I highly doubt all compilers warn.  Current gcc versions do, but not
everyone uses current gcc.

> Accidentally mistaking the scope of a block is unlikely using proper
> tools.  We're not using NOTEPAD.EXE any more.  Editors can indent code
> automatically these days (last 20 years or so).  Again, using tools is
> better than depending on writing style because it catches more mistakes.

The problem is when one wants to add a printf or something for debuging
in a condition and accidentally made the conditional code now happen
all the time.  It also means adding something to the condition now
requires changing 3 lines, to add one, while removing a line will
sometimes involve changing 3 lines when you drop down to a single
expression.

if (x > 0)
	printf("Found an x\n");
	y = x;

is a really annoying bug to find caused by trying to debug things when
braces are not used.  Now adding the debug line requires changing the
if line and adding another line afterwards.

> Actually I'm sympathetic to people who prefer braces around every
> block, and I happily follow such a style when requested.  It doesn't
> look too painful or weird, unlike the "unnatural" comparison order. :-)
> But I don't think it's necessary on projects where everyone uses other
> equally good practices instead to catch the same errors.

-- 
Len Sorensen