Hi all!
Quoting "Andrew C. Greenberg" werdna@mucow.com:
On Wednesday, August 28, 2002, at 06:12 AM, goran.hultgren@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.
Yes, I know. I was just trying to emphasize the fact that the situation can occur without having coded it explicitly. A developer typing "x removeAll: x" would probably reflect on the fact that it could mean trouble. But as I said, the situation could be "hidden" in the code.
- 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.
Excuse me for my ignorance in this area, but:
1. I (and a newbie I guess) wouldn't consider removeAll: to be an iteration operation. That to me seems to be an implementation thing. 2. Where is this "semantics rule" you mention documented? I haven't read many Smalltalk books...
- "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.
Ok, I have been coding Smalltalk for many years and not until this thread popped up had I really reflected on the fact that "x removeAll: x copy" would be the "obvious" way of emptying a collection. I do not do this routinely. And you can not convince me that a newbie would "guess" this.
- 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.
I must say that I am on Richard side in this. We are not talking "featuritis", we are fixing buggy code. Noone can convince me that:
|oc| o := OrderedCollection with: 1 with: 2. o removeAll: o. o
Should give the result it does. NO WAY. An assert (as I gather you favour) is much better than the current situation, but I still can't really understand why we just couldn't "repair" removeAll: so that it works as expected. And I can't really see what "iteration semantics" has got to do with it...
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.
So the semantics you keep mentioning, what is that? I guess we are talking about "Do not change the collection during an iteration over the same, it is not defined." (or something like that)
And where am I iterating when I type: "x removeAll: x"? I have no idea what x will do to accomplish the goal of removing all those elements I am sending in - I could guess there might be an iteration in there somewhere but why should I need to know that?
(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).
Sure. It was a pretty far fetched comparison, I was trying to show that methods failing "silently" because of unexpected input is simply buggy IMHO.
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
Hmmm. I can't see where the "implicit iteration" takes place. With my proposed changes (check for identity calls #removeAll instead) no iteration would take place - the collection would simply realize it should empty itself and just does that.
And by the way, what do you think of removeAllSuchThat: ? It also changes the underlying colllection during the operation. But it uses a copy to get away with it... Perhaps we should then remove that method according to these arguments? Or perhaps I misunderstood it.
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.
I understand what you are saying, but I don't agree that #removeAll: should be considered to be an "iteration method".
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.
But surely you agree that "x removeAll" is much more precise and obvious than coming up with "x removeAll: x copy"? I mean, just standing there in front of a newbie Smalltalker and saying "No, there is no message for emptying a collection. Instead you write 'x removeAll: x copy'. Isn't that obvious?" Nah. Don't buy it.
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.
I didn't understand the last sentence.
regards, Göran
PS. I saw the commit emails, did you get the RePlugin up there alright? DS
Göran Hultgren, goran.hultgren@bluefish.se GSM: +46 70 3933950, http://www.bluefish.se "Department of Redundancy department." -- ThinkGeek