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

Return vector instead of iterator #80

Merged
merged 2 commits into from
Mar 18, 2025

Conversation

yancyribbens
Copy link
Collaborator

An earlier implementation called collect() before returning the results. Instead of calling collect(), the API was changed to just return the iterator since it's possible dependant applications may prefer to use the iterator. However, after a refactor, the call to collect() was removed, and now it no longer makes sense to return an iterator. Furthermore, return an opaque type makes for a more complicated return type, making it hard to alias for example. Returning a concrete type therefore simplifies the API.

@yancyribbens yancyribbens force-pushed the return-vector branch 3 times, most recently from a696d2c to 72a8a38 Compare March 17, 2025 19:55
An earlier implementation called collect() before returning the results.
Instead of calling collect(), the API was changed to just return the
iterator since it's possible dependant applications may prefer to use
the iterator.  However, after a refactor, the call to collect() was
removed, and now it no longer makes sense to return an iterator.
Furthermore, return an opaque type makes for a more complicated return
type, making it hard to alias for example.  Returning a concrete type
therefore simplifies the API.
@yancyribbens
Copy link
Collaborator Author

This is a breaking API change, however no changes to algorithm behavior. Tests are not modified either except being strengthened by an iteration count assertion.

@yancyribbens yancyribbens merged commit 495953d into p2pderivatives:master Mar 18, 2025
3 checks passed
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