Nice. It seems like we have consensus on what to change.
I'll push these changes (with the tests) to trunk soon.
Hey, "seems" carries enough uncertainty to give us one final look before trunk. By "these changes" are you referring to just the Interval>>#hash or some Array changes, too? All we've seen so far are Collections-cbc.810.mcz, could we get one look at your final draft proposal before trunk?
On a less important note, I personally find a pure conditional nomenclature more attractive than the embedded ifTrue:ifFalse:, like:
= anObject ^ self == anObject or: [ (anObject isInterval and: [ start = anObject first and: [ step = anObject increment and: [ self last = anObject last ] ] ]) or: [ super = anObject ] ]
For whatever and whenever you push, I'm sure you already were but, just in case, I would be grateful if you would please base it solely off the current top trunk version with no intermediate versions in the ancestry. :-)
Thanks a lot finding this and helping get it fixed!
Best Regards, Chris
- Chris
The fix I have for #hash was exactly what Elliot suggested. I'll make sure to include the rehash as well (thanks for the code snippit Bert!) If no one objects strenuously, I'll also include Eliot's slight rewrite of #= has well - it is marginally cleaner and equally fast, so now is a reasonable time to include it.
I'll delay working on bug #3380 for now - to fix this, we'd have to also add in a check on class in #= to make sure we aren't comparing an interval to an array. Unless someone has been bitten by this recently, I'd rather wait.
-cbc
On Thu, Nov 15, 2018 at 1:32 PM Chris Muller asqueaker@gmail.com wrote:
Isn't this extremely simple to fix?
#= is implemented in terms of start, step, and last.
So why not implement #hash as
^(start hash bitXor: step hash) bitXor: self last hash
Then in the postscript do a
HashedCollection rehashAll
and that should be it, right?
I did a quick check using
Dictionary allInstances select: [ :dict | dict keys anySatisfy: #isInterval]
and the system seems fine after that.
You forgot about the universe that lies beyond the fringes of the image. There be persistent data files out there. And users. Possibly with systems that rely on Interval>>#hash.
But since most applications don't use non-Integer based Intervals (since Interval doesn't really support them, by design, I guess), there's no reason whatsoever to decimate the universe when Eliot's simply fixes the corner case that no one is using anyway.
+1 to Eliots suggestion to fix Interval>>#hash.
- Chris