On Sun, Apr 7, 2019 at 11:10 PM <commits at source.squeak.org> wrote:
>
> Nicolas Cellier uploaded a new version of Collections to project The Inbox:
> http://source.squeak.org/inbox/Collections-nice.825.mcz
>
> ==================== Summary ====================
>
> Name: Collections-nice.825
> Author: nice
> Time: 7 April 2019, 11:10:11.564693 pm
> UUID: c9836c60-9acc-45eb-83b2-851f147b70ca
> Ancestors: Collections-nice.824
>
> Try to improve some Interval of Float Behavior.
>
> - change #last so that it agrees with (self at: self size)
>  Note that last behavior slightly changes for empty Interval: (3 to: 0) last is now 2 instead of 0 previously. I don't know of any code impacted, but it's not a completely neutral change
>
> Note: this solves 2 out of the 3 failures of #testIntervalOfFloatLast... The last element might still overshoot the prescribed upper bound. I can also fix it, but implementation of #last is getting overkill.
>
> - implement a ReversedInterval that lazily reverse an Interval of Float, and resolves #testIntervalOfFloatReversed.
>
> The alternative would be resorting to an Array...
>
> - renounce fuzzy inclusion test. As exhibited by #testSurprisingFuzzyInclusionBehavior, it creates more weirdness than it tries to solve. There will be two more test failing, asserting this fuzzy inclusion, but we can later change them.
>
> =============== Diff against Collections-nice.824 ===============
>
> Item was changed:
>   ----- Method: Interval>>indexOf:startingAt: (in category 'accessing') -----
>   indexOf: anElement startingAt: startIndex
>         "startIndex is an positive integer, the collection index where the search is started."
> -       "during the computation of val , floats are only used when the receiver contains floats"
>
> +       | index |
> -       | index val |
>         (self rangeIncludes: anElement) ifFalse: [ ^0 ].
> +       index := (anElement - start / step) rounded + 1.
> -       val := anElement - self first / self increment.
> -       val isFloat
> -               ifTrue: [
> -                       (val - val rounded) abs * 100000000 < 1 ifFalse: [ ^0 ].
> -                       index := val rounded + 1 ]
> -               ifFalse: [
> -                       val isInteger ifFalse: [ ^0 ].
> -                       index := val + 1 ].
> -       "finally, the value of startIndex comes into play:"
>         (index between: startIndex and: self size) ifFalse: [ ^0 ].
> +       (self at: index) = anElement ifFalse: [ ^0 ].
>         ^index!
>
> Item was changed:
>   ----- Method: Interval>>last (in category 'accessing') -----
>   last
>         "Refer to the comment in SequenceableCollection|last."
>
> +       ^start + (step * (self size - 1))!
> -       ^stop - (stop - start \\ step)!
>
> Item was changed:
>   ----- Method: Interval>>reversed (in category 'converting') -----
>   reversed
>         self isEmpty ifTrue: [^stop to: start by: step negated].
> +       (start isFraction and: [step isFraction]) ifFalse: [^ReversedInterval newFrom: self].
>         ^self last to: start by: step negated!
>
> Item was added:
> + SequenceableCollection subclass: #ReversedInterval
> +       instanceVariableNames: 'original'
> +       classVariableNames: ''
> +       poolDictionaries: ''
> +       category: 'Collections-Sequenceable'!
> +
> + !ReversedInterval commentStamp: 'nice 4/6/2019 16:30' prior: 0!
> + A ReversedInterval is an Interval reversed.
> + An Interval reversed is generally an Interval, except when made of Float.
> + This is because Float arithmetic does not guarantee reversible operations.
> +
> + Instead of transforming the reversed Float Interval into an Array, I keep a lazier and cheaper representation.
> + I can even pretend being an Interval myself. As long as I quake like a duck...
> +
> + Instance Variables
> +       original:               <Interval>
> +
> + original
> +       - the Interval being reversed
> + !
>
> Item was added:
> + ----- Method: ReversedInterval class>>newFrom: (in category 'as yet unclassified') -----
> + newFrom: aCollection
> +       "Answer an instance of me containing the same elements as aCollection, reversed."
> +
> +       aCollection isInterval ifTrue: [^self basicNew setOriginal: aCollection].
> +       ^(Interval newFrom: aCollection) reversed!
>
> Item was added:
> + ----- Method: ReversedInterval>>+ (in category 'arithmetic') -----
> + + aNumber
> +       ^(original + aNumber) reversed!
>
> Item was added:
> + ----- Method: ReversedInterval>>- (in category 'arithmetic') -----
> + - aNumber
> +       ^(original - aNumber) reversed!
>
> Item was added:
> + ----- Method: ReversedInterval>>= (in category 'comparing') -----
> + = anObject
> +       ^ self == anObject
> +               or: [anObject class = self class
> +                       and: [original = anObject reversed]]!
>
> Item was added:
> + ----- Method: ReversedInterval>>at: (in category 'accessing') -----
> + at: anInteger
> +       ^original at: original size + 1 - anInteger!
>
> Item was added:
> + ----- Method: ReversedInterval>>copyFrom:to: (in category 'copying') -----
> + copyFrom: startIndex to: stopIndex
> +       (startIndex = 1 and: [stopIndex = self size]) ifTrue: [^self].
> +       stopIndex < startIndex ifTrue: [^self copyEmpty].
> +       ^((self at: stopIndex) to: (self at: startIndex) by: original increment) reversed!
>
> Item was added:
> + ----- Method: ReversedInterval>>do: (in category 'enumerating') -----
> + do: aBlock
> +       original reverseDo: aBlock!
>
> Item was added:
> + ----- Method: ReversedInterval>>hash (in category 'comparing') -----
> + hash
> +       ^original hash bitXor: SmallInteger maxVal!
>
> Item was added:
> + ----- Method: ReversedInterval>>isEmpty (in category 'testing') -----
> + isEmpty
> +       ^self size = 0!
>
> Item was added:
> + ----- Method: ReversedInterval>>printOn: (in category 'printing') -----
> + printOn: aStream
> +       aStream
> +               nextPut: $(;
> +               print: original;
> +               nextPut: $);
> +               space;
> +               nextPutAll: #reversed!
>
> Item was added:
> + ----- Method: ReversedInterval>>reverseDo: (in category 'enumerating') -----
> + reverseDo: aBlock
> +       original do: aBlock!
>
> Item was added:
> + ----- Method: ReversedInterval>>reversed (in category 'converting') -----
> + reversed
> +       ^original!
>
> Item was added:
> + ----- Method: ReversedInterval>>setOriginal: (in category 'private') -----
> + setOriginal: anInterval
> +       original := anInterval!
>
> Item was added:
> + ----- Method: ReversedInterval>>size (in category 'accessing') -----
> + size
> +       ^original size!
>
> Item was added:
> + ----- Method: ReversedInterval>>sorted (in category 'sorting') -----
> + sorted
> +       original increment > 0 ifTrue: [^original].
> +       ^self!
>
> Item was added:
> + ----- Method: ReversedInterval>>species (in category 'private') -----
> + species
> +
> +       ^original species!
>
> Item was added:
> + ----- Method: ReversedInterval>>storeOn: (in category 'printing') -----
> + storeOn: aStream
> +       aStream
> +               nextPut: $(;
> +               store: original;
> +               nextPut: $);
> +               space;
> +               nextPutAll: #reversed!
>
> Item was added:
> + ----- Method: ReversedInterval>>sum (in category 'accessing') -----
> + sum
> +       ^original sum!
>
>

"An Interval reversed is generally an Interval, except when made of Float."
Should ReversedInterval be renamed to ReversedFloatInterval then?

Nonetheless, this seems to fix #testIntervalOfFloatLast and
#testIntervalOfFloatReversed of IntervalTest. Do we want to integrate
this proposal or shall we mark these two tests as expected failures?

Fabio