[vwnc] [7.6] Collection>>removeAll inconsistency?

Travis Griggs wrote:
> Is there really 2 *real* use cases defined? removeAll was added as a
> result of a variety of customers over the years requesting this
> utility be added, since they were adding it themselves. Of the 2 or 3
> submissions we looked at, none of them bothered to return something
> other than the receiver.

Any conclusions drawn from that should be tempered by the fact that most
of the time, the return value of #remove* is thrown away. An implementer
of a new #removeAll is thus unlikely to think about an explicit return
value. In my image only 2 out of 51 senders of #removeAll: use the
return value. And 2 out of 6 implementors of #removeAll: don't bother to
return the removed elements (Polymorphic.GFCompositeGO and
Net.MessageHeader).

In addition, [Abstract]ChangeList[Controller] and
LinkedOrderedCollection have both implemented #removeAll just returning
self since at least VW2.0 - and were the only base #removeAll
implementors until 7.6.

For my money, the use cases don't split on what is returned, but on what
the internal state of the resulting collection is. Sometimes I want its
capacity to be trimmed back to the minimum to reduce storage, sometimes
I want to keep the capacity intact to prevent the cost of regrowing. I'd
actually name the methods #empty and #emptyAndTrim. I count 6
instance-side implementors of #empty in a normal VW image, all with the
same semantics. (Class-side implementers return a new empty instance.)

All this is just to answer Travis' request for comments - #removeAll[:]
is needed so infrequently that it's not something I care deeply about.

Steve

_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc

>
>I certainly agree. I don't like being surprised.
>
>But isn't this a dangerous rule Dave?


Open the pod bay doors, Hal.


>It basically gives every programmer the ability to proclaim something  
>is bad because they got surprised when their expectations weren't met.  


Don't sweat the small stuff.
You can't `give' an ability that is already hard-wired.


>_Anecdotal vs Theory
>It's interesting that we're projecting what might happen because of  
>the way the removeAll was implemented.


www.c2.com/wiki/JustAProgrammer


>So we've got practice vs theory here.

  === Anecdotal ==

  " A1 ::= as a procedure: remove all contents. "
  self removeAll.
  self assertFurtherInterest: nil.

  " A2 ::= as a function: remove and answer all contents. "
  self removeAll do:
      [:each|
       self oneLastUseFor: each
      ]

 == Theory ==

  " T1 ::= as a procedure: remove all contents, answer self. "
  >>removeAll
        self removeAll: self

  " T2 ::= as a function: remove and answer all contents. "
  >>removeAll
        ^self removeAll: self copy

  Note that anecdote #2 is actually suggesting something different,
  because the values returned are logically required to be used immediately,
  just before they become garbage.  There are four variants:

  " V1 ::= enumerate and remove contents. "
  >>removeAllDo: a1block
        self do: a1block.
        self removeAll

  " V2 ::= remove and enumerate contents. "
  >>removeAllDo: a1block
        (self removeAll: self copy) do: a1block

  " V3 ::= enumerate contents, removing each before evaluating it "
  >>removeAllDo: a1block
        [ self isEmpty
        ] whileFalse:
              [ a1block value: self removeFirst ]

  " V4 ::= enumerate contents, removing each after evaluating it "
  >>removeAllDo: a1block
        self do:
            [:each|
             a1block value: each.
             self removeFirst
            ]

>_How Obvious is it for removeAnything to Return Anything in Particular?
>This is another interesting aspect to look at. Only when taken in the  
>context of just the VisualWorks class library does this make sense.


Of course - because VW rocks.
Or it did, until the big #removeAll snafu.
So just who is responsibile for this catastrophe?
Heads must roll!


>A precedent was set there.


Not much of a precedent, according to your citations.
Other people did things differently, at nearly every opportunity.


>_What is the Convention Anyway?
>...
>One thing all the other methods have  
>that removeAll doesn't, is an argument.


Aha - now that's an argument worth persuing.


>Not just the removeVariants:, but the various addVariants: too.


Right on.
And as an interesting aside -
- what should #addAll return?


> What's being returned is the argument


True, but here comes another one...


>(or the implied one, e.g.  removeAt:).


Eyooh.  The implied one?  Really?
Yuck.  A predicate calculus this ain't.


>Why is this?


Good question.  Survey says...



Not too bad, as hand waving goes.

I'd argue that the
add and remove methods
all mutate the receiver,
answering the difference.


>The point of interest is now
>the collection getting the enema,
>not what's coming out.
>So it might make sense
>to make that the return value.


  !


> From this view point, it is the least astonishing


Got to be the first time
that an analogy by enema
is the least astonishing!


>The thoughtful reader might point out removeLast has no argument.


An alpha would pick removeFirst.
Now stand still while I sniff your butt...


>... and fold the same argument on to the removeAll,
>which is a short cut for self removeAll: self.


or a shortcut for
  self removeAll: self copy.
in which case, you can't.


>... an argument revolving around whether there is an argument or not).


Gmta!


>_Perverse Principle Allegiance Principle?


Perhaps.  If so, then
TSTTCPW is - remove it.

Too ambiguous, too costly to do right,
and too easy to do without.

For the procedural use case (A1), we have (T1).
For the functional version (T2), we really have
multiple use cases (A2 and V1,V2,V3,V4).

A2 is not much of a use case for T2,
and is actually less desirable than any of
the V1, V2, V3, V4 implementations.
But T2 may have other use cases.

On the other hand, consistency has the benefit
of being, well, consistent. Often a good thing.

In which case #clearAll is a better name for T1,
and while #popAll might be a more intention revealing
name for T2, #removeAll is definitely more consistent.

 

Regards,

-cstb

"Ever meta circulara tree?
 Eschews leaves and branches.
" - Uell Gibbons


_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc

Travis Griggs wrote:
Yes! Without looking at the implementation, can anyone tell me what this
returns?

| added removed |
added := 1.
removed := 1.0.
#(#{Bag} #{List} #{OrderedCollection} #{OrderedSet} #{Set})
        collect: [:each |
                | collection |
                collection := each value new.
                collection add: added.
                (collection remove: removed) class]


Running the following on Dolphin and VA give different results:

| added removed |
added := 1.
removed := 1.0.
#(Bag OrderedCollection Set)
        collect: [:each |
                | collection |
                collection := (Smalltalk at: each) new.
                collection add: added.
                (collection remove: removed) class]

In Dolphin you get Float and two SmallIntegers, but in VA, you get two
Floats and SmallInteger. I tried it in a 3.9 Squeak image and it gave an
error since:
        1 = 1.0 --> true
        1 hash --> 1
        1.0 hash --> 61440

I can't find a Smalltalk that is consistent -- let alone follows any
principle of least astonishment. If we are going to return the object
that was removed, my own preference is to return the object in the
collection that was removed, and not the object that was passed in. I
can assign the passed in object to a temporary variable if I need it.

Of course, if we change remove: to return the actual object from the
collection that was removed, what should removeAll: return? Should we
create a new collection where each object is the object that was the
actual removed object? However, since 31 out of 33 cases of #removeAll:
in my image don't use the return value, why penalize the common case by
creating a new collection? Instead we could insist that the 2 cases that
needed the value of #removeAll: be rewritten to explicitly create the
new collection if it was needed. BTW, 3 of the 33 cases would have
preferred the #removeAll: method to return the receiver as they cascaded
other messages to the collection (e.g., a removeAll: b; yourself).

Anyway, after 17 years of Smalltalk, my expectation of a
#remove:/removeAll: method is that they return.


John Brant
_______________________________________________
vwnc mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/vwnc