Make LongRational immutable

src/ImageSharp/Primitives/LongRational.cs

@@ -13,7 +13,7 @@ namespace SixLabors.ImageSharp.Primitives
     /// <remarks>
     /// This is a very simplified implementation of a rational number designed for use with metadata only.
     /// </remarks>
-    internal struct LongRational : IEquatable<LongRational>
+    internal readonly struct LongRational : IEquatable<LongRational>
     {
         /// <summary>
         /// Initializes a new instance of the <see cref="LongRational"/> struct.
@@ -26,126 +26,25 @@ namespace SixLabors.ImageSharp.Primitives
         /// The number below the line in a vulgar fraction; a divisor.
         /// </param>
         public LongRational(long numerator, long denominator)
-          : this(numerator, denominator, false)
-        {
-        }
-
-        /// <summary>
-        /// Initializes a new instance of the <see cref="LongRational"/> struct.
-        /// </summary>
-        /// <param name="numerator">
-        /// The number above the line in a vulgar fraction showing how many of the parts
-        /// indicated by the denominator are taken.
-        /// </param>
-        /// <param name="denominator">
-        /// The number below the line in a vulgar fraction; a divisor.
-        /// </param>
-        /// <param name="simplify">
-        /// Whether to attempt to simplify the fractional parts.
-        /// </param>
-        public LongRational(long numerator, long denominator, bool simplify)
-            : this()
         {
             this.Numerator = numerator;
             this.Denominator = denominator;
-
-            if (simplify)
-            {
-                this.Simplify();
-            }
-        }
-
-        /// <summary>
-        /// Initializes a new instance of the <see cref="LongRational"/> struct.
-        /// </summary>
-        /// <param name="value">The <see cref="double"/> to create the instance from.</param>
-        /// <param name="bestPrecision">Whether to use the best possible precision when parsing the value.</param>
-        public LongRational(double value, bool bestPrecision)
-            : this()
-        {
-            if (double.IsNaN(value))
-            {
-                this.Numerator = this.Denominator = 0;
-                return;
-            }
-
-            if (double.IsPositiveInfinity(value))
-            {
-                this.Numerator = 1;
-                this.Denominator = 0;
-                return;
-            }
-
-            if (double.IsNegativeInfinity(value))
-            {
-                this.Numerator = -1;
-                this.Denominator = 0;
-                return;
-            }
-
-            this.Numerator = 1;
-            this.Denominator = 1;
-
-            double val = Math.Abs(value);
-            double df = this.Numerator / (double)this.Denominator;
-            double epsilon = bestPrecision ? double.Epsilon : .000001;
-
-            while (Math.Abs(df - val) > epsilon)
-            {
-                if (df < val)
-                {
-                    this.Numerator++;
-                }
-                else
-                {
-                    this.Denominator++;
-                    this.Numerator = (int)(val * this.Denominator);
-                }
-
-                df = this.Numerator / (double)this.Denominator;
-            }
-
-            if (value < 0.0)
-            {
-                this.Numerator *= -1;
-            }
-
-            this.Simplify();
         }
 
         /// <summary>
         /// Gets the numerator of a number.
         /// </summary>
-        public long Numerator
+        public long Numerator { get; }
-        {
-            get;
-            private set;
-        }
 
         /// <summary>
         /// Gets the denominator of a number.
         /// </summary>
-        public long Denominator
+        public long Denominator { get; }
-        {
-            get;
-            private set;
-        }
 
         /// <summary>
         /// Gets a value indicating whether this instance is indeterminate.
         /// </summary>
-        public bool IsIndeterminate
+        public bool IsIndeterminate => this.Denominator == 0 && this.Numerator == 0;
-        {
-            get
-            {
-                if (this.Denominator != 0)
-                {
-                    return false;
-                }
-
-                return this.Numerator == 0;
-            }
-        }
 
         /// <summary>
         /// Gets a value indicating whether this instance is an integer (n, 1)
@@ -155,76 +54,28 @@ namespace SixLabors.ImageSharp.Primitives
         /// <summary>
         /// Gets a value indicating whether this instance is equal to negative infinity (-1, 0)
         /// </summary>
-        public bool IsNegativeInfinity
+        public bool IsNegativeInfinity => this.Denominator == 0 && this.Numerator == -1;
-        {
-            get
-            {
-                if (this.Denominator != 0)
-                {
-                    return false;
-                }
-
-                return this.Numerator == -1;
-            }
-        }
 
         /// <summary>
         /// Gets a value indicating whether this instance is equal to positive infinity (1, 0)
         /// </summary>
-        public bool IsPositiveInfinity
+        public bool IsPositiveInfinity => this.Denominator == 0 && this.Numerator == 1;
-        {
-            get
-            {
-                if (this.Denominator != 0)
-                {
-                    return false;
-                }
-
-                return this.Numerator == 1;
-            }
-        }
 
         /// <summary>
         /// Gets a value indicating whether this instance is equal to 0 (0, 1)
         /// </summary>
-        public bool IsZero
+        public bool IsZero => this.Denominator == 1 && this.Numerator == 0;
-        {
-            get
-            {
-                if (this.Denominator != 1)
-                {
-                    return false;
-                }
-
-                return this.Numerator == 0;
-            }
-        }
 
         /// <inheritdoc/>
         public bool Equals(LongRational other)
         {
-            if (this.Denominator == other.Denominator)
+            return this.Numerator == other.Numerator && this.Denominator == other.Denominator;
-            {
-                return this.Numerator == other.Numerator;
-            }
-
-            if (this.Numerator == 0 && this.Denominator == 0)
-            {
-                return other.Numerator == 0 && other.Denominator == 0;
-            }
-
-            if (other.Numerator == 0 && other.Denominator == 0)
-            {
-                return this.Numerator == 0 && this.Denominator == 0;
-            }
-
-            return (this.Numerator * other.Denominator) == (this.Denominator * other.Numerator);
         }
 
         /// <inheritdoc/>
         public override int GetHashCode()
         {
-            return this.GetHashCode(this);
+            return ((this.Numerator * 397) ^ this.Denominator).GetHashCode();
         }
 
         /// <inheritdoc/>
@@ -276,78 +127,100 @@ namespace SixLabors.ImageSharp.Primitives
         }
 
         /// <summary>
-        /// Finds the greatest common divisor of two <see cref="long"/> values.
+        /// Create a new instance of the <see cref="LongRational"/> struct from a double value.
         /// </summary>
-        /// <param name="left">The first value</param>
+        /// <param name="value">The <see cref="double"/> to create the instance from.</param>
-        /// <param name="right">The second value</param>
+        /// <param name="bestPrecision">Whether to use the best possible precision when parsing the value.</param>
-        /// <returns>The <see cref="long"/></returns>
+        public static LongRational FromDouble(double value, bool bestPrecision)
-        private static long GreatestCommonDivisor(long left, long right)
         {
-            return right == 0 ? left : GreatestCommonDivisor(right, left % right);
+            if (double.IsNaN(value))
-        }
+            {
+                return new LongRational(0, 0);
+            }
 
-        /// <summary>
+            if (double.IsPositiveInfinity(value))
-        /// Simplifies the <see cref="LongRational"/>
-        /// </summary>
-        private void Simplify()
-        {
-            if (this.IsIndeterminate)
             {
-                return;
+                return new LongRational(1, 0);
             }
 
-            if (this.IsNegativeInfinity)
+            if (double.IsNegativeInfinity(value))
             {
-                return;
+                return new LongRational(-1, 0);
             }
 
-            if (this.IsPositiveInfinity)
+            long numerator = 1;
+            long denominator = 1;
+
+            double val = Math.Abs(value);
+            double df = numerator / (double)denominator;
+            double epsilon = bestPrecision ? double.Epsilon : .000001;
+
+            while (Math.Abs(df - val) > epsilon)
             {
-                return;
+                if (df < val)
+                {
+                    numerator++;
+                }
+                else
+                {
+                    denominator++;
+                    numerator = (int)(val * denominator);
+                }
+
+                df = numerator / (double)denominator;
             }
 
-            if (this.IsInteger)
+            if (value < 0.0)
             {
-                return;
+                numerator *= -1;
             }
 
-            if (this.IsZero)
+            return new LongRational(numerator, denominator).Simplify();
+        }
+
+        /// <summary>
+        /// Finds the greatest common divisor of two <see cref="long"/> values.
+        /// </summary>
+        /// <param name="left">The first value</param>
+        /// <param name="right">The second value</param>
+        /// <returns>The <see cref="long"/></returns>
+        private static long GreatestCommonDivisor(long left, long right)
+        {
+            return right == 0 ? left : GreatestCommonDivisor(right, left % right);
+        }
+
+        /// <summary>
+        /// Simplifies the <see cref="LongRational"/>
+        /// </summary>
+        public LongRational Simplify()
+        {
+            if (this.IsIndeterminate ||
+                this.IsNegativeInfinity ||
+                this.IsPositiveInfinity ||
+                this.IsInteger ||
+                this.IsZero)
             {
-                return;
+                return this;
             }
 
             if (this.Numerator == 0)
             {
-                this.Denominator = 0;
+                return new LongRational(0, 0);
-                return;
             }
 
             if (this.Numerator == this.Denominator)
             {
-                this.Numerator = 1;
+                return new LongRational(1, 1);
-                this.Denominator = 1;
             }
 
             long gcd = GreatestCommonDivisor(Math.Abs(this.Numerator), Math.Abs(this.Denominator));
+
             if (gcd > 1)
             {
-                this.Numerator = this.Numerator / gcd;
+                return new LongRational(this.Numerator / gcd, this.Denominator / gcd);
-                this.Denominator = this.Denominator / gcd;
             }
-        }
 
-        /// <summary>
+            return this;
-        /// Returns the hash code for this instance.
-        /// </summary>
-        /// <param name="rational">
-        /// The instance of <see cref="LongRational"/> to return the hash code for.
-        /// </param>
-        /// <returns>
-        /// A 32-bit signed integer that is the hash code for this instance.
-        /// </returns>
-        private int GetHashCode(LongRational rational)
-        {
-            return ((rational.Numerator * 397) ^ rational.Denominator).GetHashCode();
         }
     }
 }

src/ImageSharp/Primitives/Rational.cs

@@ -41,10 +41,18 @@ namespace SixLabors.ImageSharp.Primitives
         /// <param name="simplify">Specified if the rational should be simplified.</param>
         public Rational(uint numerator, uint denominator, bool simplify)
         {
-            var rational = new LongRational(numerator, denominator, simplify);
+            if (simplify)
-
+            {
-            this.Numerator = (uint)rational.Numerator;
+                LongRational rational = new LongRational(numerator, denominator).Simplify();
-            this.Denominator = (uint)rational.Denominator;
+
+                this.Numerator = (uint)rational.Numerator;
+                this.Denominator = (uint)rational.Denominator;
+            }
+            else
+            {
+                this.Numerator = numerator;
+                this.Denominator = denominator;
+            }
         }
 
         /// <summary>
@@ -63,7 +71,7 @@ namespace SixLabors.ImageSharp.Primitives
         /// <param name="bestPrecision">Whether to use the best possible precision when parsing the value.</param>
         public Rational(double value, bool bestPrecision)
         {
-            var rational = new LongRational(Math.Abs(value), bestPrecision);
+            var rational = LongRational.FromDouble(Math.Abs(value), bestPrecision);
 
             this.Numerator = (uint)rational.Numerator;
             this.Denominator = (uint)rational.Denominator;

src/ImageSharp/Primitives/SignedRational.cs

@@ -41,10 +41,18 @@ namespace SixLabors.ImageSharp.Primitives
         /// <param name="simplify">Specified if the rational should be simplified.</param>
         public SignedRational(int numerator, int denominator, bool simplify)
         {
-            var rational = new LongRational(numerator, denominator, simplify);
+            if (simplify)
-
+            {
-            this.Numerator = (int)rational.Numerator;
+                LongRational rational = new LongRational(numerator, denominator).Simplify();
-            this.Denominator = (int)rational.Denominator;
+
+                this.Numerator = (int)rational.Numerator;
+                this.Denominator = (int)rational.Denominator;
+            }
+            else
+            {
+                this.Numerator = numerator;
+                this.Denominator = denominator;
+            }
         }
 
         /// <summary>
@@ -63,7 +71,7 @@ namespace SixLabors.ImageSharp.Primitives
         /// <param name="bestPrecision">Whether to use the best possible precision when parsing the value.</param>
         public SignedRational(double value, bool bestPrecision)
         {
-            var rational = new LongRational(value, bestPrecision);
+            var rational = LongRational.FromDouble(value, bestPrecision);
 
             this.Numerator = (int)rational.Numerator;
             this.Denominator = (int)rational.Denominator;