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

len shorthand #5

Open
giraffekey opened this issue Dec 4, 2020 · 2 comments
Open

len shorthand #5

giraffekey opened this issue Dec 4, 2020 · 2 comments
Milestone

Comments

@giraffekey
Copy link

Should be able to exclude len() for example:

string.substring_len(3)

Could be a shorthand for string.substring(3, string.len())

@Anders429
Copy link
Owner

As I mentioned in #6, I hesitate to extend the API for methods that are merely shorthand. However, in this case I am definitely open to the possibility of adding a fn substring_from_start(index: usize) -> &str and fn substring_to_end(index: usize) -> &str to perform s.substring(0, index) and s.substring(index, s.len()) respectively. My main interest is whether these might be more optimal in certain cases. Specifically, I suspect that something like s.substring(1, s.len()) might be slightly slower than s.substring_to_end(1), do to avoiding iterating through the char_indices all the way to the end. For sufficiently large s, that may have a significant speed increase.

I'm not sure exactly what the magnitude of such an increase would be, though. I intend to set up some benchmarks to experiment. Whenever I get around to that, I can evaluate whether such an extension to the API is worth it.

@Anders429
Copy link
Owner

Quick update on this: support for this feature is on its way through the pipeline, and will be live in version 2.0.0. It isn't implemented exactly as discussed here, but I think the way it is implemented is actually far easier to use, since it just relies on an index value implementing RangeBounds<usize>, which gives all kinds of freedom that wouldn't be available otherwise.

See #9 for more details on the change, and #11 for the actual PR containing the change.

I'll close this issue when it is merged into master.

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