On Sat, Jul 27, 2002 at 01:16:36PM +0200, Alessandro Rubini wrote:
>
> Hello.
>
> > -#define CHECKFAIL(count) ((count)==0 && noneofthem())
> > +#define CHECKFAIL(count) if (count==0) noneofthem();
>
> please don't. This turns an expression into a statement, with definitely
> no advantage. Besiders, the trailing semicolong is a blatant error.
I consider getting rid of compiler warnings due to strange C code
a damn good advantage:
You all know that
CHECKFAIL(typecount);
translates into
((count)==0 && noneofthem);
which just plain makes no sense as a line of C code (thus the compiler
warning).
I'd rather say the two different ways are *both* wrong.
(a define should normally be an expression, that's right).
I shouldn't have included the trailing semicolon, true.
I'm voting for replacing the CHECKFAIL define with simply calling
if (count==0) noneofthem();
in every case where it is used now.
Yes, it's sort of ugly, since it repeats the same line of code several times,
but IMHO it's still much cleaner than the other solutions.
> Similarly, the last hunk has no advantage. It's only more verbose
> by using "else if" where "&&" is perfectly fine and definitely more
> readable (since you don't need to track every else it its own it).
BUT it caused a compiler warning, since it abuses C syntax to its highest ;-)
(it's more readable for the experienced, though, that's true).
The comment in that part was there for a very good reason IMHO...
> I've no comments on other hunks. However, I'm definitely happy my
> surname gets fixed :)
Heh :)
--
The Declaration of Software Freedom:
http://freedevelopers.net/freedomdec/index.php