Dealing with Validity

[urschrei]

See previous discussions in #56 and #122 First, what do we mean when we talk about validity? The OGC OpenGIS spec discussed in the PostGIS docs gives a good overview.

• It's probably not desirable to have validating constructors by default, as validation can incur a significant performance overhead, but they should probably be available, either as separate constructors, or with a validate=True argument, although the latter could cause a lot of headaches in terms of revising calls to the existing constructors, and dealing with invalid geometries; e.g. both of the simplification algorithms can return invalid geometries, so those traits would always have to return a Result (they should arguably already be doing this)
• Should we check for validity before carrying out certain operations? This is what Shapely does, raising an error if invalid geometries are detected. I'm not sure what JTS / GEOS do.
• I think the two points above directly imply that every geometry operation must return a Result.

[frewsxcv]

Regardless of when we check validity of geometry, we'll need to implement the validity checks before that happens. Unless anyone else has a better suggestion, I think we can use the same validity checks described by OGC, mentioned in the original post above. Opening a new issue for this: https://github.com/georust/rust-geo/issues/127

[Michael-F-Bryan]

I like the concept of "making illegal states unrepresentable". That way you can do things knowing that a particular assumption you make (e.g. all Polygons are closed) will always be valid.

These checks can often introduce O(n) or O(n^2) slow downs though, so what about adding validity checks which are only enabled in debug builds so all issues are found during development without hurting the performance of production code?

For example, you might implement the Polygon::new() function like this:

impl<F: Float> Polygon<F> {
  fn new(exterior: LineString<F>, interior: Vec<LineString<F>>) -> Polygon<F> {
    let poly = Polygon { exterior, interior };
    debug_assert!(poly.is_valid(), "All rings and lines in a polygon must be closed");
    poly
  }
  
  fn is_valid(&self) -> bool {
    exterior.is_closed() && interior.iter().all(|ring| ring.is_closed())
  }
}

Another option would be to add new_checked() constructors which create a new shape and then call is_valid() to make sure it's valid.

---

One question that comes to mind is that if there's a way for someone to create a shape which could break assumptions (e.g. making a polygon which hasn't got a closed exterior), should that constructor be marked unsafe?

[frewsxcv]

For posterity, there was a discussion about validity of Multi geometry types, and we agreed that empty Multi types are considered valid https://github.com/georust/geo/issues/444