Much improvement of addition and subtraction

src/num.rs

@@ -90,6 +90,14 @@ pub trait Int:
     /// Is this number less than zero?
     fn is_neg(self) -> bool;
 
+    /// Returns a tuple of the subtraction along with a boolean indicating whether an arithmetic
+    /// overflow would occur. If an overflow would have occurred then the wrapped value is returned.
+    fn left_overflowing_sub(self, rhs: Self) -> (Self, bool);
+
+    /// Returns a tuple of the multiplication along with a boolean indicating whether an arithmetic
+    /// overflow would occur. If an overflow would have occurred then the wrapped value is returned.
+    fn left_overflowing_mul(self, rhs: Self) -> (Self, bool);
+
     /// Convert to an unsigned number
     ///
     /// A meaningless operation when implemented on an
@@ -167,6 +175,28 @@ macro_rules! impl_int {
                     // between the same bit size
                     <$un_type>::try_from(self).unwrap()
                 }
+
+                fn left_overflowing_mul(self, rhs: Self) -> (Self, bool) {
+                    let (res, overflow) = <$type>::overflowing_mul(self, rhs);
+                    let res = if overflow {
+                        <$type>::max_value() - res + 1
+                    } else {
+                        res
+                    };
+
+                    (res, overflow)
+                }
+
+                fn left_overflowing_sub(self, rhs: Self) -> (Self, bool) {
+                    let (res, overflow) = <$type>::overflowing_sub(self, rhs);
+                    let res = if overflow {
+                        <$type>::max_value() - res + 1
+                    } else {
+                        res
+                    };
+
+                    (res, overflow)
+                }
             }
         )*
     }

src/rational.rs

@@ -1,6 +1,7 @@
 //! # Rational Numbers (fractions)
 
 use crate::num::*;
+use crate::num::Sign::*;
 use std::cmp::{Ord, Ordering, PartialOrd};
 use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Neg, Sub, SubAssign};
 
@@ -41,6 +42,7 @@ macro_rules! frac {
 
 #[derive(Debug, Copy, Clone, PartialEq)]
 enum FracOp {
+    Addition,
     Subtraction,
     Other,
 }
@@ -50,18 +52,18 @@ impl<T: Unsigned> Frac<T> {
     ///
     /// Generally, you will probably prefer to use the [frac!](../macro.frac.html) macro
     /// instead, as that is easier for mixed fractions and whole numbers
-    pub fn new<N: Int + Int<Un = T>>(n: N, d: N) -> Frac<T> {
+    pub fn new<N: Int<Un = T>>(n: N, d: N) -> Frac<T> {
         Self::new_unreduced(n, d).reduce()
     }
 
     /// Create a new rational number from signed or unsigned arguments
     /// where the resulting fraction is not reduced
-    pub fn new_unreduced<N: Int + Int<Un = T>>(n: N, d: N) -> Frac<T> {
+    pub fn new_unreduced<N: Int<Un = T>>(n: N, d: N) -> Frac<T> {
         if d.is_zero() {
             panic!("Fraction can not have a zero denominator");
         }
 
-        let mut sign = Sign::Positive;
+        let mut sign = Positive;
 
         if n.is_neg() {
             sign = !sign;
@@ -71,6 +73,7 @@ impl<T: Unsigned> Frac<T> {
             sign = !sign;
         }
 
+        // Convert the possibly signed arguments to unsigned values
         let numer = n.to_unsigned();
         let denom = d.to_unsigned();
 
@@ -81,7 +84,7 @@ impl<T: Unsigned> Frac<T> {
         }
     }
 
-    /// Create a new rational from the raw parts
+    /// Create a new rational from all the raw parts
     fn raw(n: T, d: T, s: Sign) -> Frac<T> {
         if d.is_zero() {
             panic!("Fraction can not have a zero denominator");
@@ -96,14 +99,22 @@ impl<T: Unsigned> Frac<T> {
 
     /// Determine the output sign given the two input signs
     fn get_sign(a: Self, b: Self, c: FracOp) -> Sign {
-        if a.sign != b.sign {
-            if c == FracOp::Subtraction && b.sign == Sign::Negative {
-                Sign::Positive
+        if c == FracOp::Addition {
+            return if a.sign == Positive && b.sign == Positive {
+                Positive
             } else {
-                Sign::Negative
+                Negative
+            };
+        }
+
+        if a.sign != b.sign {
+            if c == FracOp::Subtraction && b.sign == Negative {
+                Positive
+            } else {
+                Negative
             }
         } else {
-            Sign::Positive
+            Positive
         }
     }
 
@@ -132,7 +143,7 @@ where
 {
     fn cmp(&self, other: &Self) -> Ordering {
         if self.sign != other.sign {
-            return if self.sign == Sign::Positive {
+            return if self.sign == Positive {
                 Ordering::Greater
             } else {
                 Ordering::Less
@@ -147,7 +158,6 @@ where
         let mut b = other.reduce();
 
         if a.denom == b.denom {
-            assert!(false, "{:#?}\n{:#?}", a, b);
             return a.numer.cmp(&b.numer);
         }
 
@@ -231,12 +241,10 @@ where
 
         // If the sign of one input differs,
         // subtraction is equivalent
-        if self.sign != rhs.sign {
-            if a > b {
-                return a - b;
-            } else if a < b {
-                return b - a;
-            }
+        if a.sign == Negative && b.sign == Positive {
+            return b - -a;
+        } else if a.sign == Positive && b.sign == Negative {
+            return a - -b;
         }
 
         // Find a common denominator if needed
@@ -245,14 +253,14 @@ where
             // worrying about reducing to the least common denominator
             let numer = (a.numer * b.denom) + (b.numer * a.denom);
             let denom = a.denom * b.denom;
-            let sign = Self::get_sign(a, b, FracOp::Other);
+            let sign = Self::get_sign(a, b, FracOp::Addition);
 
             return Self::raw(numer, denom, sign);
         }
 
         let numer = a.numer + b.numer;
         let denom = self.denom;
-        let sign = Self::get_sign(a, b, FracOp::Other);
+        let sign = Self::get_sign(a, b, FracOp::Addition);
 
         Self::raw(numer, denom, sign)
     }
@@ -277,19 +285,31 @@ where
         let a = self;
         let b = rhs;
 
-        // @TODO handle sign "overflow" conditions
+        if a.sign == Positive && b.sign == Negative {
+            return a + -b;
+        } else if a.sign == Negative && b.sign == Positive {
+            return b + -a;
+        }
 
         if a.denom != b.denom {
-            let numer = (a.numer * b.denom) - (b.numer * a.denom);
+            let (numer, overflowed) = (a.numer * b.denom).left_overflowing_sub(b.numer * a.denom);
+
             let denom = a.denom * b.denom;
-            let sign = Self::get_sign(a, b, FracOp::Subtraction);
+            let mut sign = Self::get_sign(a, b, FracOp::Subtraction);
+            if overflowed {
+                sign = !sign
+            }
 
             return Self::raw(numer, denom, sign);
         }
 
-        let numer = a.numer - b.numer;
+        let (numer, overflowed) = a.numer.left_overflowing_sub(b.numer);
+
         let denom = a.denom;
-        let sign = Self::get_sign(a, b, FracOp::Subtraction);
+        let mut sign = Self::get_sign(a, b, FracOp::Subtraction);
+        if overflowed {
+            sign = !sign
+        }
 
         Self::raw(numer, denom, sign)
     }
@@ -308,7 +328,7 @@ impl<T: Unsigned> Neg for Frac<T> {
     type Output = Self;
 
     fn neg(self) -> Self::Output {
-        let mut out = self.clone();
+        let mut out = self;
         out.sign = !self.sign;
 
         out
@@ -331,16 +351,15 @@ mod tests {
 
     #[test]
     fn add_test() {
-        assert_eq!(frac!(5 / 6), frac!(1 / 3) + frac!(1 / 2));
+        assert_eq!(frac!(5 / 6), frac!(1 / 3) + frac!(1 / 2), "1/3 + 1/2");
         assert_eq!(frac!(1 / 3), frac!(2 / 3) + -frac!(1 / 3), "2/3 + -1/3");
-        // assert_eq!(-frac!(1 / 3), -frac!(2 / 3) + frac!(1 / 3), "-2/3 + 1/3");
     }
 
     #[test]
     fn sub_test() {
-        assert_eq!(frac!(1 / 6), frac!(1 / 2) - frac!(1 / 3));
-        // assert_eq!(frac!(1), frac!(1 / 3) - -frac!(2 / 3), "1/3 - -2/3");
-        // assert_eq!(-frac!(1), -frac!(2 / 3) - frac!(1 / 3), "-2/3 - 1/3");
+        assert_eq!(4u8.left_overflowing_sub(2).0, 2u8);
+        assert_eq!(0u8.left_overflowing_sub(2).0, 2u8);
+        assert_eq!(frac!(1 / 6), frac!(1 / 2) - frac!(1 / 3), "1/2 - 1/3");
     }
 
     #[test]
@@ -351,6 +370,14 @@ mod tests {
         assert_eq!(frac!(1 / 2), frac!(3 / 6));
     }
 
+    #[test]
+    fn negative_fractions() {
+        assert_eq!(-frac!(1 / 3), -frac!(2 / 3) + frac!(1 / 3), "-2/3 + 1/3");
+        assert_eq!(frac!(1), frac!(1 / 3) - -frac!(2 / 3), "1/3 - -2/3");
+        // assert_eq!(-frac!(1), -frac!(2 / 3) - frac!(1 / 3), "-2/3 - +1/3");
+        assert_eq!(-frac!(1), -frac!(2 / 3) + -frac!(1/3), "-2/3 + -1/3");
+    }
+
     #[test]
     fn macro_test() {
         let frac1 = frac!(1 / 3);