[BUG]Collection>>removeAll:

Richard A. O'Keefe ok at cs.otago.ac.nz
Mon Aug 26 00:07:46 UTC 2002


"Andrew C. Greenberg" <werdna at mucow.com> wrote:
	I don't know about this Richard.  #removeAll: implies certain 
	side-effects with respect to the receiver that render its meaningful 
	definition undefined when the parameter is modified during its 
	operation.

I wish people wouldn't use the term "SIDE-effect" for what is the
MAIN effect of an operation.  It's an important distinction.  Just as
when a doctor prescribes you a drug, the SIDE effects of that drug are
almost certainly not intended by her, so when an operation has effects
other than its main defined purpose, those side effects may well not be
intended by the user.

Now #removeAll:, though grossly inefficient, is in other ways a well
behaved operation.  It doesn't _have_ any side effects.  Except in the
case where the bug bites, it has *precisely* the intended effect on the
receiver, and does not try to change any other object.

It is only when the receiver and parameter are aliased that there is
any confusion, and that confusion is purely a result of the sloppy way
the method is coded, not an intrinsic property.

As I have noted in an earlier message, the ANSI Smalltalk standard
(at least in the 1.9 draft) does in fact give a complete definition of
what should happen in this case (whether it was _intended_ to do so or
not is a separate and not very interesting topic).

It is clear from the ANSI spec. that what should happen in the case
of x removeAll: x is the same as what should happen in the case of
x removeAll: (x shallowCopy).

The effect on the *receiver* is always well defined.

Normally, there is not any side effect.
When the parameter is aliased to the receiver, _then_ you might call
the effect on the parameter a side effect, but there is no reason why
that should have any observable consequences _during_ the invocation.

	By extending the defined range of #removeAll: for some,

But I am doing no such thing.  #removeAll: *has* a defined domain.
It's clearly and unambiguously spelled out in the ANSI spec and in the
Squeak comments.  The sloppy code in Squeak happens not to _work_ in
all of that domain, but the code isn't the definition.

	but not all parameters that are modified during its operation,
	we invite a more specific definition of the selector, perhaps:

I don't understand you here.  The definition

    removeAll: aCollection
	aCollecton == self ifTrue: [^self removeAll: aCollection copy].
	^aCollection do: [:each | self remove: each]

modifies all and only the things it is supposed to modify;
when aCollection is empty, it modifies nothing;
when aCollection is not empty, it modifies the receiver
AND NO OTHER OBJECT, whether aCollection == self or not.
If the parameter is aliased to the receiver,
then the parameter observes the changes to the receiver,
as it must.

	This suggests to me that the ugly hack characterization isn't entirely 
	off-base.
	
That characterisation might have one foot in the ground,
but the other is rammed firmly in its mouth or possibly some other orifice.
As a characterisation of the _implementation_ of the correction,
it has as much plausibility as offensiveness.
(But how _else_ would you fix an aliasing bug, if not by making a copy?)
As a characterisation of the _effect_ of the correction,
it has none.

What I want is some clear principled explanation of why
making (x addAll: x) and (x removeAll: x) should be
"an ugly hack"
but the fact that (x add: x) and (x remove: x) already DO work
is allegedly _not_ "an ugly hack".

It's precisely the same situation in both cases.
If you do
    x := OrderedCollection new.
    x add: x.
then this invocation, which succeeds quietly without any problems,
*HAS* "modified the parameter" as well as the receiver.
Why is this OK, but
    x addAll: x
beyond the pale, given that it _can_ work?




More information about the Squeak-dev mailing list