On Tue, Sep 10, 2002 at 09:54:40AM -0400, Jesse Welton wrote:
> Richard, I think you misunderstand Lex's point.  I believe it is that
> a reasonable programmer might rewrite a working call
>     a removeAll: b
> as
>     b do: [:each | a remove: each].
> 
> Now, if #removeAll: detects the special case a==b and handles it, it
> is possible that this case happens in practice, and the programmer's
> rewritten code will break silently.  On the other hand, is #removeAll:
> detects a==b and throws an error, this case probably does not happen
> in practice, and the programmer's code will work.  Therefore, given
> that this is a reasonable transformation for the programmer to make,
> throwing an error is safer.
> 
> I don't buy it, though.  I don't believe that it is, in general, a
> reasonable transformation to make.  It is longer and less intention
> revealing.  It is more error-prone than either fixed version of
> #removeAll:, since it has the same silent failure everyone seems to
> agree should be fixed in the current implementation of #removeAll:.
> It is also potentially slower, since it cannot benefit from (future)
> enhancements to #removeAll: performance.


Yeah, that's what I meant.  I still think it's likely -- it's the way I
code, and it's the way I've heard many people suggest coding.  You make
reasonable-looking changes, and you assume that the code you are
calling does reasonable things.  Then you test it, to see if everything
seems to be working okay.  It's unreasonable to do full proofs of
correctness, and it's unhelpful to point the finger and say it's so and
so's responsibility to do such an unreasonable thing.

Incidentally, I don't think people would stop with the loop.  I meant
it as a step along the way to something like this:


	b do: [ :each |
		logfile logImminentRemoval: each.
		a remove: each.
		logfile logRemovalCompletion: each ]


That is, opening up the loop gives you extra power, helping you to
deal with requirements that become more complicated over time.  I do
this kind of thing all the time, and if I saw an error from this change,
then I'd first look at the logRemoval: code, not the conversion of a
removeAll: to a loop of remove's.

And besides, if you have copies of the whole collection being returned
from methods, and that collection gets mutated, then you are in a
dangerous situation regardless of the instance of removeAll:.

	| x |
	x := self allFooishMembers.
	self removeNonTrueBelievers.

	"at this point, is x the same as it was
	 up above?  you'd like it to be!!"




-Lex