[BUG]Collection>>removeAll:

goran.hultgren at bluefish.se goran.hultgren at bluefish.se
Tue Aug 20 07:32:10 UTC 2002


I agree with everything said. :-)

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:?

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])

Also note that I could add a "return" here, anyway, my new
implementation was for most cases (unscientific tests) clearly slower
due to the cost of select: (using add: etc). My original idea was that
it seemed wasteful to copy the whole collection since mostly (I would
guess) much fewer objects are removed than "all". But #copy is fast. Ok,
so forget it - it was just a thought. :-)

And then I also noted that only Path and OrderedCollection overrides
this implementation... Ok, but what about say other subclasses? There
are perhaps others but one albeit silly example is LinkedList. Hmmm, I
hacked up a new version in LinkedList (which I know isn't used much at
all) like this:

removeAllSuchThat2: aBlock

	| aLink linkBefore |
	aLink _ firstLink.
	[aLink == nil] whileFalse:
		[(aBlock value: aLink)
			ifTrue:[aLink == firstLink
						ifTrue:[firstLink _ aLink nextLink.
								aLink == lastLink
									ifTrue:[lastLink _ nil]]
						ifFalse:[linkBefore nextLink: aLink nextLink.
								aLink == lastLink
									ifTrue:[lastLink _ linkBefore]].
					linkBefore _ aLink nextLink.
					aLink nextLink: nil.
					aLink _ linkBefore]
			ifFalse:[linkBefore _ aLink.
					aLink _ linkBefore nextLink]]

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. Garf. LinkedList was obviously a bad
choice. Anyway, I think the above implementation works.

Ah, well. Back to SqueakMap.

regards, Göran

PS. Wrote this fast, perhaps I missed a few things... DS



More information about the Squeak-dev mailing list