[BUG]Collection>>removeAll:

Richard A. O'Keefe ok at cs.otago.ac.nz
Tue Aug 20 23:40:36 UTC 2002


goran.hultgren at bluefish.se wrote:
	And when we are on the subject of removals in Collections. Here are a
	few discoveries/thoughts/musings this morning - they may not lead
	anywhere, but I thought I could share them anyhow:
	
	What about Collection>>removeAllSuchThat: ?
	
	I noticed the other day that this code:
	
	'abc' removeAllSuchThat: [:x | x = $d ]
	
	...runs just fine! But if you change "$d" to "$c" then, oops! ;-) This
	doesn't feel so good. Perhaps we should override this method in some
	places just like #remove:/#remove:ifAbsent:?
	
In Collection we find
    remove: oldObject
	"ArrayedCollections cannot respond to this message."
    remove: oldObject ifAbsent: anExceptionBlock
	"ArrayedCollections cannot respond to this message."
    removeAll: aCollection
	"ArrayedCollections cannot respond to this message."
    removeAllFoundIn: aCollection
	"ArrayedCollections cannot respond to this message."
    removeAllSuchThat: aBlock
	"No comment about ArrayedCollections but they DON'T work."

So I was expecting to find
    remove: oldObject
	self shouldNotImplement
	(Needed so that error message will refer to right method.)
    remove: oldObject ifAbsent: anExceptionBlock
	self shouldNotImplement
	(Needed so that error message will not be misleading.)
    removeAll: aCollection
	self shouldNotImplement
	(Needed so that 'x' removeAll: '' will be caught.)
    removeAllFoundIn: aCollection
	self shouldNotImplement
	(Needed so that 'x' removeAllFoundIn: #() will be caught.)
    removeAllSuchThat: aBlock
	self shouldNotImplement
	(Needed so that 'x' removeAllSuchThat: [:x | false] will be caught.)
in ArrayedCollection, but blow me down (as Captain Feathersword says),
there's nothing like that there at all.

Expect these methods in a [FIX] coming soon.

	And another thing, the implementation in Collection:
    
    removeAllSuchThat: aBlock 
        "Evaluate aBlock for each element and remove all that
        elements from the receiver for that aBlock evaluates to
        true.  Use a copy to enumerate collections whose order
        changes when an element is removed (i.e. Sets)."
    
        self copy do: [:each |
            (aBlock value: each) ifTrue: [self remove: each]]
    
    Should the base implementation really use a copy here? I tried:
    
        self removeAll: (self select: [:each | aBlock value: each])
    
Why not
	self removeAll: (self select: aBlock)

    Also note that I could add a "return" here,

This method doesn't have a defined value.  What would the return return,
if not self?

Given that #select: basically makes a copy, it isn't clear that there's
much scope for improvement here.

    [He considered overriding #removeAllSuchThat: for LinkedList,]
    But when I was going to compare it with the inherited version I suddenly
    discovered that the inherited version doesn't even work! ;-) #copy
    doesn't work for LinkedList.

The [FIX] mentioned above addresses this.  It's not clear that there IS a
satisfactory solution, because the LinkedList designers tried to make the
internal structure of the list (the Link objects) its elements.  This is in
marked contrast to most other collections, where it is perfectly possible
for the same object to be an element of two collections, and where
collections can, in general, hold any kind of object.  Basically, LinkedList
needs to be thrown away and redesigned from scratch.
    
While testing the new LinkedList>>shallowCopy, I tried
    ell includesAnyOf: ell copy
only to be told that LinkedLists are not indexable.

If they are not indexable, why do they inherit from SequenceableCollection?
It's easy enough to implement LinkedList>>at:, though not fast...

OK, I've just sent the [FIX].  It was developed in Squeak 2.8 and has been
checked out in Squeak 3.0 and Squeak 3.2.  removeAll-raok.2.cs supersedes
removeALl-raok.1.cs

There's another issue.

More-or-less parallel to the #remove* family there's a #copyWithout* family.
But they're not QUITE parallel...




More information about the Squeak-dev mailing list