Add is_closed for MultiLineString and fix is_closed for empty LineString

geo-types/CHANGES.md

@@ -1,8 +1,11 @@
 # Changes
 
-## 0.6.2
+## Unreleased
 
 * `Rect::new` automatically determines min/max points
+  * <https://github.com/georust/geo/pull/519>
+* Add `MultiLineString::is_closed` method
+  * <https://github.com/georust/geo/pull/523>
 
 ## 0.6.1
 

geo-types/src/line_string.rs

@@ -178,11 +178,13 @@ impl<T: CoordinateType> LineString<T> {
         })
     }
 
-    /// Close the `LineString`. Specifically, if the `LineString` has is at least one coordinate,
-    /// and the value of the first coordinate does not equal the value of the last coordinate, then
-    /// a new coordinate is added to the end with the value of the first coordinate.
+    /// Close the `LineString`. Specifically, if the `LineString` has at least one coordinate, and
+    /// the value of the first coordinate does not equal the value of the last coordinate, then a
+    /// new coordinate is added to the end with the value of the first coordinate.
     pub fn close(&mut self) {
         if !self.is_closed() {
+            // by definition, we treat empty LineString's as closed.
+            debug_assert!(self.0.len() > 0);
             self.0.push(self.0[0]);
         }
     }
@@ -215,6 +217,16 @@ impl<T: CoordinateType> LineString<T> {
     /// let line_string: LineString<f32> = coords.into_iter().collect();
     /// assert!(line_string.is_closed());
     /// ```
+    ///
+    /// Note that we diverge from some libraries (JTS et al), which have a LinearRing type,
+    /// separate from LineString. Those libraries treat an empty LinearRing as closed, by
+    /// definition, while treating an empty LineString as open. Since we don't have a separate
+    /// LinearRing type, and use a LineString in its place, we adopt the JTS LinearRing `is_closed`
+    /// behavior in all places, that is, we consider an empty LineString as closed.
+    ///
+    /// This is expected when used in the context of a Polygon.exterior and elswhere; And there
+    /// seems to be no reason to maintain the separate behavior for LineStrings used in
+    /// non-LinearRing contexts.
     pub fn is_closed(&self) -> bool {
         self.0.first() == self.0.last()
     }

geo-types/src/multi_line_string.rs

@@ -3,7 +3,7 @@ use std::iter::FromIterator;
 
 /// A collection of
 /// [`LineString`s](line_string/struct.LineString.html). Can
-/// be created from a `Vec` of `LineString`s, or from an
+/// be created from a `Vec` of `LineString`s or from an
 /// Iterator which yields `LineString`s. Iterating over this
 /// object yields the component `LineString`s.
 ///
@@ -34,6 +34,33 @@ pub struct MultiLineString<T>(pub Vec<LineString<T>>)
 where
     T: CoordinateType;
 
+impl<T: CoordinateType> MultiLineString<T> {
+    /// True if the MultiLineString is empty or if all of its LineStrings are closed - see
+    /// [`LineString::is_closed`].
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// use geo_types::{MultiLineString, LineString, line_string};
+    ///
+    /// let open_line_string: LineString<f32> = line_string![(x: 0., y: 0.), (x: 5., y: 0.)];
+    /// assert!(!MultiLineString(vec![open_line_string.clone()]).is_closed());
+    ///
+    /// let closed_line_string: LineString<f32> = line_string![(x: 0., y: 0.), (x: 5., y: 0.), (x: 0., y: 0.)];
+    /// assert!(MultiLineString(vec![closed_line_string.clone()]).is_closed());
+    ///
+    /// // MultiLineString is not closed if *any* of it's LineStrings are not closed
+    /// assert!(!MultiLineString(vec![open_line_string, closed_line_string]).is_closed());
+    ///
+    /// // An empty MultiLineString is closed
+    /// assert!(MultiLineString::<f32>(vec![]).is_closed());
+    /// ```
+    pub fn is_closed(&self) -> bool {
+        // Note: Unlike JTS et al, we consider an empty MultiLineString as closed.
+        self.0.iter().all(LineString::is_closed)
+    }
+}
+
 impl<T: CoordinateType, ILS: Into<LineString<T>>> From<ILS> for MultiLineString<T> {
     fn from(ls: ILS) -> Self {
         MultiLineString(vec![ls.into()])

geo/src/algorithm/is_convex.rs

@@ -15,7 +15,7 @@ use crate::{Coordinate, LineString};
 ///
 /// - This definition is closely related to the notion
 /// of [convexity of polygons][convex set]. In particular, a
-/// [`Polygon`] is convex, if and only if its `exterior` is
+/// [`Polygon`](crate::Polygon) is convex, if and only if its `exterior` is
 /// convex, and `interiors` is empty.
 ///
 /// - The [`ConvexHull`] algorithm always returns a strictly

geo/src/utils.rs

@@ -98,7 +98,7 @@ where
     //
     // See: https://en.wikipedia.org/wiki/Point_in_polygon
 
-    assert!(linestring.is_closed());
+    debug_assert!(linestring.is_closed());
 
     // LineString without points
     if linestring.0.is_empty() {