Fixed bug in the magnitude computation. (thanks to 0jpq0, closes #20)

tompazourek · Jan 15, 2019 · ed83bb8 · ed83bb8
1 parent 69dd519
commit ed83bb8
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 3 deletions.
diff --git a/src/Rationals/BigIntegerUtils.cs b/src/Rationals/BigIntegerUtils.cs
@@ -0,0 +1,24 @@
+using System.Numerics;
+
+namespace Rationals
+{
+    internal static class BigIntegerUtils
+    {
+        /// <summary>
+        /// Returns the number of digits of the given number
+        /// </summary>
+        public static int GetNumberOfDigits(BigInteger x)
+        {
+            x = BigInteger.Abs(x);
+
+            var digits = 0;
+            while (x > 0)
+            {
+                digits++;
+                x /= 10;
+            }
+
+            return digits;
+        }
+    }
+}
diff --git a/src/Rationals/ExtendedProperties.cs b/src/Rationals/ExtendedProperties.cs
@@ -46,9 +46,32 @@ public int Magnitude
                 if (IsZero)
                     return 0;
 
-                var numLog = BigInteger.Log10(BigInteger.Abs(Numerator));
-                var denLog = BigInteger.Log10(BigInteger.Abs(Denominator));
-                return (int)Math.Floor(numLog - denLog);
+                // thanks to 0jpq0 for this magnitude algorithm
+                // https://github.com/tompazourek/Rationals/issues/20#issue-398771661
+
+                var numeratorDigits = BigIntegerUtils.GetNumberOfDigits(Numerator);
+                var denominatorDigits = BigIntegerUtils.GetNumberOfDigits(Denominator);
+
+                var magnitude = numeratorDigits - denominatorDigits;
+
+                var numeratorAbs = BigInteger.Abs(Numerator);
+                var denominatorAbs = BigInteger.Abs(Denominator);
+
+                if (numeratorDigits > denominatorDigits)
+                {
+                    denominatorAbs *= BigInteger.Pow(10, numeratorDigits - denominatorDigits);
+                }
+                else if (numeratorDigits < denominatorDigits)
+                {
+                    numeratorAbs *= BigInteger.Pow(10, denominatorDigits - numeratorDigits);
+                }
+
+                if (numeratorAbs < denominatorAbs)
+                {
+                    magnitude--;
+                }
+
+                return magnitude;
             }
         }
     }

diff --git a/tests/Rationals.Tests/ExtendedPropertiesTests.cs b/tests/Rationals.Tests/ExtendedPropertiesTests.cs
@@ -10,6 +10,8 @@ public class ExtendedPropertiesTests
         [InlineData(2, 1, 0)]
         [InlineData(0, 1, 0)]
         [InlineData(10, 1, 1)]
+        [InlineData(100, 1, 2)]
+        [InlineData(1000, 1, 3)]
         [InlineData(11, 1, 1)]
         [InlineData(11, 2, 0)]
         [InlineData(1, 2, -1)]