georust / geo

Geospatial primitives and algorithms for Rust
https://crates.io/crates/geo
Other
1.49k stars 194 forks source link

Make `Polygon` fields public. #1065

Closed gibbz00 closed 10 months ago

gibbz00 commented 10 months ago

Polygon::{exterior,interiors}_mut() are for example insufficient when using closures that return a Result.

I do realize that this might cause issues with polygons that aren't closed if instantiation is created without the proper new constructor, so there's probably a better way to go about this. I simply chose the method that resulted in the least amount of changes possible.

Example:

/// Old
impl Transform for Polygon {
    fn transform_coordinates<F: TransformClosure>(&mut self, f: &mut F) -> Result<()> {
        let (mut exterior, mut interiors) = self.clone().into_inner();
        exterior.transform_coordinates(f)?;
        interiors
            .iter_mut()
            .try_for_each(|line_string| line_string.transform_coordinates(f))?;
        *self = Polygon::new(exterior, interiors);
        Ok(())
    }
}

/// New
impl Transform for Polygon {
    fn transform_coordinates<F: TransformClosure>(&mut self, f: &mut F) -> Result<()> {
        self.exterior.transform_coordinates(f)?;
        self.interiors
            .iter_mut()
            .try_for_each(|interior| interior.transform_coordinates(f))
    }
}

frewsxcv commented 10 months ago

Right now Polygon is guaranteed to be close, and by publicizing the fields we lose that guarantee. I'd rather explore other options to increase ergonomics and keep our guarantee before resorting to this.

gibbz00 commented 10 months ago

Right now Polygon is guaranteed to be close, and by publicizing the fields we lose that guarantee. I'd rather explore other options to increase ergonomics and keep our guarantee before resorting to this.

Gotcha. Got an alternative solution to work. Sending in a new PR soon.