From 77260934a1ad7cbdc2450e1c4cae0ec9d0ee75a2 Mon Sep 17 00:00:00 2001
From: Brendan Zabarauskas <bjzaba@yahoo.com.au>
Date: Wed, 26 Apr 2017 21:02:05 +1000
Subject: [PATCH] Clean up Sum and Product impls

This is for consistency with other impls
---
 src/matrix.rs       | 24 ++++++++++++------------
 src/quaternion.rs   | 24 ++++++++++++------------
 src/rotation.rs     | 24 ++++++++++++------------
 src/structure.rs    |  4 ++--
 src/vector.rs       | 12 ++++++------
 tests/matrix.rs     | 25 ++++++++++++-------------
 tests/quaternion.rs | 15 +++++++++++----
 tests/vector.rs     |  6 ++----
 tests/vector4f32.rs |  2 --
 9 files changed, 69 insertions(+), 67 deletions(-)

diff --git a/src/matrix.rs b/src/matrix.rs
index d2af6f0..bf58784 100644
--- a/src/matrix.rs
+++ b/src/matrix.rs
@@ -657,7 +657,7 @@ impl<S: BaseFloat> SquareMatrix for Matrix4<S> {
                      self[3][3])
     }
 
-    // The new implementation results in negative optimization when used 
+    // The new implementation results in negative optimization when used
     // without SIMD. so we opt them in with configuration.
     // A better option would be using specialization. But currently somewhat
     // specialization is too buggy, and it won't apply here. I'm getting
@@ -968,31 +968,31 @@ macro_rules! impl_matrix {
             fn sub_assign(&mut self, other: $MatrixN<S>) { $(self.$field -= other.$field);+ }
         }
 
-        impl<S: BaseFloat> iter::Sum for $MatrixN<S> {
+        impl<S: BaseFloat> iter::Sum<$MatrixN<S>> for $MatrixN<S> {
             #[inline]
-            fn sum<I: Iterator<Item=Self>>(iter: I) -> Self {
-                iter.fold(Self::zero(), Add::add)
+            fn sum<I: Iterator<Item=$MatrixN<S>>>(iter: I) -> $MatrixN<S> {
+                iter.fold($MatrixN::zero(), Add::add)
             }
         }
 
-        impl<'a, S: 'a + BaseFloat> iter::Sum<&'a Self> for $MatrixN<S> {
+        impl<'a, S: 'a + BaseFloat> iter::Sum<&'a $MatrixN<S>> for $MatrixN<S> {
             #[inline]
-            fn sum<I: Iterator<Item=&'a Self>>(iter: I) -> Self {
-                iter.fold(Self::zero(), Add::add)
+            fn sum<I: Iterator<Item=&'a $MatrixN<S>>>(iter: I) -> $MatrixN<S> {
+                iter.fold($MatrixN::zero(), Add::add)
             }
         }
 
         impl<S: BaseFloat> iter::Product for $MatrixN<S> {
             #[inline]
-            fn product<I: Iterator<Item=Self>>(iter: I) -> Self {
-                iter.fold(Self::identity(), Mul::mul)
+            fn product<I: Iterator<Item=$MatrixN<S>>>(iter: I) -> $MatrixN<S> {
+                iter.fold($MatrixN::identity(), Mul::mul)
             }
         }
 
-        impl<'a, S: 'a + BaseFloat> iter::Product<&'a Self> for $MatrixN<S> {
+        impl<'a, S: 'a + BaseFloat> iter::Product<&'a $MatrixN<S>> for $MatrixN<S> {
             #[inline]
-            fn product<I: Iterator<Item=&'a Self>>(iter: I) -> Self {
-                iter.fold(Self::identity(), Mul::mul)
+            fn product<I: Iterator<Item=&'a $MatrixN<S>>>(iter: I) -> $MatrixN<S> {
+                iter.fold($MatrixN::identity(), Mul::mul)
             }
         }
 
diff --git a/src/quaternion.rs b/src/quaternion.rs
index 92be09e..126baa4 100644
--- a/src/quaternion.rs
+++ b/src/quaternion.rs
@@ -188,31 +188,31 @@ impl<S: BaseFloat> One for Quaternion<S> {
     }
 }
 
-impl<S: BaseFloat> iter::Sum for Quaternion<S> {
+impl<S: BaseFloat> iter::Sum<Quaternion<S>> for Quaternion<S> {
     #[inline]
-    fn sum<I: Iterator<Item=Self>>(iter: I) -> Self {
-        iter.fold(Self::zero(), Add::add)
+    fn sum<I: Iterator<Item=Quaternion<S>>>(iter: I) -> Quaternion<S> {
+        iter.fold(Quaternion::<S>::zero(), Add::add)
     }
 }
 
-impl<'a, S: 'a + BaseFloat> iter::Sum<&'a Self> for Quaternion<S> {
+impl<'a, S: 'a + BaseFloat> iter::Sum<&'a Quaternion<S>> for Quaternion<S> {
     #[inline]
-    fn sum<I: Iterator<Item=&'a Self>>(iter: I) -> Self {
-        iter.fold(Self::zero(), Add::add)
+    fn sum<I: Iterator<Item=&'a Quaternion<S>>>(iter: I) -> Quaternion<S> {
+        iter.fold(Quaternion::<S>::zero(), Add::add)
     }
 }
 
-impl<S: BaseFloat> iter::Product for Quaternion<S> {
+impl<S: BaseFloat> iter::Product<Quaternion<S>> for Quaternion<S> {
     #[inline]
-    fn product<I: Iterator<Item=Self>>(iter: I) -> Self {
-        iter.fold(Self::one(), Mul::mul)
+    fn product<I: Iterator<Item=Quaternion<S>>>(iter: I) -> Quaternion<S> {
+        iter.fold(Quaternion::<S>::one(), Mul::mul)
     }
 }
 
-impl<'a, S: 'a + BaseFloat> iter::Product<&'a Self> for Quaternion<S> {
+impl<'a, S: 'a + BaseFloat> iter::Product<&'a Quaternion<S>> for Quaternion<S> {
     #[inline]
-    fn product<I: Iterator<Item=&'a Self>>(iter: I) -> Self {
-        iter.fold(Self::one(), Mul::mul)
+    fn product<I: Iterator<Item=&'a Quaternion<S>>>(iter: I) -> Quaternion<S> {
+        iter.fold(Quaternion::<S>::one(), Mul::mul)
     }
 }
 
diff --git a/src/rotation.rs b/src/rotation.rs
index 6769634..65f9a13 100644
--- a/src/rotation.rs
+++ b/src/rotation.rs
@@ -159,17 +159,17 @@ impl<S: BaseFloat> From<Basis2<S>> for Matrix2<S> {
     fn from(b: Basis2<S>) -> Matrix2<S> { b.mat }
 }
 
-impl<S: BaseFloat> iter::Product for Basis2<S> {
+impl<S: BaseFloat> iter::Product<Basis2<S>> for Basis2<S> {
     #[inline]
-    fn product<I: Iterator<Item=Self>>(iter: I) -> Self {
-        iter.fold(Basis2 { mat: Matrix2::identity() }, Mul::mul)
+    fn product<I: Iterator<Item=Basis2<S>>>(iter: I) -> Basis2<S> {
+        iter.fold(Basis2::one(), Mul::mul)
     }
 }
 
-impl<'a, S: 'a + BaseFloat> iter::Product<&'a Self> for Basis2<S> {
+impl<'a, S: 'a + BaseFloat> iter::Product<&'a Basis2<S>> for Basis2<S> {
     #[inline]
-    fn product<I: Iterator<Item=&'a Self>>(iter: I) -> Self {
-        iter.fold(Basis2 { mat: Matrix2::identity() }, Mul::mul)
+    fn product<I: Iterator<Item=&'a Basis2<S>>>(iter: I) -> Basis2<S> {
+        iter.fold(Basis2::one(), Mul::mul)
     }
 }
 
@@ -279,17 +279,17 @@ impl<S: BaseFloat> From<Basis3<S>> for Quaternion<S> {
     fn from(b: Basis3<S>) -> Quaternion<S> { b.mat.into() }
 }
 
-impl<S: BaseFloat> iter::Product for Basis3<S> {
+impl<S: BaseFloat> iter::Product<Basis3<S>> for Basis3<S> {
     #[inline]
-    fn product<I: Iterator<Item=Self>>(iter: I) -> Self {
-        iter.fold(Basis3 { mat: Matrix3::identity() }, Mul::mul)
+    fn product<I: Iterator<Item=Basis3<S>>>(iter: I) -> Basis3<S> {
+        iter.fold(Basis3::one(), Mul::mul)
     }
 }
 
-impl<'a, S: 'a + BaseFloat> iter::Product<&'a Self> for Basis3<S> {
+impl<'a, S: 'a + BaseFloat> iter::Product<&'a Basis3<S>> for Basis3<S> {
     #[inline]
-    fn product<I: Iterator<Item=&'a Self>>(iter: I) -> Self {
-        iter.fold(Basis3 { mat: Matrix3::identity() }, Mul::mul)
+    fn product<I: Iterator<Item=&'a Basis3<S>>>(iter: I) -> Basis3<S> {
+        iter.fold(Basis3::one(), Mul::mul)
     }
 }
 
diff --git a/src/structure.rs b/src/structure.rs
index b29a1b3..697df52 100644
--- a/src/structure.rs
+++ b/src/structure.rs
@@ -154,10 +154,10 @@ pub trait ElementWise<Rhs = Self> {
 /// ```
 pub trait VectorSpace: Copy + Clone where
     Self: Zero,
-    Self: iter::Sum<Self>,
 
     Self: Add<Self, Output = Self>,
     Self: Sub<Self, Output = Self>,
+    Self: iter::Sum<Self>,
 
     // FIXME: Ugly type signatures - blocked by rust-lang/rust#24092
     Self: Mul<<Self as VectorSpace>::Scalar, Output = Self>,
@@ -457,7 +457,7 @@ pub trait SquareMatrix where
     Self::Scalar: BaseFloat,
 
     Self: One,
-    Self: iter::Product,
+    Self: iter::Product<Self>,
 
     Self: Matrix<
         // FIXME: Can be cleaned up once equality constraints in where clauses are implemented
diff --git a/src/vector.rs b/src/vector.rs
index df3f7f8..fe58ae5 100644
--- a/src/vector.rs
+++ b/src/vector.rs
@@ -164,17 +164,17 @@ macro_rules! impl_vector {
             }
         }
 
-        impl<S: BaseNum> iter::Sum for $VectorN<S> {
+        impl<S: BaseNum> iter::Sum<$VectorN<S>> for $VectorN<S> {
             #[inline]
-            fn sum<I: Iterator<Item=Self>>(iter: I) -> Self {
-                iter.fold(Self::zero(), Add::add)
+            fn sum<I: Iterator<Item=$VectorN<S>>>(iter: I) -> $VectorN<S> {
+                iter.fold($VectorN::zero(), Add::add)
             }
         }
 
-        impl<'a, S: 'a + BaseNum> iter::Sum<&'a Self> for $VectorN<S> {
+        impl<'a, S: 'a + BaseNum> iter::Sum<&'a $VectorN<S>> for $VectorN<S> {
             #[inline]
-            fn sum<I: Iterator<Item=&'a Self>>(iter: I) -> Self {
-                iter.fold(Self::zero(), Add::add)
+            fn sum<I: Iterator<Item=&'a $VectorN<S>>>(iter: I) -> $VectorN<S> {
+                iter.fold($VectorN::zero(), Add::add)
             }
         }
 
diff --git a/tests/matrix.rs b/tests/matrix.rs
index 43940d3..74fb101 100644
--- a/tests/matrix.rs
+++ b/tests/matrix.rs
@@ -15,7 +15,6 @@
 
 #[macro_use]
 extern crate approx;
-#[macro_use]
 extern crate cgmath;
 
 pub mod matrix2 {
@@ -98,14 +97,14 @@ pub mod matrix2 {
 
     #[test]
     fn test_sum_matrix() {
-        let res: Matrix2<f64> = [A, B, C].iter().sum();
-        assert_eq!(res, A + B + C);
+        assert_eq!(A + B + C, [A, B, C].iter().sum());
+        assert_eq!(A + B + C, [A, B, C].iter().cloned().sum());
     }
 
     #[test]
     fn test_product_matrix() {
-        let res: Matrix2<f64> = [A, B, C].iter().product();
-        assert_eq!(res, A * B * C);
+        assert_eq!(A * B * C, [A, B, C].iter().product());
+        assert_eq!(A * B * C, [A, B, C].iter().cloned().product());
     }
 
     #[test]
@@ -272,14 +271,14 @@ pub mod matrix3 {
 
     #[test]
     fn test_sum_matrix() {
-        let res: Matrix3<f64> = [A, B, C, D].iter().sum();
-        assert_eq!(res, A + B + C + D);
+        assert_eq!(A + B + C + D, [A, B, C, D].iter().sum());
+        assert_eq!(A + B + C + D, [A, B, C, D].iter().cloned().sum());
     }
 
     #[test]
     fn test_product_matrix() {
-        let res: Matrix3<f64> = [A, B, C, D].iter().product();
-        assert_eq!(res, A * B * C * D);
+        assert_eq!(A * B * C * D, [A, B, C, D].iter().product());
+        assert_eq!(A * B * C * D, [A, B, C, D].iter().cloned().product());
     }
 
     #[test]
@@ -641,14 +640,14 @@ pub mod matrix4 {
 
     #[test]
     fn test_sum_matrix() {
-        let res: Matrix4<f64> = [A, B, C, D].iter().sum();
-        assert_eq!(res, A + B + C + D);
+        assert_eq!(A + B + C + D, [A, B, C, D].iter().sum());
+        assert_eq!(A + B + C + D, [A, B, C, D].iter().cloned().sum());
     }
 
     #[test]
     fn test_product_matrix() {
-        let res: Matrix4<f64> = [A, B, C, D].iter().product();
-        assert_eq!(res, A * B * C * D);
+        assert_eq!(A * B * C * D, [A, B, C, D].iter().product());
+        assert_eq!(A * B * C * D, [A, B, C, D].iter().cloned().product());
     }
 
     #[test]
diff --git a/tests/quaternion.rs b/tests/quaternion.rs
index d2fbff5..e27dbbf 100644
--- a/tests/quaternion.rs
+++ b/tests/quaternion.rs
@@ -15,7 +15,6 @@
 
 #[macro_use]
 extern crate approx;
-#[macro_use]
 extern crate cgmath;
 
 macro_rules! impl_test_mul {
@@ -53,14 +52,22 @@ mod operators {
         impl_test_div!(2.0f32, Quaternion::from(Euler { x: Rad(1f32), y: Rad(1f32), z: Rad(1f32) }));
     }
 
+    #[test]
+    fn test_iter_sum() {
+        let q1 = Quaternion::from(Euler { x: Rad(2f32), y: Rad(1f32), z: Rad(1f32) });
+        let q2 = Quaternion::from(Euler { x: Rad(1f32), y: Rad(2f32), z: Rad(1f32) });
+        let q3 = Quaternion::from(Euler { x: Rad(1f32), y: Rad(1f32), z: Rad(2f32) });
+
+        assert_eq!(q1 + q2 + q3, [q1, q2, q3].iter().sum());
+    }
+
     #[test]
     fn test_iter_product() {
         let q1 = Quaternion::from(Euler { x: Rad(2f32), y: Rad(1f32), z: Rad(1f32) });
         let q2 = Quaternion::from(Euler { x: Rad(1f32), y: Rad(2f32), z: Rad(1f32) });
         let q3 = Quaternion::from(Euler { x: Rad(1f32), y: Rad(1f32), z: Rad(2f32) });
 
-        let res: Quaternion<f32> = [q1, q2, q3].iter().product();
-        assert_eq!(res, q1 * q2 * q3);
+        assert_eq!(q1 * q2 * q3, [q1, q2, q3].iter().product());
     }
 }
 
@@ -321,4 +328,4 @@ mod rotate_between_vectors {
 
         assert_ulps_eq!(Quaternion::between_vectors(a, b), expected);
     }
-}
\ No newline at end of file
+}
diff --git a/tests/vector.rs b/tests/vector.rs
index 1bee749..dafcdf7 100644
--- a/tests/vector.rs
+++ b/tests/vector.rs
@@ -15,7 +15,6 @@
 
 #[macro_use]
 extern crate approx;
-#[macro_use]
 extern crate cgmath;
 
 use cgmath::*;
@@ -90,9 +89,8 @@ macro_rules! impl_test_rem {
 
 macro_rules! impl_test_iter_sum {
     ($VectorN:ident { $($field:ident),+ }, $ty:ty, $s:expr, $v:expr) => (
-        let res: $VectorN<$ty> = iter::repeat($v).take($s as usize).sum();
-        assert_eq!(res,
-                   $VectorN::new($($v.$field * $s),+));
+        assert_eq!($VectorN::new($($v.$field * $s),+),
+                   iter::repeat($v).take($s as usize).sum());
     )
 }
 
diff --git a/tests/vector4f32.rs b/tests/vector4f32.rs
index b860d76..94e386b 100644
--- a/tests/vector4f32.rs
+++ b/tests/vector4f32.rs
@@ -15,7 +15,6 @@
 
 #[macro_use]
 extern crate approx;
-#[macro_use]
 extern crate cgmath;
 
 use cgmath::*;
@@ -168,7 +167,6 @@ mod test_magnitude {
             assert_relative_eq!(a.sqrt_element_wide().recip_element_wide(), Vector4::new(1f32, 1f32/2f32, 1f32/3f32, 1f32/4f32), max_relative = 0.005f32);
             assert_relative_eq!(a.rsqrt_element_wide(), Vector4::new(1f32, 1f32/2f32, 1f32/3f32, 1f32/4f32), max_relative= 0.005f32);
         }
-        
     }
 }