Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[traits] rename CoordTrait::nth_unchecked -> nth_or_panic to match conventional semantics #1242

Merged
merged 2 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions geo-traits/CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

- BREAKING: Mark `CoordTrait::nth_unchecked` as `unsafe` and add `CoordTrait::nth_or_panic`.
- <https://github.com/georust/geo/pull/1242>

## 0.1.1

- Fix `TriangleTrait::second` and `TriangleTrait::third` to return the second and third coordinates instead of the first.
Expand Down
37 changes: 30 additions & 7 deletions geo-traits/src/coord.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,16 @@ pub trait CoordTrait {

/// Access the n'th (0-based) element of the CoordinateTuple.
/// Returns `None` if `n >= DIMENSION`.
/// See also [`nth_unchecked()`](Self::nth_unchecked).
///
/// See also [`nth_or_panic()`](Self::nth_or_panic) and [`nth_unchecked()`](Self::nth_unchecked).
///
/// # Panics
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a panic condition.

///
/// This method may panic if [`dim()`](Self::dim) does not correspond to
/// the actual number of dimensions in this coordinate.
fn nth(&self, n: usize) -> Option<Self::T> {
if n < self.dim().size() {
Some(self.nth_unchecked(n))
Some(self.nth_or_panic(n))
} else {
None
}
Expand All @@ -39,13 +45,30 @@ pub trait CoordTrait {
/// Access the n'th (0-based) element of the CoordinateTuple.
/// May panic if n >= DIMENSION.
/// See also [`nth()`](Self::nth).
fn nth_unchecked(&self, n: usize) -> Self::T;
fn nth_or_panic(&self, n: usize) -> Self::T;

/// Access the n'th (0-based) element of the CoordinateTuple.
/// May panic if n >= DIMENSION.
///
/// See also [`nth()`](Self::nth), [`nth_or_panic()`](Self::nth_or_panic).
///
/// You might want to override the default implementation of this method
/// if you can provide a more efficient implementation.
///
/// # Safety
///
/// Though it may panic, the default implementation actually is safe. However, implementors
/// are allowed to implement this method themselves with an unsafe implementation. See the
/// individual implementations for more information on their own Safety considerations.
unsafe fn nth_unchecked(&self, n: usize) -> Self::T {
self.nth_or_panic(n)
}
}

impl<T: CoordNum> CoordTrait for Coord<T> {
type T = T;

fn nth_unchecked(&self, n: usize) -> Self::T {
fn nth_or_panic(&self, n: usize) -> Self::T {
match n {
0 => self.x(),
1 => self.y(),
Expand All @@ -69,7 +92,7 @@ impl<T: CoordNum> CoordTrait for Coord<T> {
impl<T: CoordNum> CoordTrait for &Coord<T> {
type T = T;

fn nth_unchecked(&self, n: usize) -> Self::T {
fn nth_or_panic(&self, n: usize) -> Self::T {
match n {
0 => self.x(),
1 => self.y(),
Expand All @@ -93,7 +116,7 @@ impl<T: CoordNum> CoordTrait for &Coord<T> {
impl<T: CoordNum> CoordTrait for (T, T) {
type T = T;

fn nth_unchecked(&self, n: usize) -> Self::T {
fn nth_or_panic(&self, n: usize) -> Self::T {
match n {
0 => self.x(),
1 => self.y(),
Expand Down Expand Up @@ -127,7 +150,7 @@ impl<T: CoordNum> CoordTrait for UnimplementedCoord<T> {
unimplemented!()
}

fn nth_unchecked(&self, _n: usize) -> Self::T {
fn nth_or_panic(&self, _n: usize) -> Self::T {
unimplemented!()
}

Expand Down
11 changes: 2 additions & 9 deletions geo-traits/src/point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,9 @@ pub trait PointTrait {
/// Dimensions of the coordinate tuple
fn dim(&self) -> Dimensions;

/// Whether this point is `empty` or not.
/// The location of this 0-dimensional geometry.
///
/// According to Simple Features, a Point can have zero coordinates and be considered `empty`.
///
/// If `is_empty` returns `true`, then the values of `x()`, `y()`, `nth()` and `nth_unchecked`
/// have no semantic meaning.
///
/// Only a top-level geometry can be empty. That is, when this point is contained within
/// another geometry, such as a [`LineStringTrait`][crate::LineStringTrait], those points
/// can never be empty, and a consumer does not need to check this method.
/// According to Simple Features, a Point can have zero coordinates and be considered "empty".
fn coord(&self) -> Option<Self::CoordType<'_>>;
}

Expand Down
Loading