grid icon indicating copy to clipboard operation
grid copied to clipboard

Signed indexed_iter?

Open henryiii opened this issue 1 year ago • 2 comments

I often use signed values for computing locations on the grid, and one annoyance is that indexed_iter() forces usize. It would be nice to have some way to control the type of the values is produces. I think it would be invasive to add <T> to indexed_iter(), though, as Rust would start asking users to add the template param if you have a situation where the result could not be inferred. This is what I tried:

    pub fn indexed_iter<I>(&self) -> impl Iterator<Item = ((I, I), &T)> 
    where
    I: TryFrom<usize> + Div<I> + Rem<I> + Copy,
    I: From<<I as Div>::Output>,
    I: From<<I as Rem>::Output>,
    {
        let cols: I = self.cols.try_into().ok().expect("Type too small to hold cols");
        let rows: I = self.rows.try_into().ok().expect("Type too small to hold rows");
        self.data.iter().enumerate().map(move |(idx, i)| {
            let idx: I = idx.try_into().ok().unwrap();
            let position: (I, I) = match self.order {
                Order::RowMajor => ((idx / cols).into(), (idx % cols).into()),
                Order::ColumnMajor => ((idx % rows).into(), (idx / rows).into()),
            };
            (position, i)
        })
    }

Note that the bounds checking only happens once, rather than every time, as it would for a user converting to the type later. But call sites can't always infer I, requiring a type here sometimes where it wasn't required before. Would this make sense as indexed_iter_t, perhaps?

henryiii avatar Dec 16 '24 17:12 henryiii

Hm... I am not so sure about adding another _t function. My goal with this library was to more or less mimic the features of rust's std vec type. Since as far as I know a vec can also only be accessed with an unsigned type, I don't really think it does make sense to add the featuer for grid. But maybe I am also wrong and std vec supports and inferred type by now?

See failing playground: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&code=fn+swap%28x%3A+i32%2C+y%3A+i32%29+-%3E+%28i32%2C+i32%29+%7B%0A++++return+%28y%2C+x%29%3B%0A%7D%0A%0Afn+main%28%29+%7B%0A++++let+v+%3D+vec%21%5B1%2C2%2C3%5D%3B%0A++++let+idx+%3A+i32+%3D+1%3B%0A++++println%21%28%22%7B%7D%22%2C+v%5Bidx%5D%29%3B%0A%7D%0A

becheran avatar Dec 18 '24 20:12 becheran

For std::vec, you can do vec.iter().enumerate().map(|(i, v)| ...), which is conceptually the same as grid.indexed_iter.map(|ind, v| ...). In the docs for enumerate, it says "if you need a custom type, use zip(0..) instead". So I'm looking for the equivalent to vec.iter().zip(0..).map(|(v, i)| ...), which allows you to either pick a type (0i32.., for example), or will infer the type if possible. I don't see any equivalent currently for grid.

Not particularly happy with indexed_iter_t, but not sure how it could be more elegantly done.

henryiii avatar Dec 18 '24 21:12 henryiii