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

DoubleEndedIterator and ExactSizeIterator for To*case. #38968

Closed
wants to merge 1 commit into from

Conversation

clarfonthey
Copy link
Contributor

@clarfonthey clarfonthey commented Jan 10, 2017

Because the iterator itself is exact, I see no reason to not implement these traits. Sort of iffy in hindsight about DoubleEndedIterator but I've offered both to see what people think.

@rust-highfive
Copy link
Collaborator

r? @brson

(rust_highfive has picked a reviewer for you, use r? to override)

@clarfonthey clarfonthey changed the title DoubleEndedIterator and ExactSizeIterator for DoubleEndedIterator and ExactSizeIterator for To*case. DoubleEndedIterator and ExactSizeIterator for To*case. Jan 10, 2017
@clarfonthey clarfonthey force-pushed the to_case_extra branch 9 times, most recently from 4d1f73d to 4c6a6fc Compare January 13, 2017 19:23
@alexcrichton alexcrichton added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jan 19, 2017
@clarfonthey
Copy link
Contributor Author

Ping @brson

@alexcrichton
Copy link
Member

I'm not sure that this is a guarantee we want to provide perhaps? Is there a motivation for this beyond just completeness?

I could imagine future implementations, for example, that lazily compute future characters as opposed to buffering them all up in memory immediately as soon as the iterator is constructed. Implementing next_back for such an iterator may be much more difficult.

@clarfonthey
Copy link
Contributor Author

I will agree that shortly after I made this I thought that implementing DEI was a bit unnecessary, although I kept it in anyway. I'll remove that.

ExactSizeIterator seems reasonable, though.

@alexcrichton
Copy link
Member

Even that, though, do we get much benefit? It seems like it may preclude "interesting" future implementations maybe

@clarfonthey
Copy link
Contributor Author

Initially my thought was that it could be used to help figure out more exact allocations for str::to_*case, but in hindsight I'm not sure how useful it would be.

I guess that in principle, it makes sense to know how many characters are required for a lowercase form, but I'm also not sure if that's actually a strong desire.

@alexcrichton
Copy link
Member

@brson may have more comments, but I think I personally have yet to be convinced (but certainly could be!)

@alexcrichton
Copy link
Member

Ok we discussed this during libs triage today but the general sentiment was along the lines of what I've been saying previously which is that these are niche enough API use cases and without a concrete use case in mind we'd prefer to avoid restricting ourselves.

If you've got a use case in mind for these though feel free to resubmit!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants