git.iko.soy/rowtype-yoga/purescript-fixed-precision
on main help sign in
Fix String conversions with negatives #26 (#27)

Both toString and fromString were very broken in the presence of negative numbers. This commit fixes them, and also fixes the tests so that they generate negative numbers in the property tests too.

commitab302f2a90e3a82a4fc9bad9cc7305f8e1e7f298 authorHarry Garrood <harry@garrood.me> dateThu 5 Mar 2020, 12:45 (7 years ago) parentabe61bc
2 files changed+29−14
compare with main
unifiedsplit
src/Data/Fixed.purs+16−5 test/Main.purs+13−9
src/Data/Fixed.purs+16−5
@@ -44,6 +44,7 @@ import Data.BigInt as BigInt
4444import Data.Int as Int
4545import Data.Maybe (Maybe(..), fromJust)
4646import Data.Monoid as Monoid
47+import Data.Ord (abs)
4748import Data.String.CodeUnits as StringCU
4849import Data.String as String
4950import Math as Math
@@ -344,12 +345,19 @@ fromString
344345 . KnownPrecision precision
345346 => String
346347 -> Maybe (Fixed precision)
347−fromString str =
348+fromString str' =
348349 let
349350 numDigits = reflectPrecisionDecimalPlaces (PProxy :: PProxy precision)
350351 denom = reflectPrecision (PProxy :: PProxy precision)
351352
352353 isDigit = between '0' '9'
354+
355+ { sign, str } =
356+ case String.stripPrefix (String.Pattern "-") str' of
357+ Just str ->
358+ { sign: negate one, str }
359+ Nothing ->
360+ { sign: one, str: str' }
353361 wholeDigits = StringCU.countPrefix isDigit str
354362 { before, after } = StringCU.splitAt wholeDigits str
355363
@@ -365,7 +373,7 @@ fromString str =
365373 guard (StringCU.charAt 0 after == Just '.')
366374 let raw = StringCU.drop 1 after
367375 BigInt.fromString (rightJustify numDigits '0' raw)
368− pure (Fixed (wholePart * denom + fractionPart))
376+ pure (Fixed (sign * (wholePart * denom + fractionPart)))
369377
370378-- | Represent a `Fixed` value as a string, with the given number of decimal
371379-- | places.
@@ -392,10 +400,13 @@ toStringWithPrecision requestedDigits fixed@(Fixed n) =
392400 let
393401 denom = denominator fixed
394402 denomDigits = reflectPrecisionDecimalPlaces (PProxy :: PProxy precision)
395− wholePart = n / denom
396− fractionalPart = n `mod` denom
403+ absNum = abs n
404+ -- Use 'quot' and 'rem' because we need to round towards zero
405+ wholePart = BigInt.quot absNum denom
406+ fractionalPart = BigInt.rem absNum denom
397407 in
398− BigInt.toString wholePart
408+ (if n < zero then "-" else "")
409+ <> BigInt.toString wholePart
399410 <> Monoid.guard (requestedDigits > 0)
400411 ("." <>
401412 rightJustify requestedDigits '0'
test/Main.purs+13−9
@@ -2,7 +2,7 @@ module Test.Main where
22
33import Prelude
44
5−import Data.Fixed (class KnownPrecision, Fixed, PProxy, fromNumber, toNumber, fromString, reifyPrecision, reflectPrecision, toString, P100, rescale, fromBigInt, floor)
5+import Data.Fixed (class KnownPrecision, Fixed, PProxy, fromNumber, toNumber, fromString, reifyPrecision, reflectPrecision, toString, P100, rescale, fromBigInt, floor, ceil, numerator)
66import Data.BigInt as BigInt
77import Data.Maybe (Maybe(..), fromJust, isJust)
88import Effect (Effect)
@@ -10,14 +10,17 @@ import Effect.Console (log)
1010import Math as Math
1111import Partial.Unsafe (unsafePartial)
1212import Test.QuickCheck (arbitrary, quickCheck', (<?>), (===))
13−import Test.QuickCheck.Gen (Gen, chooseInt, suchThat)
13+import Test.QuickCheck.Gen (Gen, chooseInt, uniform, suchThat)
1414
1515-- Just here to package up the safe use of `unsafePartial`.
1616trying :: forall a b. Gen a -> (a -> Maybe b) -> Gen b
1717trying gen f = map (unsafePartial fromJust) (map f gen `suchThat` isJust)
1818
1919genFixedP :: forall p. KnownPrecision p => PProxy p -> Gen (Fixed p)
20−genFixedP _ = map fromNumber arbitrary `trying` identity
20+genFixedP _ = map (fromNumber <<< stretch) uniform `trying` identity
21+ where
22+ -- maps [0,1] to [-10,10]
23+ stretch = (_ * 20.0) >>> (_ - 10.0)
2124
2225fromNumberWith
2326 :: forall precision
@@ -35,15 +38,15 @@ withPrecisionOf
3538 -> Fixed precision
3639withPrecisionOf _ x = x
3740
38−floorToPrecision
41+truncateToPrecision
3942 :: forall precision
4043 . KnownPrecision precision
4144 => Int
4245 -> Fixed precision
4346 -> Fixed precision
44−floorToPrecision p =
47+truncateToPrecision p =
4548 (_ / scaleFactor)
46− <<< floor
49+ <<< (\x -> if x > zero then floor x else ceil x)
4750 <<< (_ * scaleFactor)
4851 where
4952 scaleFactor =
@@ -123,7 +126,8 @@ main = do
123126 precision <- chooseInt 0 100
124127 unsafePartial fromJust $ reifyPrecision precision \p -> do
125128 a <- genFixedP p
126− pure $ fromString (toString a) == Just a <?> show { precision, a }
129+ let b = fromString (toString a)
130+ pure $ b == Just a <?> show { precision, a, b, numeratorA: numerator a, numeratorB: map numerator b }
127131
128132 log ""
129133 log "Conversions to/from Number"
@@ -162,7 +166,7 @@ main = do
162166 let xx = rescale y
163167 pure $ x == xx <?> show { x, xx, y }
164168
165− log "rescaling down then up should be the same as flooring"
169+ log "rescaling down then up should be the same as truncating"
166170 quickCheck' 1000 do
167171 let hi = 20
168172 lo <- chooseInt 10 20
@@ -171,5 +175,5 @@ main = do
171175 reifyPrecision hi \hi_ -> do
172176 x <- genFixedP hi_
173177 let y = withPrecisionOf lo_ $ rescale x
174− let xx = floorToPrecision lo x
178+ let xx = truncateToPrecision lo x
175179 pure $ xx == rescale y <?> show { x, xx, y }