Avoid Box in methods returning iterators

georust · bors · May 12, 2018 · May 12, 2018 · May 12, 2018 · a56c52e6da21017554034256456614a3f2893f7a
commit a56c52e6da21017554034256456614a3f2893f7a
diff --git a/CHANGES.md b/CHANGES.md
@@ -6,6 +6,8 @@
   * <https://github.com/georust/rust-geo/pull/209>
 * Use the `proj` crate, rename feature to `use-proj`
   * <https://github.com/georust/rust-geo/pull/214>
+* Return unboxed iterators from `LineString::lines`, `Winding::points_cw`, and `Winding::points_ccw`
+  * <https://github.com/georust/rust-geo/pull/218>
 * Fix compilation errors when using the `proj` feature
   * <https://github.com/georust/rust-geo/commit/0924f3179c95bfffb847562ee91675d7aa8454f5>
 

diff --git a/geo-types/src/lib.rs b/geo-types/src/lib.rs
@@ -17,7 +17,7 @@ use std::ops::Neg;
 use std::ops::Sub;
 
 use num_traits::{Float, Num, NumCast, Signed, ToPrimitive};
-use std::iter::{self, FromIterator, Iterator};
+use std::iter::{FromIterator, Iterator};
 
 #[cfg(feature = "spade")]
 use spade::{BoundingRect, PointN, SpatialObject, TwoDimensional, SpadeNum};
@@ -518,15 +518,12 @@ impl<T: CoordinateType> LineString<T> {
     /// );
     /// assert!(lines.next().is_none());
     /// ```
-    pub fn lines<'a>(&'a self) -> Box<Iterator<Item = Line<T>> + 'a> {
-        if self.0.len() < 2 {
-            return Box::new(iter::empty());
-        }
-        Box::new(self.0.windows(2).map(|w| unsafe {
+    pub fn lines<'a>(&'a self) -> impl Iterator<Item = Line<T>> + 'a {
+        self.0.windows(2).map(|w| unsafe {
             // As long as the LineString has at least two points, we shouldn't
             // need to do bounds checking here.
             Line::new(*w.get_unchecked(0), *w.get_unchecked(1))
-        }))
+        })
     }
 
     pub fn points(&self) -> ::std::slice::Iter<Point<T>> {

diff --git a/geo/src/algorithm/winding_order.rs b/geo/src/algorithm/winding_order.rs
@@ -1,4 +1,6 @@
-use ::{CoordinateType, LineString, Point};
+use std::iter::Rev;
+use std::slice::Iter;
+use {CoordinateType, LineString, Point};
 
 pub(crate) fn twice_signed_ring_area<T>(linestring: &LineString<T>) -> T where T: CoordinateType {
     if linestring.0.is_empty() || linestring.0.len() == 1 {
@@ -12,14 +14,53 @@ pub(crate) fn twice_signed_ring_area<T>(linestring: &LineString<T>) -> T where T
     tmp
 }
 
+enum EitherIter<T, I1, I2>
+where
+    I1: Iterator<Item = T>,
+    I2: Iterator<Item = T>,
+{
+    A(I1),
+    B(I2),
+}
+
+impl<T, I1, I2> Iterator for EitherIter<T, I1, I2>
+where
+    I1: Iterator<Item = T>,
+    I2: Iterator<Item = T>,
+{
+    type Item = T;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        match self {
+            EitherIter::A(iter) => iter.next(),
+            EitherIter::B(iter) => iter.next(),
+        }
+    }
+}
+
+/// Iterates through a list of `Point`s
+pub struct Points<'a, T>(EitherIter<&'a Point<T>, Iter<'a, Point<T>>, Rev<Iter<'a, Point<T>>>>)
+where
+    T: CoordinateType + 'a;
+
+impl<'a, T> Iterator for Points<'a, T>
+where
+    T: CoordinateType,
+{
+    type Item = &'a Point<T>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.0.next()
+    }
+}
+
 /// How a linestring is wound, clockwise or counter-clockwise
 #[derive(PartialEq, Clone, Debug, Eq)]
 pub enum WindingOrder {
     Clockwise,
     CounterClockwise,
 }
 
-
 /// Calculate, and work with, the winding order
 pub trait Winding<T> where T: CoordinateType
 {
@@ -40,13 +81,13 @@ pub trait Winding<T> where T: CoordinateType
     ///
     /// The object isn't changed, and the points are returned either in order, or in reverse
     /// order, so that the resultant order makes it appear clockwise
-    fn points_cw<'a>(&'a self) -> Box<Iterator<Item=&'a Point<T>> + 'a>;
+    fn points_cw<'a>(&'a self) -> Points<'a, T>;
 
     /// Iterate over the points in a counter-clockwise order
     ///
     /// The object isn't changed, and the points are returned either in order, or in reverse
     /// order, so that the resultant order makes it appear counter-clockwise
-    fn points_ccw<'a>(&'a self) -> Box<Iterator<Item=&'a Point<T>> + 'a>;
+    fn points_ccw<'a>(&'a self) -> Points<'a, T>;
 
     /// Change this objects's points so they are in clockwise winding order
     fn make_cw_winding(&mut self);
@@ -96,21 +137,21 @@ impl<T> Winding<T> for LineString<T>
     ///
     /// The Linestring isn't changed, and the points are returned either in order, or in reverse
     /// order, so that the resultant order makes it appear clockwise
-    fn points_cw<'a>(&'a self) -> Box<Iterator<Item=&'a Point<T>> + 'a> {
+    fn points_cw<'a>(&'a self) -> Points<'a, T> {
         match self.winding_order() {
-            Some(WindingOrder::CounterClockwise) => Box::new(self.0.iter().rev()),
-            _ => Box::new(self.0.iter()),
+            Some(WindingOrder::CounterClockwise) => Points(EitherIter::B(self.0.iter().rev())),
+            _ => Points(EitherIter::A(self.0.iter())),
         }
     }
 
     /// Iterate over the points in a counter-clockwise order
     ///
     /// The Linestring isn't changed, and the points are returned either in order, or in reverse
     /// order, so that the resultant order makes it appear counter-clockwise
-    fn points_ccw<'a>(&'a self) -> Box<Iterator<Item=&'a Point<T>> + 'a> {
+    fn points_ccw<'a>(&'a self) -> Points<'a, T> {
         match self.winding_order() {
-            Some(WindingOrder::Clockwise) => Box::new(self.0.iter().rev()),
-            _ => Box::new(self.0.iter()),
+            Some(WindingOrder::Clockwise) => Points(EitherIter::B(self.0.iter().rev())),
+            _ => Points(EitherIter::A(self.0.iter())),
         }
     }