what if we combined both of our PRs and unify the coordinate iterators into a single trait like this?

trait CoordsIterator<T: Float> {
    fn iter_coords(&self) -> Box<Iterator<Item = &Coordinate<T>>>;

    fn iter_coords_mut(&mut self) -> Box<Iterator<Item = &mut Coordinate<T>>>;

    fn map_coords<NT: Float>(&self, f: &Fn(&Coordinate<T> -> Coordinate<NT>)) -> Self::Output;

    fn map_coords_inplace(&mut self, f: &Fn(&Coordinate<T> -> Coordinate<T>));
}

does some of this feel redundant? i'm undecided right now. even if they are a little redundant, they all do slightly different things and at this point i'm not too concerned. thoughts?

My main nagging bit with CoordsIter is that it's seems like a non-obvious way to get the same functionality as MapCoords. For (Multi)Point & LineString it makes sense to iterate over the points, but iterating over all the points in a Polygon (or MultiLinestring) without knowing what the Coordinate is (is it exterior? interior? Is this point part of the last linestring?) seems, well, a little silly. I can't see why you'd need it (maybe for bbox calculation 🤔). I can see how you can use CoordsIterMut to get a MapCoordsInplace functionality, but it's not as very clear as "here's a trait for mapping the coordinates". Sorry for being a little harsh.

I'm personally in favour of keeping an in-place & clone versions. I know you can replace the clone with the inplace easily, but I personally just like having an API which makes easy things easy. But it does increase the API and amount of code. You don't want to wide up like PHP with a mishmash of everything. I'm not wedded to the idea though. We should definitely have a way to do it inplace, so there's always an allocation-free option.

I tried to merge MapCoords and MapCoordsInplace as the same trait, but I wanted MapCoords to be able to have a different underlying type (i.e. allow mapping from Coordinate<T> → Coordinate<U>), whereas MapCoordsInplace requires that the type is the same. I couldn't get that to compile though. So I took an easy way out. If anyone knows how to combine them all in one trait, I'm all ears.

I've rebased this. Any thoughts?

For my application at work I used the geo crate for a lot of prototyping before making our own geometry library which fits our domain better (we needed object-safety and built-in curves like splines, arcs, and ellipses). In our geometry library I found a really nice way to get the same functionality as coords_iter() and map_coords_inplace(). The geo crate was incredibly useful for getting up and running fast, so if the following ideas help feel free to use them 😃

The idea is you have a basic ForEach trait which iterates over all the "important" points in an object (spline handles, polyline vertices, etc) invoking some code.

This then lets you build nice things on top of ForEach, like a Translate trait which will translate the object in-place (with default methods that give you a translated clone and so on). The idea being you can almost trivially call for_each_mut() on the object and translate() each vertex in turn.

Our Rotate trait works in a very similar way.

pub trait ForEach {
    fn for_each<F>(&self, func: F)
    where
        F: FnMut(&Vector);
    fn for_each_mut<F>(&mut self, func: F)
    where
        F: FnMut(&mut Vector);
}

pub trait Translate {
    fn translate(&mut self, by: Vector);

    fn translated(&self, by: Vector) -> Self
    where
        Self: Sized + Clone,
    { ... }
    fn move_to(&mut self, new_centre: Vector)
    where
        Self: Centroid,
    { ... }
    fn moved(&self, new_centre: Vector) -> Self
    where
        Self: Centroid + Sized + Clone,
    { ... }
}

pub trait Rotate {
    fn rotate_about(&mut self, angle: f64, point: Vector);

    fn rotate(&mut self, angle: f64)
    where
        Self: Centroid,
    { ... }
    fn rotated(&self, angle: f64) -> Self
    where
        Self: Sized + Clone + Centroid,
    { ... }
}

bors r=frewsxcv

Build succeeded

• continuous-integration/travis-ci/push