Skip to content

Commit

Permalink
Merge #611
Browse files Browse the repository at this point in the history
611: macro for delegating Geometry impl to it's inner cases r=michaelkirk a=michaelkirk

- [x] I agree to follow the project's [code of conduct](https://github.com/georust/geo/blob/master/CODE_OF_CONDUCT.md).
- [TODO] I added an entry to `CHANGES.md` if knowledge of this change could be valuable to users.
---

Note there are some cases *not* yet covered by the macro, where the delegation isn't *quite* trivial enough (see CoordsIter). It's possible that the macro could be expanded, but my macro-fu is pretty weak and I'm trying to avoid proc_macro+syn as a dependency unless it's really worthwhile.

This does clean up some code, but the higher comprehension cost might be controversial. I'd be ok with not merging this for now if people aren't excited about it.

As an aside:

There is a crate `enum_dispatch`, which does something similar, and I
actually initially intended to use it, but I steered away for a couple
reasons:

1. enum_dispatch seems intended for the cases where "I have a trait and
   an arbitrary subset of impls. I'd like to group that arbitrary subset of impls
   into a **new enum per trait**"
   Whereas here we're really only targeting one enum: geo-types::Geometry across many traits.
2. I'd like to avoid relying on `syn`. It's a big dependency. Our dev
   dependencies (rayon) and some of our features already use it (postgres),
   but our base feature set does not.
3. enum_dispatch (and thus `syn`) would have to be added to *both* geo
   and geo-types. In particular we want to keep geo-types really light.

There's also a `delegate` crate with a similar macro. It's more sophisticated, but also relies on syn.

I could be wrong about any of the above, but that was my understanding after trying to make it work for a while.



Co-authored-by: Michael Kirk <michael.code@endoftheworl.de>
  • Loading branch information
bors[bot] and michaelkirk committed Feb 5, 2021
2 parents 1deacc7 + 65ffde5 commit e59fb5a
Show file tree
Hide file tree
Showing 7 changed files with 124 additions and 125 deletions.
31 changes: 3 additions & 28 deletions geo/src/algorithm/area.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,34 +236,9 @@ impl<T> Area<T> for Geometry<T>
where
T: CoordFloat,
{
fn signed_area(&self) -> T {
match self {
Geometry::Point(g) => g.signed_area(),
Geometry::Line(g) => g.signed_area(),
Geometry::LineString(g) => g.signed_area(),
Geometry::Polygon(g) => g.signed_area(),
Geometry::MultiPoint(g) => g.signed_area(),
Geometry::MultiLineString(g) => g.signed_area(),
Geometry::MultiPolygon(g) => g.signed_area(),
Geometry::GeometryCollection(g) => g.signed_area(),
Geometry::Rect(g) => g.signed_area(),
Geometry::Triangle(g) => g.signed_area(),
}
}

fn unsigned_area(&self) -> T {
match self {
Geometry::Point(g) => g.unsigned_area(),
Geometry::Line(g) => g.unsigned_area(),
Geometry::LineString(g) => g.unsigned_area(),
Geometry::Polygon(g) => g.unsigned_area(),
Geometry::MultiPoint(g) => g.unsigned_area(),
Geometry::MultiLineString(g) => g.unsigned_area(),
Geometry::MultiPolygon(g) => g.unsigned_area(),
Geometry::GeometryCollection(g) => g.unsigned_area(),
Geometry::Rect(g) => g.unsigned_area(),
Geometry::Triangle(g) => g.unsigned_area(),
}
crate::geometry_delegate_impl! {
fn signed_area(&self) -> T;
fn unsigned_area(&self) -> T;
}
}

Expand Down
15 changes: 2 additions & 13 deletions geo/src/algorithm/bounding_rect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -161,19 +161,8 @@ where
{
type Output = Option<Rect<T>>;

fn bounding_rect(&self) -> Self::Output {
match self {
Geometry::Point(g) => Some(g.bounding_rect()),
Geometry::Line(g) => Some(g.bounding_rect()),
Geometry::LineString(g) => g.bounding_rect(),
Geometry::Polygon(g) => g.bounding_rect(),
Geometry::MultiPoint(g) => g.bounding_rect(),
Geometry::MultiLineString(g) => g.bounding_rect(),
Geometry::MultiPolygon(g) => g.bounding_rect(),
Geometry::GeometryCollection(g) => g.bounding_rect(),
Geometry::Rect(g) => Some(g.bounding_rect()),
Geometry::Triangle(g) => Some(g.bounding_rect()),
}
crate::geometry_delegate_impl! {
fn bounding_rect(&self) -> Self::Output;
}
}

Expand Down
15 changes: 2 additions & 13 deletions geo/src/algorithm/contains/geometry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,8 @@ impl<T> Contains<Coordinate<T>> for Geometry<T>
where
T: GeoNum,
{
fn contains(&self, coord: &Coordinate<T>) -> bool {
match self {
Geometry::Point(g) => g.contains(coord),
Geometry::Line(g) => g.contains(coord),
Geometry::LineString(g) => g.contains(coord),
Geometry::Polygon(g) => g.contains(coord),
Geometry::MultiPoint(g) => g.contains(coord),
Geometry::MultiLineString(g) => g.contains(coord),
Geometry::MultiPolygon(g) => g.contains(coord),
Geometry::GeometryCollection(g) => g.contains(coord),
Geometry::Rect(g) => g.contains(coord),
Geometry::Triangle(g) => g.contains(coord),
}
geometry_delegate_impl! {
fn contains(&self, coord: &Coordinate<T>) -> bool;
}
}

Expand Down
18 changes: 3 additions & 15 deletions geo/src/algorithm/coords_iter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -395,21 +395,9 @@ impl<'a, T: CoordNum + 'a> CoordsIter<'a> for Geometry<T> {
Geometry::Triangle(g) => GeometryCoordsIter::Triangle(g.coords_iter()),
}
}

/// Return the number of coordinates in the `Geometry`.
fn coords_count(&'a self) -> usize {
match self {
Geometry::Point(g) => g.coords_count(),
Geometry::Line(g) => g.coords_count(),
Geometry::LineString(g) => g.coords_count(),
Geometry::Polygon(g) => g.coords_count(),
Geometry::MultiPoint(g) => g.coords_count(),
Geometry::MultiLineString(g) => g.coords_count(),
Geometry::MultiPolygon(g) => g.coords_count(),
Geometry::GeometryCollection(g) => g.coords_count(),
Geometry::Rect(g) => g.coords_count(),
Geometry::Triangle(g) => g.coords_count(),
}
crate::geometry_delegate_impl! {
/// Return the number of coordinates in the `Geometry`.
fn coords_count(&'a self) -> usize;
}

fn exterior_coords_iter(&'a self) -> Self::ExteriorIter {
Expand Down
47 changes: 4 additions & 43 deletions geo/src/algorithm/dimensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,49 +132,10 @@ pub trait HasDimensions {
}

impl<C: CoordNum> HasDimensions for Geometry<C> {
fn is_empty(&self) -> bool {
match self {
Geometry::Point(g) => g.is_empty(),
Geometry::Line(g) => g.is_empty(),
Geometry::LineString(g) => g.is_empty(),
Geometry::Polygon(g) => g.is_empty(),
Geometry::MultiPoint(g) => g.is_empty(),
Geometry::MultiLineString(g) => g.is_empty(),
Geometry::MultiPolygon(g) => g.is_empty(),
Geometry::GeometryCollection(g) => g.is_empty(),
Geometry::Rect(g) => g.is_empty(),
Geometry::Triangle(g) => g.is_empty(),
}
}

fn dimensions(&self) -> Dimensions {
match self {
Geometry::Point(g) => g.dimensions(),
Geometry::Line(g) => g.dimensions(),
Geometry::LineString(g) => g.dimensions(),
Geometry::Polygon(g) => g.dimensions(),
Geometry::MultiPoint(g) => g.dimensions(),
Geometry::MultiLineString(g) => g.dimensions(),
Geometry::MultiPolygon(g) => g.dimensions(),
Geometry::GeometryCollection(g) => g.dimensions(),
Geometry::Rect(g) => g.dimensions(),
Geometry::Triangle(g) => g.dimensions(),
}
}

fn boundary_dimensions(&self) -> Dimensions {
match self {
Geometry::Point(g) => g.boundary_dimensions(),
Geometry::Line(g) => g.boundary_dimensions(),
Geometry::LineString(g) => g.boundary_dimensions(),
Geometry::Polygon(g) => g.boundary_dimensions(),
Geometry::MultiPoint(g) => g.boundary_dimensions(),
Geometry::MultiLineString(g) => g.boundary_dimensions(),
Geometry::MultiPolygon(g) => g.boundary_dimensions(),
Geometry::GeometryCollection(g) => g.boundary_dimensions(),
Geometry::Rect(g) => g.boundary_dimensions(),
Geometry::Triangle(g) => g.boundary_dimensions(),
}
crate::geometry_delegate_impl! {
fn is_empty(&self) -> bool;
fn dimensions(&self) -> Dimensions;
fn boundary_dimensions(&self) -> Dimensions;
}
}

Expand Down
15 changes: 2 additions & 13 deletions geo/src/algorithm/intersects/collections.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,19 +14,8 @@ where
Polygon<T>: Intersects<G>,
MultiPolygon<T>: Intersects<G>,
{
fn intersects(&self, rhs: &G) -> bool {
match self {
Geometry::Point(geom) => geom.intersects(rhs),
Geometry::MultiPoint(geom) => geom.intersects(rhs),
Geometry::Line(geom) => geom.intersects(rhs),
Geometry::LineString(geom) => geom.intersects(rhs),
Geometry::MultiLineString(geom) => geom.intersects(rhs),
Geometry::Triangle(geom) => geom.intersects(rhs),
Geometry::Rect(geom) => geom.intersects(rhs),
Geometry::Polygon(geom) => geom.intersects(rhs),
Geometry::MultiPolygon(geom) => geom.intersects(rhs),
Geometry::GeometryCollection(geom) => geom.intersects(rhs),
}
geometry_delegate_impl! {
fn intersects(&self, rhs: &G) -> bool;
}
}
symmetric_intersects_impl!(Coordinate<T>, Geometry<T>);
Expand Down
108 changes: 108 additions & 0 deletions geo/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,3 +36,111 @@ impl<F: GeoFloat> Closest<F> {
}
}
}

/// Implements the common pattern where a Geometry enum simply delegates its trait impl to it's inner type.
///
/// e.g.
/// ```
/// # use geo::{GeoNum, Polygon, Point, Coordinate, Line, Rect, Triangle, LineString, Geometry, MultiLineString, MultiPoint, MultiPolygon, GeometryCollection};
///
/// trait Foo<T: GeoNum> {
/// fn foo_1(&self, coord: Coordinate<T>) -> bool;
/// fn foo_2(&self) -> i32;
/// }
///
/// // Assuming we have an impl for all the inner types like this:
/// impl<T: GeoNum> Foo<T> for Point<T> {
/// fn foo_1(&self, coord: Coordinate<T>) -> bool { true }
/// fn foo_2(&self) -> i32 { 3 }
/// }
/// impl<T: GeoNum> Foo<T> for LineString<T> {
/// fn foo_1(&self, coord: Coordinate<T>) -> bool { true }
/// fn foo_2(&self) -> i32 { 9 }
/// }
/// impl<T: GeoNum> Foo<T> for Line<T> {
/// fn foo_1(&self, coord: Coordinate<T>) -> bool { true }
/// fn foo_2(&self) -> i32 { 8 }
/// }
/// impl<T: GeoNum> Foo<T> for Rect<T> {
/// fn foo_1(&self, coord: Coordinate<T>) -> bool { true }
/// fn foo_2(&self) -> i32 { 7 }
/// }
/// impl<T: GeoNum> Foo<T> for Triangle<T> {
/// fn foo_1(&self, coord: Coordinate<T>) -> bool { true }
/// fn foo_2(&self) -> i32 { 6 }
/// }
/// impl<T: GeoNum> Foo<T> for Polygon<T> {
/// fn foo_1(&self, coord: Coordinate<T>) -> bool { true }
/// fn foo_2(&self) -> i32 { 5 }
/// }
/// impl<T: GeoNum> Foo<T> for MultiPoint<T> {
/// fn foo_1(&self, coord: Coordinate<T>) -> bool { false }
/// fn foo_2(&self) -> i32 { 4 }
/// }
/// impl<T: GeoNum> Foo<T> for MultiPolygon<T> {
/// fn foo_1(&self, coord: Coordinate<T>) -> bool { false }
/// fn foo_2(&self) -> i32 { 3 }
/// }
/// impl<T: GeoNum> Foo<T> for GeometryCollection<T> {
/// fn foo_1(&self, coord: Coordinate<T>) -> bool { false }
/// fn foo_2(&self) -> i32 { 2 }
/// }
/// impl<T: GeoNum> Foo<T> for MultiLineString<T> {
/// fn foo_1(&self, coord: Coordinate<T>) -> bool { false }
/// fn foo_2(&self) -> i32 { 1 }
/// }
///
/// // If we want the impl for Geometry to simply delegate to it's
/// // inner case...
/// impl<T: GeoNum> Foo<T> for Geometry<T> {
/// // Instead of writing out this trivial enum delegation...
/// // fn foo_1(&self, coord: Coordinate<T>) -> bool {
/// // match self {
/// // Geometry::Point(g) => g.foo_1(coord),
/// // Geometry::LineString(g) => g.foo_1(coord),
/// // _ => todo!("...etc for other cases")
/// // }
/// // }
/// //
/// // fn foo_2(&self) -> i32 {
/// // match self {
/// // Geometry::Point(g) => g.foo_2(),
/// // Geometry::LineString(g) => g.foo_2(),
/// // _ => todo!("...etc for other cases")
/// // }
/// // }
///
/// // we can equivalently write:
/// geo::geometry_delegate_impl! {
/// fn foo_1(&self, coord: Coordinate<T>) -> bool;
/// fn foo_2(&self) -> i32;
/// }
/// }
/// ```
#[macro_export]
macro_rules! geometry_delegate_impl {
(
$(
$(#[$outer:meta])*
fn $func_name: ident(&$($self_life:lifetime)?self $(, $arg_name: ident: $arg_type: ty)*) -> $return: ty;
)+
) => {
$(
$(#[$outer])*
fn $func_name(&$($self_life)? self, $($arg_name: $arg_type),*) -> $return {
match self {
Geometry::Point(g) => g.$func_name($($arg_name),*).into(),
Geometry::Line(g) => g.$func_name($($arg_name),*).into(),
Geometry::LineString(g) => g.$func_name($($arg_name),*).into(),
Geometry::Polygon(g) => g.$func_name($($arg_name),*).into(),
Geometry::MultiPoint(g) => g.$func_name($($arg_name),*).into(),
Geometry::MultiLineString(g) => g.$func_name($($arg_name),*).into(),
Geometry::MultiPolygon(g) => g.$func_name($($arg_name),*).into(),
Geometry::GeometryCollection(g) => g.$func_name($($arg_name),*).into(),
Geometry::Rect(g) => g.$func_name($($arg_name),*).into(),
Geometry::Triangle(g) => g.$func_name($($arg_name),*).into(),
}
}
)+
};
}

0 comments on commit e59fb5a

Please sign in to comment.