[squeak-dev] The Trunk: Collections-nice.820.mcz
Nicolas Cellier
nicolas.cellier.aka.nice at gmail.com
Thu Feb 14 09:59:17 UTC 2019
Chris Muller wrote in another thread:
> There was another change to earlier today that you may be interested
> in asking that question about too, since it changed 19-year old
> SequenceableCollection>>#= with a one-day old replacement and actually
> went into trunk.
Please note that this has been discussed more than a month ago, and the
discussion clearly converged to this consensus:
- having Interval and Array being = is not an important feature
- it seriously impacts efficiency of Interval hash
- it causes long standing bugs that we carry for years (19 years or so)
What's the point of having Interval = Array, when we have OrderedCollection
~= Array at the same time?
A less arbitrary feature would be a generalization of eachOperand
isSequenceable ==> testThatElementsAreSameSequence:
But it's both too hard to maintain (a bug factory), aand a problem for
efficient implementation of some specialized collection.
At the end, it's maybe not so much a desirable feature and it's practically
not used in trunk images, apart for writing shorter tests.
I'm not a big fan of C motto when used with extremism: you don't pay for
what you don't buy.
But I have the feeling that it makes sense here.
BTW, I don't like hasEqualElements: because I mentally associate elements
to Set, and for me #(1 2 3) hasEqualElements: #(3 1 2), just with a
different order.
I would prefer something like isSameSequenceAs: which clearly tells about
the ordering.
Le mer. 13 févr. 2019 à 17:21, Chris Cunningham <cunningham.cb at gmail.com> a
écrit :
> Thank you. This was an angle I hadn't thought of - but definitely opens up
> how to fix it correctly:
> *>Definitively abandon SequenceableCollection equality tests based on
> equal species. *
>
> On Tue, Feb 12, 2019 at 2:56 PM <commits at source.squeak.org> wrote:
>
>> Nicolas Cellier uploaded a new version of Collections to project The
>> Trunk:
>> http://source.squeak.org/trunk/Collections-nice.820.mcz
>>
>> ==================== Summary ====================
>>
>> Name: Collections-nice.820
>> Author: nice
>> Time: 12 February 2019, 11:56:35.262017 pm
>> UUID: bb383133-067c-4133-987b-c481a7de69c7
>> Ancestors: Collections-ul.819, Collections-cbc.813
>>
>> Definitively abandon SequenceableCollection equality tests based on equal
>> species.
>>
>> Old behaviour can still be obtained thru hasEqualElements: but the
>> default is to not try to support such trans-class equality feature because
>> it is much too complex.
>>
>> Particularly Interval are no more equal to Arrays with same sequence. We
>> can thus optimize hash a bit more and fix the old bugs of equa objects with
>> different hashes. Merge Collections-cbc.813 for this and rehashAll in
>> postscript.
>>
>> There are not so many classes concerned by this change, mainly RunArray,
>> Interval and LinkedList:
>>
>> Collection withAllSubclasses select: [:e | [e basicNew species ~= e] on:
>> Error do: [false]]
>> -> an OrderedCollection(WeakRegistry LinkedList Interval ByteCharacterSet
>> CharacterSetComplement LazyCharacterSet WideCharacterSet ShortRunArray
>> Semaphore Mutex TextLineInterval WeakArray Monitor MCVersionName ByteSymbol
>> WideSymbol)
>>
>> We will have to change the tests that rely on such equality.
>>
>> =============== Diff against Collections-ul.819 ===============
>>
>> Item was changed:
>> ----- Method: FloatArray>>= (in category 'comparing') -----
>> = aFloatArray
>> - | length |
>> <primitive: 'primitiveEqual' module: 'FloatArrayPlugin'>
>> + ^super = aFloatArray!
>> - aFloatArray class = self class ifFalse: [^ false].
>> - length := self size.
>> - length = aFloatArray size ifFalse: [^ false].
>> - 1 to: self size do: [:i | (self at: i)
>> - = (aFloatArray at: i) ifFalse: [^ false]].
>> - ^ true!
>>
>> Item was changed:
>> ----- Method: Interval>>= (in category 'comparing') -----
>> = anObject
>> -
>> ^ self == anObject
>> + or: [anObject isInterval
>> + ifFalse: [super = anObject]
>> + ifTrue:
>> + [start = anObject first
>> + and: [step = anObject increment
>> + and: [self last = anObject last]]]]!
>> - ifTrue: [true]
>> - ifFalse: [anObject isInterval
>> - ifTrue: [start = anObject first
>> - and: [step = anObject increment
>> - and: [self last = anObject last]]]
>> - ifFalse: [super = anObject]]!
>>
>> Item was changed:
>> ----- Method: Interval>>hash (in category 'comparing') -----
>> hash
>> "Hash is reimplemented because = is implemented."
>> + ^((start hash hashMultiply bitXor: self last hash) hashMultiply
>> + bitXor: self size)!
>> -
>> - ^(((start hash bitShift: 2)
>> - bitOr: stop hash)
>> - bitShift: 1)
>> - bitOr: self size!
>>
>> Item was changed:
>> ----- Method: RunArray>>= (in category 'comparing') -----
>> = anObject
>> + self == anObject ifTrue: [^ true].
>> - "Test if all my elements are equal to those of anObject"
>> -
>> ^anObject class == self class
>> + and:
>> - ifTrue: "Faster test between two RunArrays"
>> [(runs hasEqualElements: anObject runs)
>> + and: [values hasEqualElements: anObject values]]!
>> - and: [values hasEqualElements: anObject values]]
>> - ifFalse:
>> - [anObject isCollection and: [self
>> hasEqualElements: anObject]]!
>>
>> Item was changed:
>> ----- Method: SequenceableCollection>>= (in category 'comparing') -----
>> = otherCollection
>> "Answer true if the receiver is equivalent to the otherCollection.
>> + First test for identity, then rule out different class and sizes
>> of
>> - First test for identity, then rule out different species and
>> sizes of
>> collections. As a last resort, examine each element of the
>> receiver
>> and the otherCollection."
>>
>> self == otherCollection ifTrue: [^ true].
>> + self class == otherCollection class ifFalse: [^ false].
>> - self species == otherCollection species ifFalse: [^ false].
>> ^ self hasEqualElements: otherCollection!
>>
>> Item was changed:
>> ----- Method: Symbol>>= (in category 'comparing') -----
>> = aSymbol
>> "Compare the receiver and aSymbol."
>> self == aSymbol ifTrue: [^ true].
>> + aSymbol isSymbol ifTrue: [^ false].
>> - self class == aSymbol class ifTrue: [^ false].
>> "Use String comparison otherwise"
>> ^ super = aSymbol!
>>
>> Item was changed:
>> + (PackageInfo named: 'Collections') postscript: 'Character
>> initializeClassificationTable.
>> + HashedCollection rehashAll.'!
>> - (PackageInfo named: 'Collections') postscript: 'Character
>> initializeClassificationTable'!
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20190214/39434cf5/attachment.html>
More information about the Squeak-dev
mailing list
|