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

Impl Default for Coordinates #612

Closed
sbland opened this issue Jan 29, 2021 · 5 comments
Closed

Impl Default for Coordinates #612

sbland opened this issue Jan 29, 2021 · 5 comments

Comments

@sbland
Copy link
Contributor

sbland commented Jan 29, 2021

Can we safely implement default for the Coordinate struct as below:

impl<T: Default + CoordNum> Default for Coordinate<T> {
    fn default() -> Coordinate<T> {
        Coordinate {
            x: T::default(),
            y: T::default(),
        }
    }
}
@frewsxcv
Copy link
Member

It is possible to add this implementation. @sbland Would you mind sharing your use case for this?

@sbland
Copy link
Contributor Author

sbland commented Jan 31, 2021

@frewsxcv I have many structs that use the Point struct and can otherwise implement default with #[derive(Default)]. Their initial position is not important and will be set later. See below for example of code that would be simplified by including a default implementation of Point:

// #[derive(Default)
pub struct CellState {
    pub id: u32,
    pub position: Point<f64>,
    pub population: u32,
}

// All this could be removed if Point had a default implementation
impl Default for CellState {
    fn default() -> CellState {
        CellState {
            id: 0,
            position: point!(x:0.0, y:0.0),
            population: 0,
        }
    }
}

I can submit a quick PR if there are no potential issues to this.

@michaelkirk
Copy link
Member

michaelkirk commented Feb 1, 2021

You probably already know, but just in case, to get defaults for the other fields you can write:

CellState {          
    position: point!(x:0.0, y:0.0),
    ..Default::default()
}

I can submit a quick PR if there are no potential issues to this.

There is always potential for issues 😉, but I think relying on T::Default seems pretty reasonable - for 99.9% of people, that means they'll get a point at (0,0).

@sbland
Copy link
Contributor Author

sbland commented Feb 2, 2021

Thanks @michaelkirk I use this for instantiating later but still need to put in position: point!(x:0.0, y:0.0), every time.
I'll submit the PR and see if anyone complains :).

bors bot added a commit that referenced this issue Feb 5, 2021
616: Implement Default for Coordinate and Point r=frewsxcv a=sbland

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

PR for issue #612

Co-authored-by: Sam Bland <sbland.co.uk@gmail.com>
Co-authored-by: Corey Farwell <coreyf@rwell.org>
@frewsxcv
Copy link
Member

frewsxcv commented Feb 6, 2021

Done in #616

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants