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.
Dave