[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFA]: BOOL coding standards (Was: Problem with +numberWithBool: ?)
From: |
Sheldon Gill |
Subject: |
Re: [RFA]: BOOL coding standards (Was: Problem with +numberWithBool: ?) |
Date: |
Sat, 31 Jan 2004 12:44:50 +0800 |
User-agent: |
KMail/1.6.50 |
> if ([myObject doSomething]) ...
>
> which is immediately understandable at a glance. But when I have a
> misbehaving program, I often wonder, is it really doing what I think
> it's doing?
All of this touches things which I feel are quite important. Let me start out
by clearly declaring
1) I'm a big fan of the principles behind "Design By Contract".
2) I firmly believe in being assert()ive
In regards to this discussion, it raises the point
* Say what you mean and mean what you say.
ObjC, following from Smalltalk, sends messages to objects. Those messages here
are written in English giving rise to some quite literate programming. Caveat
is that the writer needs to be literate.
So considering your example, Adam, I know it's supposed to be generic but...
doSomething for a test condition is an invitation to problems, because 'do' is
a verb clause. Let's make the discussion a little more concrete by
considering this example:
if ([aValve checkStatus])
it matches the 'doSomething' approach but is actually confusing because it
fails to be explicit. You're obviously checking the status of the valve but
what does the return mean? Does 'No' mean something about the current status
of the valve or does it mean something about the check process itself?
I feel a much better form is a possessive clause like
if([aValve isOperable])
if([aValve hasPower])
if([aValue isCommunicating])
because these have fairly explicit meanings for which the truth value is
reasonably apparent. This becomes very important when the required logic
becomes necessarily complex and involved. (think Karnaugh maps...)
So when developers write:
if( [] == YES)
what they generally mean is
if( isTrue() )
in the Boolean algebra sense. Confusion arises because of the
_implementation_ of testing in 'C'. Thanks to it's origins (and I'll happily
rant on that for many pages) the way if() works comes from testing the
processor 'Z' flag and branching. So those writing code discovered that
anything non-zero wouldn't set the Z flag... Not a real language design
decision. It arises from practice, not theory {cue Dijkstra}
but what we have here is just one of many such cases in testing. Let me
highlight another one dear to my heart in C:
linked_list *ptr;
if ( ptr != NULL )
when what you really mean is
if ( isValidPtr(ptr) )
or better yet
if ( isValidPtrToLinkedList(ptr) )
because it's not just that you don't want the pointer value to be non-zero.
Although it's more verbose, what you really mean is much more apparent. This
is invaluable when it comes to debugging because it allows you to create
functions for the above to check that what you expect really is the case and
it allows other developers to know what you were trying to achieve.
Contrast this to the other situation in C where
if ( ptr != NULL )
is supposed to mean
if ( haveArgument(ptr ) )
All too often I see code where it checks for a non-zero pointer, simply
returning otherwise, without considering whether a zero input is meaningful.
In many cases, it's not and only serves to obscure/propagate an error which
could have been easily and quickly picked up by throwing an exception.
So, within GNUstep code I think the _right_ thing to do is precisely Richard's
recommendation of
isYES(x): IF x NOT IN {YES|NO} THEN raise
which we can use to test all "+/- (BOOL)aMethod". Any method which returns
something other than {YES|NO} is bad practice we should catch and eliminate.
Any method which is defined to return one of these two values but actually can
return additional values is out of specification and should be corrected.
I also think that this approach should be adopted more generally.
From my own experience
- it highlights mistakes in specifications
- it catches mismatch between specification and implementation
- it catches bad assumptions
- it catches errors I might otherwise have missed
Finally, to Helge's example from OGo:
Well, that's right or wrong depending on the methods' definition. Consider:
- (BOOL)isEqual: anObject
the right thing here is "return [self compare:obj] != 0 ? NO : YES;" because
you've stated that the return is conceptually BOOLEAN (one of two values).
The method
- (int)compare: anObject
by definition returns one of _three_ possibilities and so has a return type to
suit. Indeed, that's also why it is called something like "compare" rather
than "isGreaterThan:".
If the calling code used the proposed "isYes()" then you would have easily
identified the issue during development. Nice and early when correction is
cheap :)
Conversely, if the definition was like
- (int)compareTo: anObject
then the calling code is at fault if it's explicitly testing against either NO
or YES because that's not what the method is about!
In light of the previous paragraphs, "compare" is another verb clause so
writing
if ([anObject compareTo: anotherObject])
i
s poor form. One better way would be
if ([anObject isEqualTo: anotherObject])
if that is what you mean. You could also use a functional approach:
if ( isEqual(anObject, anotherObject) )
Comparison also applies to two different concepts: equality and equivalence.
Which is meant is not apparent from "compare". Isn't English wonderful?
BTW, #define is the old C way to do enum {} because they didn't exist...
Coding Standards should explain good practice and approach. I'd really like to
see the standard enhanced more completely along this line to explain what is
good and what is bad and why this is thought so.
My apologies if this seems a little long winded. I wanted to be fairly clear
and reasonable in my case first up, rather than extending the thread with
many questions and clarifications.
Regards,
Sheldon
- Re[3]: [RFA]: BOOL coding standards (Was: Problem with +numberWithBool: ?), (continued)
- Re[3]: [RFA]: BOOL coding standards (Was: Problem with +numberWithBool: ?), Manuel Guesdon, 2004/01/30
- Re: [RFA]: BOOL coding standards (Was: Problem with +numberWithBool: ?), David Ayers, 2004/01/31
- Re: [RFA]: BOOL coding standards (Was: Problem with +numberWithBool: ?), Richard Frith-Macdonald, 2004/01/31
- Re: [RFA]: BOOL coding standards (Was: Problem with +numberWithBool: ?), David Ayers, 2004/01/31
- Re: [RFA]: BOOL coding standards (Was: Problem with +numberWithBool: ?), David Ayers, 2004/01/31
- Re: [RFA]: BOOL coding standards (Was: Problem with+numberWithBool: ?), Alexander Malmberg, 2004/01/31
- Re: [RFA]: BOOL coding standards (Was: Problem with +numberWithBool: ?), Kazunobu Kuriyama, 2004/01/30
- Re: [RFA]: BOOL coding standards (Was: Problem with +numberWithBool: ?), Richard Frith-Macdonald, 2004/01/30
- Re: [RFA]: BOOL coding standards (Was: Problem with +numberWithBool: ?), Richard Frith-Macdonald, 2004/01/30
- Re: [RFA]: BOOL coding standards (Was: Problem with +numberWithBool: ?), Adam Fedor, 2004/01/30
- Re: [RFA]: BOOL coding standards (Was: Problem with +numberWithBool: ?),
Sheldon Gill <=
- Re[2]: [RFA]: BOOL coding standards (Was: Problem with +numberWithBool: ?), Manuel Guesdon, 2004/01/31
- Re: [RFA]: BOOL coding standards (Was: Problem with+numberWithBool: ?), Alexander Malmberg, 2004/01/31
- Re: Problem with +numberWithBool: ?, Alexander Malmberg, 2004/01/30
- Re: Problem with +numberWithBool: ?, Nicola Pero, 2004/01/30
- Re: Problem with +numberWithBool: ?, Alexander Malmberg, 2004/01/30
- Re: Problem with +numberWithBool: ?, Philippe C.D. Robert, 2004/01/30
- Re: Problem with +numberWithBool: ?, Pascal J . Bourguignon, 2004/01/30
- Re: Problem with +numberWithBool: ?, Richard Frith-Macdonald, 2004/01/30
- Re: Problem with +numberWithBool: ?, Pascal J . Bourguignon, 2004/01/30
- Re[2]: Problem with +numberWithBool: ?, Manuel Guesdon, 2004/01/30