Date: Fri, 30 Nov 2007 12:46:46 +0100 From: =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= <des@des.no> To: Bruce Evans <brde@optusnet.com.au> Cc: arch@freebsd.org Subject: Re: Code review request: small optimization to localtime.c Message-ID: <86ve7kgdll.fsf@ds4.des.no> In-Reply-To: <20071130094705.E6718@besplex.bde.org> (Bruce Evans's message of "Fri\, 30 Nov 2007 10\:04\:54 %2B1100 \(EST\)") References: <20071128.151021.709401576.imp@bsdimp.com> <86lk8hhzs0.fsf@ds4.des.no> <20071129.084108.-713549098.imp@bsdimp.com> <20071130094705.E6718@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
Bruce Evans <brde@optusnet.com.au> writes: > KNF rules are sort of the opposite in some respects -- > -- "if ((flags & MASK) !=3D 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 =3D=3D 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 --=20 Dag-Erling Sm=C3=B8rgrav - des@des.no
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86ve7kgdll.fsf>