From f89baa75128b9df3777cf7f288d9f1436d7e2b13 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Ols=CC=8Cavsky=CC=81?= Date: Sun, 7 Aug 2022 12:01:43 +0200 Subject: [PATCH 1/2] Simplify before toFloat conversion + test cases --- src/BigRational.php | 3 ++- tests/BigRationalTest.php | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/BigRational.php b/src/BigRational.php index 9f7ccae..4625781 100644 --- a/src/BigRational.php +++ b/src/BigRational.php @@ -429,7 +429,8 @@ public function toInt() : int */ public function toFloat() : float { - return $this->numerator->toFloat() / $this->denominator->toFloat(); + $simplified = $this->simplified(); + return $simplified->numerator->toFloat() / $simplified->denominator->toFloat(); } /** diff --git a/tests/BigRationalTest.php b/tests/BigRationalTest.php index 80985d5..43d654e 100644 --- a/tests/BigRationalTest.php +++ b/tests/BigRationalTest.php @@ -892,6 +892,42 @@ public function providerToIntThrowsException() : array ]; } + public function testIdentityOperationResultsInDifferentToFloatValueWithoutSimplification() : void + { + $expectedValue = 11.46; + $conversionFactor = BigRational::of('0.45359237'); + $value = BigRational::of($expectedValue); + + $identicalValueAfterMathOperations = $value->multipliedBy($conversionFactor) + ->dividedBy($conversionFactor) + ->multipliedBy($conversionFactor) + ->dividedBy($conversionFactor) + ->multipliedBy($conversionFactor) + ->dividedBy($conversionFactor); + + self::assertSame($expectedValue, $identicalValueAfterMathOperations->toFloat()); + + // Assert that simplification is required and the test would fail without it + self::assertNotSame( + $expectedValue, + $identicalValueAfterMathOperations->getNumerator()->toFloat() / $identicalValueAfterMathOperations->getDenominator()->toFloat(), + ); + } + + public function testToFloatConversionPerformsSimplificationToPreventOverflow() : void + { + $val = BigRational::of(1); + $factor = 10000; + + for ($i = 0; $i < 1000; ++$i) { + $val = $val->multipliedBy($factor)->dividedBy($factor); + } + + self::assertInfinite($val->getNumerator()->toFloat()); + // Assert that simplification is required and the test would fail without it + self::assertSame(1.0, $val->toFloat()); + } + /** * @dataProvider providerToFloat * From 403de38843c6762cee231604e757eddb845de845 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Ols=CC=8Cavsky=CC=81?= Date: Wed, 10 Aug 2022 15:41:07 +0200 Subject: [PATCH 2/2] Review fix --- tests/BigRationalTest.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/BigRationalTest.php b/tests/BigRationalTest.php index 43d654e..5ebb0c1 100644 --- a/tests/BigRationalTest.php +++ b/tests/BigRationalTest.php @@ -916,12 +916,8 @@ public function testIdentityOperationResultsInDifferentToFloatValueWithoutSimpli public function testToFloatConversionPerformsSimplificationToPreventOverflow() : void { - $val = BigRational::of(1); - $factor = 10000; - - for ($i = 0; $i < 1000; ++$i) { - $val = $val->multipliedBy($factor)->dividedBy($factor); - } + $int = BigInteger::of('1e4000'); + $val = BigRational::nd($int, $int); self::assertInfinite($val->getNumerator()->toFloat()); // Assert that simplification is required and the test would fail without it