[squeak-dev] The Trunk: Collections-nice.820.mcz
Chris Muller
asqueaker at gmail.com
Fri Feb 15 22:23:57 UTC 2019
> > 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)
Yes, I remember that discussion.
> 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.
I have no reason to critique this reasoning, I simply did not have
time to test with it yet. Sometimes testing after tweaks at low
levels can "reveal" things that weren't thought about before. But
also, what I really dislike are continued use == on the class test.
I'm sorry, but these kinds of hacks are killing Squeak's dynamism by a
thousand cuts and me along with it. :( Would you **Please
Delegate** equivalence tests (#=) to the receiver, and let them
inherit the identity implementation from Object?
- Chris
- Chris
> 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'!
>>>
>>>
>>
>
More information about the Squeak-dev
mailing list
|