Code review request: small optimization to localtime.c

Dag-Erling Smørgrav des at des.no
Fri Nov 30 03:46:56 PST 2007


Bruce Evans <brde at optusnet.com.au> writes:
> KNF rules are sort of the opposite in some respects --
> -- "if ((flags & MASK) != 0)", which is like the above, is slightly
>    more normal than "if (flags & MASK)". -- The unary "!" operator is
> rarely used.  "if (!isfoo)" and
>    "if (!(flags & MASK))" are not normal.

This is not "opposite" - as I said, we have a rule about when we
*should* use an explicit comparison, but we lack a rule about when we
*should not*.  I think we *should not* use an explicit comparison when
the expression being tested is obviously a predicate, for instance when
it is a variable or a call to a function whose name begins with "is",
"can" or similar.  A corollary is that variables and functions *should
not* have names that begin with "is", "can" or similar unless they can
be used correctly without an explicit comparison.  You should never have
to write something like "if (__isthreaded == 5)".

BTW, (flags & MASK) is a poor example; depending on the value of MASK,
there may actually be several distinct non-zero values, so an explicit
comparison is justified.

> Anyway, there is too much existing code with bad style to change.  I
> draw the line (for non-booleans) between !error and !strcmp().

I loathe !strcmp(), but I also generally try to avoid !error.

DES
-- 
Dag-Erling Smørgrav - des at des.no


More information about the freebsd-arch mailing list