[BUG]Collection>>removeAll:

Andrew C. Greenberg werdna at mucow.com
Wed Aug 28 18:24:49 UTC 2002


On Wednesday, August 28, 2002, at 06:12 AM, goran.hultgren at bluefish.se 
wrote:

>> - it is clear that people do try to write (x removeAll: x), and that 
>> it
>> doesn't do what they expect.  This seems to happen frequently enough
>> that it would be desirable to at least let people know when this
>> happens, so they can write their code a different way.  The currently
>> accepted valid ways to empty a collection are to either create a new
>> collection (usually the fastest way) or say (x removeAll: x copy).
>
> A few remarks:
> - The situation can occur without it being written "x removeAll: x"
> intentionally by the developer. Rather "x removeAll:
> (some-funky-expression-that-just-happens-to-evaluate-to-x)".

This is handled by the assertion proposal.

> - Creating a new collection does not retain identity.

Yes, this is the reason why it is valuable to have a method to remove 
all elements in some cases.  It does not suggest that an implicit 
iteration method should break Smalltalk's "don't change the iterating 
collection" semantics rule for just a few special cases.

> - "x removeAll: x copy" is really IMHO much more silly than "x
> removeAll". It demands that you are aware that "x removeAll: x" doesn't
> work and also that you realize that just sending in a copy would do the
> trick. Not at all obvious, but finding a method called "removeAll" is
> very obvious.

Hardly.  It is an idiom of the language, and those familiar with most 
collection iterations do this sort of thing routinely.

>> - If we come up with a way to detect this case and generate an error,
>> then the only remaining rationale for extending removeAll: to support
>> the special case of the receiver as an argument would be if it was
>> required by ANSI, or made the library or its specification simpler and
>> cleaner, or fixed an actual bug.  I believe that none of these has 
>> been
>> demonstrated to be true.
>
> Why would that be "the only remaining rationale"? Honestly, I am not
> joking?
> A good reason IMHO is to improve the class. Squeak is not bound by 
> ANSI.
> Sure, being ANSI compliant (which it isn't) is a good thing, but NOT if
> it stops us from improvement.

I think David is saying that he doesn't see any requirements for 
improvement, once ad hoc "would-be-nice-creeping-featuritis" is removed 
from the notion of an "improvement."  There are a kazillion ways to 
provide the functionality you require.

> If we see a "developer trap" (=a good possibility of making mistakes),
> which I surely would call this, then we should try to fix it.

David's assertion proposal addresses this without changing the 
semantics.

> (It's quite exactly the same problem as using removeAllSuchThat: on an
> ArrayedCollection.
>
> 'abc' removeAllSuchThat: [:c | c = $d ]

The problem here is not that it misbehaves for $a, but that it behaves 
at all for $d.   Assertions would solve the problem here as well.  (It 
is an awkward property of the Collection hierarchy as is that #remove 
is defined for immutable Collections).

> I agree with almost everything you said here about asserts except for
> the fact that I would not do an assert in this case and instead call
> removeAll on self and thus make the code work. :-) What is your 
> argument
> for not doing that? (an honest question)

Because it changes semantics by performing an implicit iteration on a 
collection that changes during the operation.  This is undefined in 
most Smalltalks (and as many have persuasively argued, this is as it 
should be).  By encouraging these special case expectations, we step 
into the land of the dangerous, handling only one of a host of possible 
cases.  The iterand, implicit or otherwise, should not be modified 
during such a loop.

>> However, the large collection argument is a good one, I think.  The
>> question then becomes, where does this method belong?  One could argue
>> that code dealing with such large collections almost always knows a 
>> lot
>> more about the collection than just that it is a collection that
>> supports remove:, so adding efficient removeAll methods to specific
>> collection classes could be done without adding it to the Collection
>> protocol itself.
>
> But why? It is such a basic message that I was actually perplexed to 
> see
> it wasn't in there already.
> But instead I find rather exotic messages like #removeAllSuchThat: ...

I have used the latter with some frequency, in fact, and haven't really 
had an application where i desired a #removeAll.  In practice, I never 
gave a second thought but to use the #removeAll: self copy solution.


> And again, why shouldn't it be used? And to be so overly defensive of
> adding (note, we are not changing an existing method, we are simply
> *adding* one) a method seems very counter productive to me. It would
> more or less totally hinder any evolution at all in the Collection
> hierarchy...

Many of David's "us" are of the view that we are "fixing" something 
that isn't broken.  Evolution of Collection is good.  Not all of "us" 
see this as even neutral.




More information about the Squeak-dev mailing list