[BUG]Collection>>removeAll:

Richard A. O'Keefe ok at cs.otago.ac.nz
Fri Aug 23 03:59:17 UTC 2002


"Norton, Chris" <chrisn at Kronos.com> wrote:
	For those reasons, I stand by my earlier statement that we
	should not change this method to what was proposed by Richard O.
	
This is a slander.  I did *NOT* propose copying aCollection
*except* in the case where the method previously did not work at all.
The overhead of what I *did* proposed is too small to measure.

Suppose that we *didn't* have a 'generic' #removeAll: method in Collection.
Since the majority of collections in Squeak _can't_ do this, the most
generally applicable definition to put in Collection is

    Collection>>
    removeAll: aCollection
	self shouldNotImplement

(By the way, the argument that the calls to #remove: might do important
things doesn't hold water.  The ANSI specification also specifies the
behaviour of #remove:, and it isn't allowed to do anything except modify	
the receiver so the element isn't there any more or report an error.)

    CharacterSet>>
    removeAll: aCollection
	aCollection == self	
	    ifTrue:
	        [map atAllPut: 0]
	    ifFalse:
	        [aCollection do: [:each | self remove: each]].

    Set>>
    removeAll: aCollection
	aCollection == self
	    ifTrue:
		[tally := 0. array atAllPut: nil]
	    ifFalse:
	    	[aCollection do: [:each | self remove: each]].

    Bag>>
    removeAll: aCollection
	aCollection == self
	    ifTrue:
	        [contents removeAll: contents]
	    ifFalse:
		[aCollection do: [:each | self remove: each]].

    Dictionary "inherit from Set, like other Set descendants"

    Heap>>
    removeAll: aCollection
        aCollection == self
            ifTrue:
                [self setCollection: (Array new: array size)]
	    ifFalse:
	        [aCollection do: [:each | self remove: each]].

    OrderedCollection>>
    removeAll: aCollection
        aCollection == self
            ifTrue:
		[array atAllPut: nil. firstIndex := 1. lastIndex := 0]
            ifFalse:
                [aCollection do: [:each | self remove: each]]

    SortedCollection "inherits from OrderedCollection"

    SharedQueue
	"Is not a descendant of Collection, so doesn't actually
	 support #removeAll: or indeed #remove: anyway."

    LinkedList "give me time, give me time"
	"Oddly enough, aLinkedList removeAll: aLinkedList is almost
	 the only call to LinkedList>>removeAll: that would ever make sense."

So we see that it is entirely practical to implement #removeAll:
 - in full compliance with ANSI Smalltalk
 - without any extra copying
 - in such a way that the overhead is too small to measure
 - for all cases, even the special case.

You can return aCollection afterwards if you want, so add
 - without changing the documentation about what is returned
 - except maybe to add that the result isn't useful if it == the receiver.

Note:  this code is not tested.  But it was not difficult to write.

Note:  suppose some subclass wants #remove: to do something special,
contrary to the ANSI standard.  Then looking at code like _this_ will
warn the implementor that there is a special case to think about, and

    SomeSequenceableCollection>>
    removeAll: aCollection
        aCollection == self
            ifTrue:
		[[self isEmpty] whileFalse: [self remove: self last]]
	    ifFalse:                        
		[aCollection do: [:each | self remove: each]]

will probably do.  The one case I haven't considered is MappedCollection,
and it's a really nasty one.  There you are in trouble if the collection
argument == the receiver, its domain, or its map, and calling self copy
won't work, because it doesn't copy the domain or map.  It is very far
from clear to me that #copy is the right way to copy a MappedCollection.

Oddly enough, trying to explore a MappedCollection using
	(MappedCollection newFrom: {1. 2. 3}) cmd-shift-I
didn't work in Squeak 3.0.

However, there isn't actually a problem because
aMappedCollection removeAll: anotherCollection
called SequenceableCollection>>remove:ifAbsent which isn't allowed.
So the #shouldNotImplement inherited from Collection would be just right.



More information about the Squeak-dev mailing list