Hi all!

This breaks subclassing for Interval. TextLineInterval is an example. Yet, it is used only in ST80, so I will change it there.

What about #species? Or something similar? Do we really want to break subclassing on Interval?

Best,
Marcel
Am 03.03.2021 02:14:24 schrieb commits at source.squeak.org <commits at source.squeak.org>:
Nicolas Cellier uploaded a new version of Collections to project The Trunk:
http://source.squeak.org/trunk/Collections-nice.925.mcz

==================== Summary ====================

Name: Collections-nice.925
Author: nice
Time: 3 March 2021, 2:14:13.09562 am
UUID: ee92856e-98d1-3a41-a3f2-3529927e7a02
Ancestors: Collections-jar.924

Distinguish questionable LimitedPrecisionInterval (those with Float bounds or step) from ordinary Interval of Integer, Fraction or ScaledDecimals.

The main interest of having a specific class is to avoid crippling Interval with Float workarounds, for the rare use case.

Explain some of the pitfalls of LimitedPrecisionInterval, and encourage alternatives in class comment, which is a second advantage of having a separate class.

Abandon fuzzy inclusion logic, which is considered to introduce more discrepancies than it tries to solve
See http://bugs.squeak.org/view.php?id=6455.
and method #testSurprisingFuzzyInclusion

Fix two other failing tests for Interval of Floats.

Fix size so that (0.3 to: 1.2 by: 0.1) includes: 1.2.
See https://github.com/dolphinsmalltalk/Dolphin/issues/1108
This makes size a bit less performant than super, a 3rd reason why a specific class is neat.

Huh, that's more comments than code ;).

=============== Diff against Collections-jar.924 ===============

Item was changed:
----- Method: Interval class>>from:to: (in category 'instance creation') -----
from: startInteger to: stopInteger
"Answer an instance of me, starting at startNumber, ending at
stopNumber, and with an interval increment of 1."

+ ^((startInteger hasLimitedPrecision or: [stopInteger hasLimitedPrecision])
+ ifTrue: [LimitedPrecisionInterval]
+ ifFalse: [Interval]) basicNew
- ^self basicNew
setFrom: startInteger
to: stopInteger
by: 1!

Item was changed:
----- Method: Interval class>>from:to:by: (in category 'instance creation') -----
from: startInteger to: stopInteger by: stepInteger
"Answer an instance of me, starting at startNumber, ending at
stopNumber, and with an interval increment of stepNumber."

+ ^((startInteger hasLimitedPrecision or: [stopInteger hasLimitedPrecision or: [stepInteger hasLimitedPrecision]])
+ ifTrue: [LimitedPrecisionInterval]
+ ifFalse: [Interval]) basicNew
- ^self basicNew
setFrom: startInteger
to: stopInteger
by: stepInteger!

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 added:
+ Interval subclass: #LimitedPrecisionInterval
+ instanceVariableNames: ''
+ classVariableNames: ''
+ poolDictionaries: ''
+ category: 'Collections-Sequenceable'!
+
+ !LimitedPrecisionInterval commentStamp: 'nice 3/3/2021 01:47' prior: 0!
+ A LimitedPrecisionInterval is an Interval whose bounds or step haveLimitedPrecision.
+ Due to inexact arithmetic, special precautions must be taken in the implementation,
+ in order to avoid unconsistent and surprising behavior as much as possible.
+
+ Despite those efforts, LimitedPrecisionInterval is full of pitfalls.
+ It is recommended to avoid using LimitedPrecisionInterval unless understanding those pitfalls.
+ For example, (0.2 to: 0.6 by: 0.1) last = 0.5.
+ This interval does not includes 0.6 because (0.1*4+0.2) is slightly greater than 0.6.
+ Another example is that (0.2 to: 0.6 by: 0.1) does not include 0.3 but a Float slightly greater.
+
+ A usual workaround is to use an Integer interval, and reconstruct the Float inside the loop.
+ For example:
+ (0 to: 4) collect: [:i | 0.1*i+0.2].
+ or better if we want to have 0.3 and 0.6:
+ (2 to: 6) collect: [:i | i / 10.0].
+ Another workaround is to not use limited precision at all, but Fraction or ScaledDecimal when possible:
+ (1/10 to: 7/10 by: 1/10).
+ (0.1s to: 0.7s by: 0.1s).
+
+ Yet another pitfall is that optimized to:by:do: might differ from (to:by:) do:
+ In the former case, repeated addition of increment is used, in the later, a multiplication is used.
+ Observe the differences:
+ Array streamContents: [:str | 0 to: 3 by: 0.3 do: [:e | str nextPut: e]].
+ Array streamContents: [:str | (0 to: 3 by: 0.3) do: [:e | str nextPut: e]].
+
+ There are many more discrepancies, so use carefully, or not use it at all.!

Item was added:
+ ----- Method: LimitedPrecisionInterval>>copyFrom:to: (in category 'copying') -----
+ copyFrom: startIndex to: stopIndex
+ startIndex = 1 ifTrue: [^super copyFrom: startIndex to: stopIndex].
+ stopIndex < startIndex ifTrue: [^self copyEmpty].
+ ^Array new: stopIndex - startIndex + 1 streamContents: [:stream |
+ startIndex to: stopIndex do: [:i | stream nextPut: (self at: i)]]!

Item was added:
+ ----- Method: LimitedPrecisionInterval>>last (in category 'accessing') -----
+ last
+ "Refer to the comment in SequenceableCollection|last."
+
+ ^start + (step * (self size - 1))!

Item was added:
+ ----- Method: LimitedPrecisionInterval>>reversed (in category 'converting') -----
+ reversed
+ "There is no guaranty that super reversed would contain same elements.
+ Answer an Array instead"
+
+ ^Array new: self size streamContents: [:stream | self reverseDo: [:each | stream nextPut: each]]!

Item was added:
+ ----- Method: LimitedPrecisionInterval>>size (in category 'accessing') -----
+ size
+ "Answer how many elements the receiver contains."
+
+ | candidateSize |
+ candidateSize := (stop - start / step max: 0) rounded.
+ step > 0
+ ifTrue: [candidateSize * step + start <= stop ifTrue: [^candidateSize + 1]]
+ ifFalse: [candidateSize * step + start >= stop ifTrue: [^candidateSize + 1]].
+ ^candidateSize!


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.squeakfoundation.org/pipermail/squeak-dev/attachments/20210304/38842aef/attachment.html>