I wrote that
	> I really don't see anything evil about Object>>isNumber or
	> Object>>isString; numbers and strings are core parts of the
	> language and it seems fair enough for an object to know it
	> isn't one.
	
Andres Valloud <sqrmax at comcast.net> wrote:
	The bad part about these messages is that numbers and strings
	already "know" what they are by virtue of their behavior, and in
	the case of Smalltalk, by virtue of their class which specifies
	their behavior.

But this completely misses the point.  Of *course* numbers and strings
know what they are.  That's not the question.  The question is whether
it is legitimate for *other* objects to know that they *aren't* numbers
or strings.

	It would be even better to eliminate the isFoo
	+ ifTrue/ifFalse construct with polymorphism, i.e.:
	
		"in the context of doing foo"
		anObject isWhatever
			ifTrue: [this foo action]
			ifFalse: [that foo action]
	
	becomes
	
		"in the context of doing foo"
		anObject fooAction
	
	where
	
		Whatever>>fooAction
	
			"this foo action"
	
	
		Object>>fooAction
	
			"that foo action"
	
	
Urk.  First off, let's consider a simple Composite example,
*not* involving any additions to Object, because the whole point of
this thread is "what may legitimately go in Object".

So we have
    Thingy
    >>isComposite ^false
    >>childrenDo: aBlock ^self
    >>descendantsDo: aBlock
        aBlock value: self.
	self childrenDo: [:each | each descendantsDo: aBlock].
    >>allChildrenSatisfy: aBlock
        self childrenDo: [:each |
	    (aBlock value: each) ifFalse: [^false]].
	^true
    >>allDescendantsSatisfy: aBlock
	self descendantsDo: [:each |
	    (aBlock value: each) ifFalse: [^false]].
        ^true
    subclass RedThingy
    subclass BlueThingy
    subclass GreenThingy
    subclass CompositeThingy 'children'
       >>isComposite ^true
       >>addChild: aThingy ^children add: aThingy
       >>removeChild: aThingy ^children remove: aThing
       >>childrenDo: aBlock children do: aBlock
        
Now, the the question for Andres Valloud is whether #isComposite should
be eliminated.  To my mind, the answer is NO.

I take it as a basic guideline about abstract data type design
that if an operation may fail, there should be a way for the caller
to determine *in advance* whether the operation will fail, if that is
reasonably practical.  For example, since (aCollection last) will
fail if the collection is empty, we need (aCollection isEmpty).  But
since (aCollection last) will _also_ fail if aCollection is not a
sequenceable collection, we need aCollection isSequenceable.  (In fact,
for the sake of #removeLast, we should also have #isOrdered.)

How is a caller to know whether it is acceptable to call #addChild:
without some way of testing whether the Thingy it wants to add to is
composite or not?  Well, with the design I've presented (where operations
only _exist_ in classes where they can succeed at least some of the time)
the test aThingy respondsTo: #addChild: will do the job.  Slower and
less clearly than #isComposite, but it will do the job.  Or we could
whack in a rather unappetising
    addChild: aThingy self error: 'non-composites have no children'
and catch the exception, which is also slower and less clear.

Andres Valloud appears to be arguing that when you see a method doing
    aThingy isComposite
        ifTrue: [~~a~~]
        ifFalse: [~~b~~]
then that means that there should be some method in Thingy which does
this job.  Of course, we could always just add

    Thingy
    >>ifComposite: blockC ifSimple: blockS ^blockS value
    subclass CompositeThingy
        >>ifComposite: blockC ifSimple: blockS ^blockC value

but via
    aThingy ifComposite: [true] ifSimple: [false]
this is equivalent to #isComposite, and is not the _kind_ of method
that Andres Valloud has in mind.  He has in mind methods with
intention-revealing names that perform some sensible *specific*
combination of operations.

Sometimes you can create such methods, and clearly you should.

But sometimes you cannot create such methods without introducing coupling.

Frankly, the idea of filling Object with dozens of methods tightly
coupled to all sorts of things horrifies me.  Once again, the cure is
worse than the complaint.

	On top of cleaner and more intention revealing code,

I have nothing to say against clean or intention-revealing code.
However, I _am_ old enough and wary enough to deny that any
mechanically applied technique will ALWAYS yield cleaner code
or ALWAYS be more intention revealing.

	you get speedups because it's no longer necessary to determine
	the value of a boolean expression at runtime using ifTrue/ifFalse.

I am also old enough and wary enough to completely disbelieve all such
claims unless backed up with experimental results.  I well remember
writing a clean module (for a Prolog system) and then spending the best
part of a day hacking it to be efficient, only to discover that it was
markedly *slower* than the original clean version.

Checking the value of a boolean expression using ifTrue/ifFalse is FAST.
Much faster than calling a method where you have to pass a real block,
which you sometimes do if you are to avoid a nasty case of coupling.

Let's take an example from the 3.5 image and see what we can make of it.
It's #isSequenceable.

    Collection>>
    adaptToCollection: rcvr andSend: selector
        "somewhat condensed"

        rcvr isSequenceable & self isSequenceable
            ifFalse: [self error: '....'].
        ^rcvr with: self collect: [:re :se |
            re perform: selector with: se]

There are *two* sends of #isSequenceable here.
We can eliminate the test of self isSequenceable by putting

    Collection>>
    adaptToCollection: rcvr andSend: selector
	self error: '...'

    SequenceableCollection>>
    adaptToCollection: rcvr andSend: selector
        rcvr isSequenceable ifFalse: [self error: '...'].
	^rcvr with: self collect: [:re :se |
	    re perform: selector with: se]

and we can eliminate the second by noticing that
#with:collect: is only defined for SequenceableCollection and its
descendants, and the function of the test is to produce a better
error message.  So our final result is

    Collection>>
    adaptToCollection: rcvr andSend: selector
        self error: 'arithmetic on non-sequenceable collection'.

    with: anotherCollection collect: aBlock
        self error: 'only sequenceable collections can be combined this way'.

    SequenceableCollection>>
    adaptToCollection: rcvr andSend: selector
        ^rcvr with: self collect: [:re :se |
            re perform: selector with: se]

This version of #adaptToCollection:andSend: _is_ clearer and I expect
it would be faster.
    
Note that if the two methods in Collection were removed, the only
effect would be the nature of the error message: reasonably specific
messages would be replaced by "does not understand".

For this example, Andres Valloud's recommendation seems to be RIGHT.

Now let's look at #isColor.

    AlphaBlendingCanvas>>mapColor: aColor
        aColor ifFalse: [^aColor]. "Should not happen but who knows..."
        ...

Defensive programming, probably unnecessary.

    B3DSceneMorph areasRemainingToFill: aRectangle
        ^(color isColor and: [color isOpaque])
            ifFalse: [^Array with: aRectangle].
	^aRectangle areasOutside: self bounds

    BorderedMorph areasRemainingToFill: aRectangle
	"rewritten"
	|b t|

        (color isColor and: [color isTranslucent])
            ifTrue: [^Array with: aRectangle].
        t := borderWidth > 0 and:
             [borderColor isColor and: [borderColor isTranslucent]].
        b := self wantsRoundedCorners
             ifTrue: [
                t ifTrue:  [self innerBounds intersect:
			    self boundsWithinCorners]
		  ifFalse: [self boundsWithinCorners]]
             ifFalse: [
                t ifTrue:  [self innerBounds]
                  ifFalse: [self bounds]].
        ^aRectangle areasOutside: b

It's not clear to my why this method tests whether color is a colour
and whether borderColor is a colour.  BorderedMorphs are born with
color = Color blue and borderColor = Color black.  Instances of
Color and ImageMorph support #isOpaque; instances of BitmapFillStyle,
Color, FillStyle, GradientFillStyle, InfiniteForm, SolidFillStyle,
and TranslucentColor support #isTranslucent.  I can't help wondering
whether the classes that define #isTranslucent should also define
#isOpaque.  I have never understood Morphic, so I don't know what
other possibilities there are for the color and borderColor instance
variables; maybe color can be some kind of form other than an InfiniteForm?
Perhaps the Morphic Cleaning Project has documented this by now; this kind
of cleanup should be right up their alley.  I suspect that it should be
possible to eliminate the uses of #isColor, but NOT the uses of
#isOpaque and #isTranslucent.

    BorderedMorph>>borderColor: colorOrSymbolOrNil
        self doesBevels ifFalse: [
	    "Why no error message here?"
            colorOrSymbolOrNil isColor ifFalse: [^self]].
	borderColor = colorOrSymbolOrNil ifFalse: [
            "Why no error checking here?"
	    borderColor := colorOrSymbolOrNil.
	    self changed].

Ah.  Now we know that borderColor can be absolutely anything.
    (BorderedMorph new borderColor: 'whee!')
gets through with no complaints, leaving the 'borderColor' instance
variable set to a string.  Is this a good idea?  It appears that the
(borderColor isColor) test really is necessary; having
#isOpaque and #isTranslucent defined on Symbol and UndefinedObject
doesn't really appeal.  Actually, neither does letting borderColor
be absolutely anything.  Would a little bit of error checking _really_
be such a bad idea?  And how would that error checking work, if it did
not have something very like #isColor to check for?

    BorderedMorph>>borderStyle
        "Work around the borderWdith/borderColor pair"
        |style|
        
        borderColor ifNil: [^BorderStyle default].
	borderWidth isZero ifTrue: [^BorderStyle default].
	style := self valueofProperty: #borderStyle
	              ifAbsent: [BorderStyle default].
        (borderWidth = style width and: [
            "Hah! Try understanding this..."
            borderColor == style style "#raised/#inset etc" or: [
            #simple == style style and: [
            borderColor = style color]]]
	) ifFalse: [
            style := borderColor isColor
                ifTrue:  [BorderStyle width: borderWidth
                                      color: borderColor]
                ifFalse: [(BorderStyle perform: borderColor "argh.")
                           width: borderWidth].
	    self setProperty: #borderStyle toValue: style].
        ^style trackColorFrom: self

We learn that the three cases for borderColor are
    nil	    : use the default border style
    a Symbol: selector of a method in BorderStyle, a border style name
    a Color : a colour.
So in fact 'borderColor' is not always a color, and while setting it to
a string is allowed by #borderColor:, it will give rise to serious
problems later.  #borderColor: really SHOULD be stricter.

Note that "aBorderedMorph borderColor: nil" and
"aBorderedMorph borderColor: #default" have the same net effect on
this method.  It looks as though 'borderColor' should actually be
either a BorderStyle object or a Color object.

If we are to eliminate #isColor from the earlier methods, we need
to implement #isOpaque and #isTranslucent for BorderStyles, which
should be easy, since "BorderStyle>>color ^Color transparent".
It seems perfectly reasonable to add
    BorderStyle
    >>isOpaque ^false
    >>isTranslucent ^true
    >>isTranslucentColor ^false
and I suspect that this would give _better_ results for
#areasRemainingToFill: than the present code.

    BorderedMorph hasTranslucentColor
        "Answer true if any of this morph is translucent
         but not transparent."
        ^(color isColor and: [color isTranslucentColor]) or: [
         (borderColor isColor and: [borderColor isTranslucentColor])]

Note that #isTranslucentColor is not the same as #isTranslucent.
By making borderColor only a BorderStyle or a Color, and by implementing
the #is{Opaque,Translucent,TranslucentColor} methods on BorderStyle, we
could simplify the borderColor test here.  I don't yet know what 'color'
can be, but if it can be any kind of Form, the #isColor test is needed.
On the other hand, if 'color' is a Form, and the form is painted with
a translucent (but not transparent) colour, the present result is
arguably wrong.  Peep at Morph>>color:.  If Color is not a Color or an
InfiniteForm, #asColor is sent to it.  So 'color' is either a colour or
an infinite form.  The methods
    isOpaque "all pixels are opaque"
    isTranslucent "some pixels are translucent"
    isTranslucentColor "either some pixels are translucentColor
                        or there is a mix of opaque and transparent
                        pixels with some of each (guess)"
would seem to make sense for InfiniteForm; if they were present, then
the 'color' test could be simplified here.

    Color>> = aColor
        ^aColor isColor and: [
         aColor privateRGB = rgb and: [
         aColor privateAlpha = self privateAlpha]]

I've used this example before.  Since aColor can be *anything*,
any replacement method would have to be added to Object.  I suppose
you could have
    isColorWithRGB: anRGB andAlpha: anAlpha
        ^false
in Object, and
    isColorWithRGB: anRGB andAlpha: anAlpha
        ^anRGB = rgb and: [anAlpha = self privateAlpha]
    = aColor
        ^aColor isColorWithRGB: rgb andAlpha: self privateAlpha
in Color, but this would couple Object to Color rather more
obnoxiously than #isColor does and would be less efficient as well.

    Morph>>color: aColor
        (aColor isColor or: [aColor isKindOf: InfiniteForm])
            ifFalse: [^self fillStyle: aColor].
        color = aColor
            ifFalse: [
                self removeProperty: #fillStyle.
                color := aColor.
                self changed].

This really does seem to be a legitimate use of #isColor.

There are lots more uses of #isColor, and I have other things to do today.

I think we have three conclusions from this incomplete study.

(1) Morphic is a mess.  THREE CHEERS AND THREE CHEERS MORE
    for the Morphic Cleaning Project.

(2) Andres Valloud is absolutely right that uses of #isFoo *can*
    be a signal that some of the code is rotten and needs refactoring;
    I have to agree that it is always appropriate to at least
    *consider* whether the kind of alternative he suggests would be
    more appropriate, and it very often is.

(3) However, there are cases (like Color>>#= and Morph>>color:)
    where it is difficult to conceive of any replacement which would
    not be worse.