Re: [RFA]: BOOL coding standards (Was: Problemwith+numberWithBool:?)

[David Ayers]

Alexander Malmberg wrote:

> Methods that produce BOOL values must produce only the values 1 (YES) or
> 0 (NO).
>
> Methods that accept BOOL values must handle all non-zero values as true,
> and may use the isYES macro to do so. isYES must, preferably by default,
> but at least optionally, expand to nothing, ie.:
>
> #define isYES(x) (x)
>
> What it expands to in other cases I don't care about, as long as the
> default is reasonably sane.
For isYES() to be useful to insure a YES/NO result, it has to be defined 
as:
#ifndef isYES
#define isYES(_exp) ((_exp) != NO)
#endif
i.e.:
-setFlag: (BOOL)flag
{
 _flag = isYES(flag);
}

I'm preparing a patch proposal for -base that clears out all == YES 
comparisons (save Testib).  I've used the following conventions:

- methods that can be used in C tests (if/while/for) w/o an extra isYES():
is*, has*, contains*, *Exists*, ... and similar common methods.
- return values of -boolValue are tested with isYES() if assigned to a 
variable.
- C tests (if/while/for) on variables which used == YES are converted to 
isYES()
- Function returning BOOL which were tested against == YES are now 
nested in isYES() with the exception of GSObjCIsKindOf(), 
GSObjCIsInstance() & co. GS_IS_* and a few other common unambiguous names.

There are a few minor execeptions where I left an redundant isYES() for 
non standard methods like -isComplete in GSMime.  I don't mind if some 
of them get deleted later, but I'll try to be as conservative as 
possible for the initial cleanup.

Within GSLock we have an ivar declared int which used YES/NO and -1 to 
define locked/unlocked/unused state.  I've defined LOCKED, UNLOCKED, 
UNUSED locally and replaces the YES/NO/-1 to make greping for "== *YES" 
easier.

GSXML often uses constructs like
 int   newVal = (yesno == YES) ? 1 : 0;
which interpret all non-YES values as 0 instead of all non-NO values as 
1.  These semantics are changed with the patch.  I'd like some feedback 
whether this was intentional due to some libxml quirks, or whether 
libxml also uses C truth semantics.

I'm pretty much finished, but still want to look at some things a bit 
closer.  So bear with me until tomorrow.  I can post the patch to -bugs 
but I guess I should gzip it as the raw diff will be something about 
8500 lines.

One question Alex brought up on #GNUstep is the visibility of isYES().  
On the one hand I beleive it should be public and used through 
GNUstep+Libs+Apps.  OTOH it may interefer with third party code.  If we 
want to make it internal to each package, it would be OK with me.

Cheers,
David