"Lic. Edgar J. De Cleene" <edgardec2001 at yahoo.com.ar> wrote:
	The error what I talking about is to try to add objects to a nil
	object.  In image, in packages and of course my owns.
	
If you try to add objects to nil, that's an error that we WANT to be
signalled, so that we know about it and can fix it.  The very _last_
thing you want is for silly mistakes to be concealed by "helpful"
methods.

If you want to add something to a collection, then by golly you want
it ADDED, not just dropped on the floor.

	Sure what you could say what this never could reach exterior
	world and is a product of bad coding practices, but again, happens .
	
It obviously _could_ reach the exterior world.
And it just as obviously *is* a product of bad coding practice,
which should be discovered as soon as possible and FIXED, not hidden.

	And still don't understand way do: if for
	each element and ifNotNilDo: not.
	
Because #do: is a method for collections and #ifNotNilDo: is not.
Or rather,

    x ifNotNilDo: aBlock
	
views x as a collection, {} if x is nil and {x} if x is not nil.
We could clarify this very simply by having a single definition for
#ifNotNil: in Object:

    Object>>
    ifNotNilDo: aBlock
        ^(self ifNil: [{}] ifNotNil: [{self}]) do: aBlock

Myself, I'm not sure that #ifNotNilDo: is a good idea; I certainly never
use it.  Let's see a couple of examples of its use and what they would
look like without it.  I'm looking at Squeak 3.6-5429-full.image; I dare
say quite a lot of this has been cleaned up since.

    AssignmentTileMorph>>
    addArrowsIfAppropriate
        (Vocabulary vocabularyForType: dataType)
            ifNotNilDo:
                [:aVocab | aVocab wantsAssignmentTileVariants ifTrue:
                    [self addArrows]]
-->
    addArrowsIfAppropriate
        |aVocab|
        aVocab := Vocabulary vocabularyForType: dataType.
        (aVocab notNil and: [aVocab wantsAssignmentTileVariants])
            ifTrue: [self addArrows].

I'd be inclined to simplify this by having Vocabulary>>vocabularyForType
return a default object which says no rather than returning nil, so we'd
have the even simpler

    addArrowsIfAppropriate
        (Vocabulary vocabularyForType: dataType) wantsAssignmentTileVariants
            iTrue: [self addArrows].

WAAAAIIT a minute!  Vocabulary>>vocabularyTypeFor: has a comment saying
"... Answer the Unknown vocabulary as a fallback.", and it _does_ that.
And the Unknown vocabulary _does_ answer false to wantsAssignmentTileVariants.
So the simplified code is _already_ right because the #ifNotNilDo: is _never_
sent to nil!

    Celeste>>replyTextFor: msgId
        ...
        (msg fieldNamed: 'message-id' ifAbsent: [])
	    ifNotNilDo: [:messageIDField |
	        z nextPutAll: 'In-reply-to: ',
			      messageIDField asHeaderValue; cr.].
	...

Now if only there were a method

    MailMessage>>
    fieldNamed: aString ifAbsent: absentBlock ifPresent: presentBlock
      ^presentBlock value: 
        (fields at: aString asLowercase ifAbsent: [^absentBlock value]) first

then this could be written was

	msg fieldNamed: 'message-id' ifAbsent: [] ifPresent: [:id |
	    z nextPutAll: 'In-reply-to: ', id asHeaderValue; cr].
	           
    CodeHolder>>
    refreshAnnotation
      (self dependents detect: [:m (m inheritsFromAnyIn: #('PluggableTextView'
'PluggableTextMorph')) and: [m getTextSelector == #annotation]]
        ifNone: [nil]) ifNotNilDo:
        [:aPane | aPane hasUnacceptedEdits ifFalse:
            [aPane update: #annotation]]

This is a rather nasty piece of code.  For one thing, the #inheritsFromAnyIn:
seems to be a clumsy way of asking about respondsTo: #getTextSelector.
Suppose we had the following method in Collection:

    detect: aBlock ifNone: noneBlock ifSome: firstBlock
        "Pass each element of this collection in turn to aBlock.
         If aBlock answers true, pass that element to firstBlock and stop.
         If aBlock never answers true, answer the value of noneBlock."
	self do: [:each |
	    (aBlock value: each) ifTrue: [^firstBlock value: each]].
	^noneBlock value

Then we would have

    CodeHolder>>
    refreshAnnotation
      self dependents
	detect: [:m | m respondsTo: #getTextSelector and: [
		      m getTextSelector == #annotation]]
	ifNone: []
	ifSome: [:aPane | aPane hasUnacceptedEdits
	                    ifFalse: [aPane update: #annotation]].

which I think is clearer.

    MethodHolder>>
    doItReceiver
      (self dependents detect: [:m | m isKindOf: MethodMorph]) ifNotNilDo:
          [:mm | (mm owner isKindOf: ScriptEditorMoprh) ifTrue:
              [^ mm owner playerScripted]].
      ^self selectedClass ifNil: [FakeClassPool new]

This one is rather odd, because I'd always thought that the dependents of
a Morph had to be Morphs themselves, which nil isn't.  If #detect: doesn't
find anything, it doesn't return nil, it raises an exception.  So I don't
see how that call to #detect: can ever answer nil.  But the same
#detect:ifNone:ifSome: method would be applicable here:

    doItReceiver
      self dependents
        detectL [:m | m isKindOf: MethodMorph]
        ifNone: []
        ifSome: [:mm | (mm owner isKindOf: ScriptEditorMorph)
                         ifTrue: [^mm owner playerScripted]].
      ^self selectedClass ifNil: [FakeClassPool new]]

    Morph>>>
      allSubmorphNamesDo: nameBlock
        self isPartsBin ifTrue: [^self].
        self submorphsDo:
          [:m | m knownName ifNotNilDo: [:n | nameBlock value: n].
          m allSubmorphNamesDo: nameBlock].

Without #ifNotNil: this would be

      allSubmorphNamesDo: nameBlock
        self isPartsBin ifTrue: [^self].
        self submorphsDo:
          [:m | m knownName ifNotNil: [nameBlock value: m knownName].
                m allSubmorphNamesDo: nameBlock].

where m knownName may be called twice.

    Preference class>>
    loadPreferencesFrom: aFileName
      ...
      dict keysAndValuesDo:
        [:key :value | (self preferenceAt: key ifAbsent: [nil]) ifNotNilDo:
          [:pref | pref preferenceValue: value preferenceValue]].
      ...

This would be simpler without dragging nil into it at all:
            
      dict keysAndValuesDo: [:key :value |
        (self preferenceAt: key ifAbsent: [value])
          preferenceValue: value preferenceValue].

(If a key is absent, then value's preference value is set to value's
preference value, which has no net effect.)

    Preference class>>
    restorePersonalPreferences
      ..
      savedPrefs associationsDo:
        [:assoc (self preferenceAt: assoc key ifAbsent: [nil]) ifNotNilDo:
          [:pref | pref preferenceValue: assoc value preferenceValue]]

Again, this could be simpler:

      savedPrefs keysAndValuesDo: [:key :value |
        (self preferenceAt: key ifAbsent: [value])
          preferenceValue: value preferenceValue].

Finally,

    RemoteString>>
    last
        ^self string ifNotNilDo: [:s | s last]

There are two problems here.  First, if I ask for "rs last" where rs is a
RemoteString, I don't think I'm going to be expecting nil for an answer.
I'd rather this was just
	^self string last
But more interestingly, how _can_ (aRemoteString string) ever return nil?
If sources are not being maintained, the default value returned is '',
not nil.  If they are being maintained, the result is the result of a
call to #nextChunk.  And _that_ always answers the result of
(out contents) where out is a WriteStream on a string.  So it looks as
though (aRemoteString string) must always be a (possibly empty) string,
and the #ifNotNilDo: is pointless.

    RemoteString>>
    last
      ^self string last

would seem to be all that is called for.

The result of this little survey is rather surprising:  if you're looking
for something nasty, #ifNotNilDo: is often the rock it is hiding under.
It certainly isn't a tool you should habitually reach for or use as a model
in designing any other *do methods.