IdentitySet>>collect:

On Wed, 26 Nov 2014, David T. Lewis wrote:

On Wed, Nov 26, 2014 at 08:01:49PM +0100, Levente Uzonyi wrote:

On Wed, 26 Nov 2014, Eliot Miranda wrote:

Hi All,
? ? IdentitySet>>collect: answers a Set, not an IdentitySet.? Anyone else
agree this is a serious bug?? Anyone else disagree?

WTF??

(IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e] a Set(1.0 2 3)
--
best,Eliot

Sometimes it's feasable to return an IdentitySet, other times it's not, so
there's no optimal solution - #collect: can't cover all cases.

Set >> #collect: explicitly returns a Set, because this is the least bad
solution to handle all of its subclasses reasonably well.
In case of WeakSet, returning a WeakSet makes no sense, because some of
your objects will disappear immediately.
In case of IdentitySet, PluggableSet, KeyedSet and KeyedIdentitySet the
returned values may not behave well in the original collection.

The best is to always be explicit:

(IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [ :e | e ] as:
IdentitySet
"==> an IdentitySet(1 2 3 1.0 2.0 3.0)"

Levente

In Squeak 2.8 (checked on http://bertfreudenberg.github.io/SqueakJS/run/):

 (IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e] ==> a Set(2.0 1 2 3 3.0 1.0)


But in trunk I get this:

 (IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e] ==> a Set(1 2 3)


So answering a Set makes sense for the reasons that Levente explains, but trunk
is definitely broken WRT the contents of that set.

This bug needs a unit test to go along with whatever fix we agree on.

It's not a bug either. The Floats are not there, because they are equal to the integers:

1 = 1.0 "==> true"
1.0 = 1 "==> true"

Therefore they have the same hash:

1 hash "==> 1"
1.0 hash "==> 1"

The bug was in Squeak 2.8:
1.0 = 1 "==> true"
1 = 1.0 "==> true"
1 hash "==> 1"
1.0 hash "==> 61440"

Here's the comment of Float >> hash from The Trunk:
        "Hash is reimplemented because = is implemented. Both words of the float are used. (The bitShift:'s ensure that the intermediate results do not become a large integer.) Care is taken to answer same hash as an equal Integer."

Levente

Dave

On Wed, Nov 26, 2014 at 1:26 PM, Levente Uzonyi <[hidden email]> wrote:

On Wed, 26 Nov 2014, David T. Lewis wrote:

On Wed, Nov 26, 2014 at 08:01:49PM +0100, Levente Uzonyi wrote:

On Wed, 26 Nov 2014, Eliot Miranda wrote:

Hi All,
? ? IdentitySet>>collect: answers a Set, not an IdentitySet.? Anyone else
agree this is a serious bug?? Anyone else disagree?

WTF??

(IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e] a Set(1.0 2 3)
--
best,Eliot

Sometimes it's feasable to return an IdentitySet, other times it's not, so
there's no optimal solution - #collect: can't cover all cases.

Set >> #collect: explicitly returns a Set, because this is the least bad
solution to handle all of its subclasses reasonably well.
In case of WeakSet, returning a WeakSet makes no sense, because some of
your objects will disappear immediately.
In case of IdentitySet, PluggableSet, KeyedSet and KeyedIdentitySet the
returned values may not behave well in the original collection.

The best is to always be explicit:

(IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [ :e | e ] as:
IdentitySet
"==> an IdentitySet(1 2 3 1.0 2.0 3.0)"

Levente

In Squeak 2.8 (checked on http://bertfreudenberg.github.io/SqueakJS/run/):

 (IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e] ==> a Set(2.0 1 2 3 3.0 1.0)


But in trunk I get this:

 (IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e] ==> a Set(1 2 3)


So answering a Set makes sense for the reasons that Levente explains, but trunk
is definitely broken WRT the contents of that set.

This bug needs a unit test to go along with whatever fix we agree on.

It's not a bug either. The Floats are not there, because they are equal to the integers:

1 = 1.0 "==> true"
1.0 = 1 "==> true"

Therefore they have the same hash:

1 hash "==> 1"
1.0 hash "==> 1"

The bug was in Squeak 2.8:
1.0 = 1 "==> true"
1 = 1.0 "==> true"
1 hash "==> 1"
1.0 hash "==> 61440"

Here's the comment of Float >> hash from The Trunk:
        "Hash is reimplemented because = is implemented. Both words of the float are used. (The bitShift:'s ensure that the intermediate results do not become a large integer.) Care is taken to answer same hash as an equal Integer."

Exactly.

My hope would be something like this (Set order is arbitrary)

        ((IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e]) => IdentitySet (1 2 3 1.0 2.0 3.0)

but that

       (Set withAll: #(1 2 3 1.0 2.0 3.0)) => Set (1 2 3)

(and it always contains just the integers since these are added first).

But Levente's collect:as: solution is perfectly acceptable and as he points out has the advantage of being explicit.

Levente

Dave

--

best,

Eliot

hmm, the problem is different IMO,

 

**corrected, but still wrong**

(IdentitySet withAll: #(1 1.0)) collect: [:e| e asInteger ]   IdentitySet(1 )

 

collect: should always preserve the size of the original collection so it must return an OrderedCollection !

---

Von: [hidden email] [mailto:[hidden email]] Im Auftrag von Eliot Miranda
Gesendet: Mittwoch, 26. November 2014 22:57
An: The general-purpose Squeak developers list
Betreff: Re: [squeak-dev] IdentitySet>>collect:

 

 

 

On Wed, Nov 26, 2014 at 1:26 PM, Levente Uzonyi <[hidden email]> wrote:

On Wed, 26 Nov 2014, David T. Lewis wrote:

On Wed, Nov 26, 2014 at 08:01:49PM +0100, Levente Uzonyi wrote:

On Wed, 26 Nov 2014, Eliot Miranda wrote:

Hi All,
? ? IdentitySet>>collect: answers a Set, not an IdentitySet.? Anyone else
agree this is a serious bug?? Anyone else disagree?

WTF??

(IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e] a Set(1.0 2 3)
--
best,Eliot

Sometimes it's feasable to return an IdentitySet, other times it's not, so
there's no optimal solution - #collect: can't cover all cases.

Set >> #collect: explicitly returns a Set, because this is the least bad
solution to handle all of its subclasses reasonably well.
In case of WeakSet, returning a WeakSet makes no sense, because some of
your objects will disappear immediately.
In case of IdentitySet, PluggableSet, KeyedSet and KeyedIdentitySet the
returned values may not behave well in the original collection.

The best is to always be explicit:

(IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [ :e | e ] as:
IdentitySet
"==> an IdentitySet(1 2 3 1.0 2.0 3.0)"

Levente

In Squeak 2.8 (checked on http://bertfreudenberg.github.io/SqueakJS/run/):

 (IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e] ==> a Set(2.0 1 2 3 3.0 1.0)


But in trunk I get this:

 (IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e] ==> a Set(1 2 3)


So answering a Set makes sense for the reasons that Levente explains, but trunk
is definitely broken WRT the contents of that set.

This bug needs a unit test to go along with whatever fix we agree on.

 

It's not a bug either. The Floats are not there, because they are equal to the integers:

1 = 1.0 "==> true"
1.0 = 1 "==> true"

Therefore they have the same hash:

1 hash "==> 1"
1.0 hash "==> 1"

The bug was in Squeak 2.8:
1.0 = 1 "==> true"
1 = 1.0 "==> true"
1 hash "==> 1"
1.0 hash "==> 61440"

Here's the comment of Float >> hash from The Trunk:
        "Hash is reimplemented because = is implemented. Both words of the float are used. (The bitShift:'s ensure that the intermediate results do not become a large integer.) Care is taken to answer same hash as an equal Integer."

 

Exactly.

 

My hope would be something like this (Set order is arbitrary)

 

        ((IdentitySet withAll: #(1 2 3 1.0 2.0 3.0)) collect: [:e| e]) => IdentitySet (1 2 3 1.0 2.0 3.0)

 

but that

       (Set withAll: #(1 2 3 1.0 2.0 3.0)) => Set (1 2 3)

 

(and it always contains just the integers since these are added first).

 

But Levente's collect:as: solution is perfectly acceptable and as he points out has the advantage of being explicit.

 

Levente

Dave

 

 

--

best,

Eliot

It seems to me that Object>>species is intended to handle this sort of issue.

For IdentitySet, it answers what Eliot was expecting:

   IdentitySet new species ==> IdentitySet
   Set new species ==> Set

However, IdentitySet>>collect: does not make use of this, and it answers
a Set for the reasons that Levente explained.

Answering an Array or and OrderedCollection would not really make sense,
because sets are unordered collections (but Bag might be better).

Shouldn't we have an implementation of IdentitySet>>species that answers
Set (or Bag), with a method comment explaining why this is the case, and
with all of the collection methods using #species to answer the right
kind of result?

I note that IdentitySet>>collect: answers a Set, but IdentitySet>select:
sends #species and therefore answers an IdentitySet.

So I think that if we want the #species of an IdentitySet to be a Set,
then we should make it so, and give it a method comment to explain the
rationale. And the #collect: and #select: methods should both answer a
result of that #species.

Dave

On Thu, Nov 27, 2014 at 01:14:30AM +0100, Levente Uzonyi wrote:
> Your example hides the problem of ordering - what Tobias is asking about -
> so here's another:
>
> (IdentitySet withAll: #(1 1.0)) collect: [ :each | each class ]
>
> If IdentitySet >> #collect: were returning an Array, then what would be the
> answer?
>
> { SmallInteger. Float } or { Float. SmallInteger } ?
>
> If you really want to have the resulting collection have the same size,
> but avoid the problem with ordering, then what you really need is a Bag.
> Luckily you can have whatever you want (as long as it makes sense):
>
> (IdentitySet withAll: #(1 1.0)) collect: [ :each | each asInteger ] as: Set.
> "==> a Set(1)"
> (IdentitySet withAll: #(1 1.0)) collect: [ :each | each asInteger ] as:
> IdentitySet.
> "==> an IdentitySet(1)"
> (IdentitySet withAll: #(1 1.0)) collect: [ :each | each asInteger ] as:
> Array.
> "==> #(1 1)"
> (IdentitySet withAll: #(1 1.0)) collect: [ :each | each asInteger ] as:
> OrderedCollection.
> "==> an OrderedCollection(1 1)."
> (IdentitySet withAll: #(1 1.0)) collect: [ :each | each asInteger ] as: Bag.
> "==> a Bag(1 1)"
> (IdentitySet withAll: #(1 1.0)) collect: [ :each | each asInteger ] as:
> IdentityBag.
> "==> an IdentityBag(1 1)"
> (IdentitySet withAll: #(1 1.0)) collect: [ :each | each asInteger ] as:
> Heap.
> "==> a Heap(1 1)"
>
> Levente
>
> On Thu, 27 Nov 2014, Frank Lesser wrote:
>
> >Hi Tobias,
> >agree, a problem of "OrderedCollection"
> >not to break a lot of other things we could return an Array.
> >but for me collecting has priority.
> >Frank
> >

> >-----Urspr?ngliche Nachricht-----

> >Von: [hidden email]
> >[mailto:[hidden email]] Im Auftrag von
> >Tobias
> >Pape
> >Gesendet: Donnerstag, 27. November 2014 00:48
> >An: The general-purpose Squeak developers list
> >Betreff: Re: [squeak-dev] IdentitySet>>collect:
> >
> >
> >On 27.11.2014, at 00:34, Frank Lesser <[hidden email]>
> >wrote:
> >
> >>hmm, not convinced
> >>
> >>(IdentitySet withAll: #(1 1.0)) collect: [:e| e asInteger ]
> >>OrderedCollection(1 1 )
> >>
> >>in LSWVST ( one-to-one ), you collect results of evaluating a block on
> >>objects.
> >>
> >>Frank
> >>maybe I am wrong ...
> >
> >Where would the order come from for that _Ordered_Collection?
> >
> >Best
> >     -Tobias
> >