<div dir="ltr">HI Chris,<div><br></div><div>The changes will be limited to Interval, and will be changes to #= and hash (and the interval test so this doesn't show up again).</div><div><br></div><div>I'll push the changes to inbox soon; and to trunk tomorrow/early next week.  The test will go to Trunk with the changes to inbox (the test will be what I've pushed to the inbox minus the 3380 part).</div><div><br></div><div>And, yes, I'll rebase if off of the current trunk version - there has been significant changes since my last proposal.</div><div><br></div><div>Interestingly:</div><div><br></div><div>= anObject<br>    ^ self == anObject or:<br>        [ (anObject isInterval<br>            and:<br>                [ start = anObject first and:<span class="gmail-im" style="color:rgb(80,0,80)"><br>                    [ step = anObject increment and: [ self last =<br></span>anObject last ] ] ])</div><div>            or: [ super = anObject ] ]  <br></div><div><br></div><div>This is actually wrong - if the two items to compare are intervals but they don't match based on interval hash (first/last/increment), then it will check if super #= returns true - that is not desirable.  But, I understand the desire you mention here - which I believe is what Eliot was driving for as well.</div><div><br></div><div>-cbc</div></div><br><div class="gmail_quote"><div dir="ltr">On Thu, Nov 15, 2018 at 2:21 PM Chris Muller <<a href="mailto:asqueaker@gmail.com">asqueaker@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">> Nice.  It seems like we have consensus on what to change.<br>
><br>
> I'll push these changes (with the tests) to trunk soon.<br>
<br>
Hey, "seems" carries enough uncertainty to give us one final look<br>
before trunk.  By "these changes" are you referring to just the<br>
Interval>>#hash or some Array changes, too?  All we've seen so far are<br>
Collections-cbc.810.mcz, could we get one look at your final draft<br>
proposal before trunk?<br>
<br>
On a less important note, I personally find a pure conditional<br>
nomenclature more attractive than the embedded ifTrue:ifFalse:, like:<br>
<br>
= anObject<br>
    ^ self == anObject or:<br>
        [ (anObject isInterval<br>
            and:<br>
                [ start = anObject first and:<br>
                    [ step = anObject increment and: [ self last =<br>
anObject last ] ] ])<br>
            or: [ super = anObject ] ]<br>
<br>
For whatever and whenever you push, I'm sure you already were but,<br>
just in case, I would be grateful if you would please base it solely<br>
off the current top trunk version with no intermediate versions in the<br>
ancestry.   :-)<br>
<br>
Thanks a lot finding this and helping get it fixed!<br>
<br>
Best Regards,<br>
  Chris<br>
<br>
 - Chris<br>
<br>
<br>
> The fix I have for #hash was exactly what Elliot suggested.<br>
> I'll make sure to include the rehash as well (thanks for the code snippit Bert!)<br>
> 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.<br>
><br>
> 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.<br>
><br>
> -cbc<br>
><br>
><br>
> On Thu, Nov 15, 2018 at 1:32 PM Chris Muller <<a href="mailto:asqueaker@gmail.com" target="_blank">asqueaker@gmail.com</a>> wrote:<br>
>><br>
>> > Isn't this extremely simple to fix?<br>
>> ><br>
>> > #= is implemented in terms of start, step, and last.<br>
>> ><br>
>> > So why not implement #hash as<br>
>> ><br>
>> > ^(start hash bitXor: step hash) bitXor: self last hash<br>
>> ><br>
>> > Then in the postscript do a<br>
>> ><br>
>> > HashedCollection rehashAll<br>
>> ><br>
>> > and that should be it, right?<br>
>> ><br>
>> > I did a quick check using<br>
>> ><br>
>> > Dictionary allInstances select: [ :dict | dict keys anySatisfy: #isInterval]<br>
>> ><br>
>> > and the system seems fine after that.<br>
>><br>
>> You forgot about the universe that lies beyond the fringes of the<br>
>> image.  There be persistent data files out there.  And users.<br>
>> Possibly with systems that rely on Interval>>#hash.<br>
>><br>
>> But since most applications don't use non-Integer based Intervals<br>
>> (since Interval doesn't really support them, by design, I guess),<br>
>> there's no reason whatsoever to decimate the universe when Eliot's<br>
>> simply fixes the corner case that no one is using anyway.<br>
>><br>
>> +1 to Eliots suggestion to fix Interval>>#hash.<br>
>><br>
>>  - Chris<br>
>><br>
><br>
<br>
</blockquote></div>