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

Suggestion: map an iterator of indexes to their values in a slice #743

Open
NightEule5 opened this issue Sep 3, 2023 · 5 comments
Open

Comments

@NightEule5
Copy link

I've run into a pattern that I'm surprised isn't an iterator method or extension already (or if it is, I haven't found it):

let slice = b"Hello world!";
let range = 2..9;
for byte in range.map(|i| &slice[i]) {
    // ...
}

An map_index extension could be added for this, like:

struct MapIndex<'a, I, T> {
    iter: I,
    source: &'a T
}

impl<'a, I: Iterator, T: Index<I::Item>> Iterator for MapIndex<'a, I, T>
where T::Output: Sized + 'a {
    type Item = &'a T::Output;

    fn next(&mut self) -> Option<&'a T::Output> {
        self.iter.next().map(|index|
            &self.source[index]
        )
    }
}

trait Itertools: Iterator {
    fn map_index<T: Index<Self::Item>>(self, source: &T) -> MapIndex<Self, T>
    where T::Output: Sized,
          Self: Sized {
        MapIndex { iter: self, source }
    }
}
@scottmcm
Copy link
Contributor

scottmcm commented Sep 3, 2023

This seems entirely fine, though?

.map(|i| &slice[i])

Generally anything that's "this particular map" needs to be incredibly common to be worth adding. (See #726 (comment) for a recent example of something not being accepted.)

@phimuemue
Copy link
Member

Hi there, I guess that would be covered by #447.

@Philippe-Cholet
Copy link
Member

Philippe-Cholet commented Sep 3, 2023

What's wrong with slice[range]?

let slice = b"Hello world!";
let range = 2..9;
for byte in &slice[range] {
    // ...
}

And range.map(|i| &slice[i]).collect() and slice[range].iter().collect() are equal.

EDIT: I guess when range is not a range but a more complex iterator of indexes?

@NightEule5
Copy link
Author

EDIT: I guess when range is not a range but a more complex iterator of indexes?

Yep, that was the intention. That was a bad example on my part. For an iterator like, say, [0, 2, 3, 1].into_iter(), or when indexing a type which doesn't support slicing, like VecDeque, you'd need map(|i| &values[i]). The title should've been "values in an Index impl" rather than "slice" specifically.

Generally anything that's "this particular map" needs to be incredibly common to be worth adding. (See #726 (comment) for a recent example of something not being accepted.)

Totally fair. I usually use slices for this too, but yesterday I was iterating over a range of VecDeque values (as mentioned above):

let buf: VecDeque<T> = ...;
let range: Range<usize> = ...;

for value in range.map(|i| &buf[i]) {
    // ...
}

This doesn't come up a lot for me either, I understand it not being common enough for this crate

@Philippe-Cholet
Copy link
Member

Philippe-Cholet commented Sep 4, 2023

It makes me think of Itertools::combinations (&co, that I'm currently editing) where a similar thing is done internally self.indices.iter().map(|i| self.pool[*i].clone()).collect().
I think it might be common enough. (The documentation should probably suggest to use slice[range] whenever possible.)

indices.map(|i| &buf[i])
indices.map_index(&buf)

map_index version would be nicer to type I guess (I don't like typing closures), but maybe map version is short enough. I think it's clearer than the proposed alternative (as it requires to know one more method). (Plus we are gonna prefix all methods with it_ soon leading to indices.it_map_index(&buf)). And I suppose it would not change/improve performance.

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

4 participants