Richard A. O'Keefe wrote:
> Chris Norton <chrisn at Kronos.com> wrote:

<snipped>

> 	In an effort to remain on a constructive plain, I'd like to promote Stephan
> 	Rudlof's implementation suggestion; it looks very promising to me.
> 	
> 	Collection>>removeAll: aCollection
> 	  self isEmpty
> 	    ifFalse: [self assert: [self ~~ aCollection].
> 	             aCollection do: [:each | self remove: each]].
> 	  ^aCollection
> 	
> 	In his implementation, he retains the original functionality and he adds a
> 	warning.  Without actually trying it, the code looks good to me.
> 	
> Is this really a practical improvement on the code that I suggested?
> Remember, I have repeatedly suggested
> 
>     self == aCollection ifTrue: [self error: 'some error message'].
>     ^aCollection do: [:each | self remove: each]
> 
> as an acceptable alternative.


> All Rudlof added to that was an efficiency
> hack (isEmpty)

I don't view this as a 'hack', if I'm looking for cheap and easy to
understand efficiency improvements.

> that doesn't look as though it will pay off all that often,

Who knows...

> and *does* most undesirably block the error message when the receiver is
> empty.

OK, this is a point: my first suggestion treats an empty collection with
itself as argument different from a not empty one.
I have thought, the result of removing something empty from itself is
obvious. But I have to admit, that this could cover programming errors.

For beeing consistent in the semantics for as empty as not empty
Collections, I'd also be happy with

Collection>>removeAll: aCollection
  self assert: [self ~~ aCollection].
  self isEmpty
    ifFalse: [aCollection do: [:each | self remove: each]].
  ^ aCollection

; which should give the best from both worlds.


Greetings,

Stephan


<snipped>
-- 
Stephan Rudlof (sr at evolgo.de)
   "Genius doesn't work on an assembly line basis.
    You can't simply say, 'Today I will be brilliant.'"
    -- Kirk, "The Ultimate Computer", stardate 4731.3