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

Feature request: Add Index<usize> to Keys #284

Closed
jrose-signal opened this issue Nov 1, 2023 · 1 comment · Fixed by #285
Closed

Feature request: Add Index<usize> to Keys #284

jrose-signal opened this issue Nov 1, 2023 · 1 comment · Fixed by #285

Comments

@jrose-signal
Copy link

#132 added usize-based indexing for the values in an IndexMap, which is absolutely the correct behavior for Index<usize> to have. However, sometimes I want to get either just the key or a key-value pair at a particular index, and get_index is a verbose way to do that if I know the index is valid. A possible alternate spelling would be map.keys()[idx], implemented by providing Index<usize> on Keys.

Of course, this particular approach means that indexing would be valid even after consuming some of the keys, even if that's not the recommended use, and the code needs to decide and document what that means.

@cuviper
Copy link
Member

cuviper commented Nov 1, 2023

It's an interesting idea!

Of course, this particular approach means that indexing would be valid even after consuming some of the keys, even if that's not the recommended use, and the code needs to decide and document what that means.

Since Keys is a wrapper around slice::Iter, I think the only thing we can currently do is index the remaining contents. Maybe we could change that wrapper, but note that Slice::keys() also feeds into this. I think indexing the current state of the iterator makes the most sense.

I feel a little hesitation about slippery-slope precedent here... e.g. does this make sense for IntoKeys too? What about the value iterators? But... maybe it's ok for Keys alone to serve as both an indexable "view" and an iterator.

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 a pull request may close this issue.

2 participants