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

[wip] Initial pass at LineString constructor #58

Closed
wants to merge 5 commits into from

Conversation

urschrei
Copy link
Member

Ref #56
This checks whether the vector being used has one element, and fails if it does.
The error is currently just a String. Is this OK?

@urschrei
Copy link
Member Author

urschrei commented Sep 20, 2016

Trying out from() and into() for constructing LineStrings using common source data structures;

  • Vec<[T; 2]>>
  • <Vec<(T, T)>>
    where T is Float

This works well imo, but where ::new() returns a Result(), and can catch the major stumbling block (1-element sequence), from() and into() can't, and will panic if you do something like
let ls: LineString = vec![[1.0, 2.0]].into()
I'm not sure to what extent this is in conflict with the guidance that the From() and Into() traits must not fail (Is an unrecoverable panic (apart from catch_unwind) a failure?) – TryInto will be stable soon-ish (I think), so we could wait for that.

@frewsxcv
Copy link
Member

I like where this is going. I'm wondering, from an API perspective, it makes sense to make the internal field on the LineString no longer pub. In concrete terms, this would be that construction would be done with LineString::new (which could also be called LineString::from_points) and if they want to access the underlying points, we could provide methods like:

  • as_slice(&self) -> &[Point]
  • into_vec(self) -> Vec<Point>

Though this might be a tangential conversation.

@urschrei
Copy link
Member Author

Oh, I like that idea. Will see how it works out…

@urschrei urschrei force-pushed the linestring_constructor branch 2 times, most recently from b84902f to 7e5d5b4 Compare October 17, 2016 11:14
@frewsxcv
Copy link
Member

Opened a similar PR: #168

@urschrei urschrei closed this Oct 4, 2017
@urschrei urschrei deleted the linestring_constructor branch October 4, 2017 22:49
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

Successfully merging this pull request may close these issues.

2 participants