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

Refactor index / count normalization in range-like methods #10753

Conversation

HertzDevil
Copy link
Contributor

This refactor captures the index / count normalization in many Indexable methods and some others outside; it is the range-like equivalent of Indexable#check_index_out_of_bounds.

Related to #10243. (Any method that uses normalize_start_and_count or range_to_index_and_count will have the same index behaviour noted there.)

There are at least two methods where this refactor does not apply, because they require exactly count elements and not "count or less" elements:

Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great simplification, thanks!

src/array.cr Outdated Show resolved Hide resolved
src/string.cr Outdated Show resolved Hide resolved
Copy link
Member

@asterite asterite left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! It seems there are merge conflicts.

@straight-shoota straight-shoota added this to the 1.1.0 milestone Jun 5, 2021
@straight-shoota
Copy link
Member

@HertzDevil this needs an update again

@straight-shoota straight-shoota merged commit b1c9a3d into crystal-lang:master Jun 17, 2021
@HertzDevil HertzDevil deleted the refactor/indexable-start-and-count branch June 17, 2021 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants