Skip to content

std::str::StrSlice::char_at() should returns Option(char) #12882

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

Closed
liigo opened this issue Mar 14, 2014 · 12 comments
Closed

std::str::StrSlice::char_at() should returns Option(char) #12882

liigo opened this issue Mar 14, 2014 · 12 comments

Comments

@liigo
Copy link
Contributor

liigo commented Mar 14, 2014

Currently, std::str::StrSlice::char_at() will fail if str is empty or index is out of bounds.

@liigo liigo closed this as completed Mar 20, 2014
@liigo liigo reopened this Mar 20, 2014
@liigo
Copy link
Contributor Author

liigo commented Mar 20, 2014

sorry, misoperation

@pongad
Copy link
Contributor

pongad commented Apr 5, 2014

This seems to contradict #12710. Might want to think carefully about the future of strings.

@huonw
Copy link
Member

huonw commented Apr 5, 2014

@pongad what do you mean? I don't see how this contradicts?

@pongad
Copy link
Contributor

pongad commented Apr 5, 2014

@huonw Aren't we trying to remove string indexing? Or did I understand you wrong?

@huonw
Copy link
Member

huonw commented Apr 5, 2014

Only the [] byte-wise string indexing.

@pongad
Copy link
Contributor

pongad commented Apr 5, 2014

Of course my mistake! In that case, should char_at(x) return an enum with error? In case no character begins at x?

@aturon
Copy link
Member

aturon commented Jun 2, 2014

I'm not sure that the API for char_at and friends should change. In general, indexing fails (rather than yielding an option) on out of range errors, and it seems sensible for this method to follow suit.

That said, the documentation should certainly mention the possibility of failure; I'll send in a PR for that momentarily.

@liigo
Copy link
Contributor Author

liigo commented Jun 3, 2014

@aturon @alexcrichton
Fails inside char_at() will force user checking char_count() first, both the operations (char_at and char_count) are O(n). This is bad, too expensive. Unlike vector/slice indexing (which are O(1)), char_at() is not a simply indexing operation. I don't think fails in char_at() is necessary.

@aturon
Copy link
Member

aturon commented Jun 3, 2014

@liigo I don't think that's true -- the indexing for char_at and related functions is based on the byte index, not the character index, so it should be possible to check using the len() method, which is O(1).

@liigo
Copy link
Contributor Author

liigo commented Jun 3, 2014

Oh, sorry. If char_at() is not indexing chars in string, maybe it should be renamed (to char_at_byte_index or something else). char_at here is ambiguous.

@bluss
Copy link
Member

bluss commented Jun 3, 2014

liigo, byte indices are the default for all string slice methods

@aturon
Copy link
Member

aturon commented Jun 3, 2014

@liigo Based on the discussion, I think this issue should be closed.

@aturon aturon closed this as completed Jun 3, 2014
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

5 participants