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

default iterator for geo traits #427

Merged
merged 13 commits into from
Jan 14, 2024
Merged

Conversation

kylebarron
Copy link
Member

@kylebarron kylebarron commented Jan 11, 2024

Change list

  • Provided trait method for iterators
  • Change required method to an unchecked version, so that the iterators can only bounds check once.
  • Require Sized bound on traits
  • Remove Iter associated type

I've been trying to use my incubating version of geo-traits heavily, but I haven't used the iterator methods on them (e.g. MultiPointTrait::points() returning an iterator instead of manually iterating through point(i) for 0..MultiPointTrait::num_points) because I haven't been able to reliably implement iterators on all the different structs due to lifetime issues.

I realized that maybe I could provide a default iterator for a generic "anything that implements this geo trait" and my proof of concept worked!

The changes to allow this are 1) removing the associated type for Iter, instead returning a concrete iterator struct and 2) making the trait require Sized. So my question is: are there any practical drawbacks to these two changes? Is there ever a reason to want to override the iterator type on the associated type of the trait? I've always thought of iterators as just syntactic sugar for for loops, but not sure if that's always the case. And is there ever a use case for implementing these traits on unsized objects. My hunch to both was no, but I thought people might have ideas.

@kylebarron kylebarron marked this pull request as ready for review January 14, 2024 04:03
@kylebarron
Copy link
Member Author

Either vscode or git or github is being very naughty

@kylebarron kylebarron enabled auto-merge (squash) January 14, 2024 06:13
@kylebarron kylebarron merged commit b743f16 into main Jan 14, 2024
@kylebarron kylebarron deleted the kyle/geo-traits-default-iterator branch January 14, 2024 06:18
kylebarron added a commit that referenced this pull request Jan 14, 2024
Simplify code following
#427.

Closes #437
@kylebarron kylebarron mentioned this pull request Feb 26, 2024
2 tasks
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.

1 participant