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

Add Path.Index helper method #56

Merged
merged 1 commit into from
Dec 2, 2017
Merged

Add Path.Index helper method #56

merged 1 commit into from
Dec 2, 2017

Conversation

dsnet
Copy link
Collaborator

@dsnet dsnet commented Dec 1, 2017

An analysis of user code shows that Path.Last was not sufficient and
that what users really wanted was an easy want to do negative indexing
without needing to constantly check for out-of-bounds.

Thus, add a generic Index method that does not panic on out-of-bounds
accesses and supports negative indexing.

@dsnet dsnet requested a review from neild December 1, 2017 01:32
@dsnet
Copy link
Collaborator Author

dsnet commented Dec 1, 2017

I regret adding the Last method and wished I went with the Index method the first time.

Copy link
Collaborator

@neild neild 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 much more convenient. +1

return pa.Index(-1)
}

// Index returns the ith step in the Path and supports negative indexing.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Pedantic: While anyone familiar with Python knows what negative indexing is, if you want to be precise you should define a negative index.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

An analysis of user code shows that Path.Last was not sufficient and
that what users really wanted was an easy want to do negative indexing
without needing to constantly check for out-of-bounds.

Thus, add a generic Index method that does not panic on out-of-bounds
accesses and supports negative indexing.
@dsnet dsnet merged commit 88141e9 into master Dec 2, 2017
@dsnet dsnet deleted the path-index branch December 2, 2017 07:55
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

Successfully merging this pull request may close these issues.

None yet

2 participants