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

Vec Iterator inconsistencies #10962

Closed
DaGenix opened this issue Dec 14, 2013 · 4 comments
Closed

Vec Iterator inconsistencies #10962

DaGenix opened this issue Dec 14, 2013 · 4 comments

Comments

@DaGenix
Copy link

DaGenix commented Dec 14, 2013

It seems like there are some inconsistencies regarding iterators in vec.rs. Just to pick two examples:

  1. SplitIterator and MutSplitIterator yield an empty slice when used on an empty vector. ChunkIter and MutChunkIter do not.
  2. SplitIterator and MutSplitIterator are named differently than ChunkIter and MutChunkIter.

Whats the correct behavior for vec iterators on an empty slice? Is there a good reason that I'm not thinking of that they behave differently?

Whats the correct naming convention?

@TeXitoi

@TeXitoi
Copy link
Contributor

TeXitoi commented Dec 16, 2013

For the naming conventions, looking at vec.rs, there is much more FooIterator than FooIter. If a cleanup is proposed, I'll be in favor of FooIterator to limit the number of modification in the file.

For the empty element, I think that ChunkIter is correct, because we want to iterate on every element by chunks. Moreover, there is an assert for chunks of size 0 (sure, it can't work).

For SplitIterator the pattern is different. Take the canonical example of such an iterator: iterating on CSV fields. With a line like "a;b;c" you want 3 elements, so with ";;", you want 3 elements, for ";" you want 2, and for the empty string, you want 1.

Conclusion: IMHO, we can harmonize the Iterators' name, but the behavior of ChunkIter and SplitIterator is quite consistent.

@DaGenix If you are convinced, you can close this issue. You can also propose a PR with the homogenization of the Iterators' names.

@DaGenix
Copy link
Author

DaGenix commented Dec 16, 2013

That makes sense to me. Thanks!

@DaGenix DaGenix closed this as completed Dec 16, 2013
@DaGenix
Copy link
Author

DaGenix commented Dec 16, 2013

And i'll open up a PR to convert all of the *Iter to *Iterator.

@DaGenix
Copy link
Author

DaGenix commented Dec 16, 2013

Opened #11001

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

2 participants