Improve `get` and `get_mut` method signature, Add From implementations for Vec<Vec<T>> and (Vec<T>, usize) #41

pcbowers · 2023-12-13T16:59:29Z

While using this library, I found it difficult to get neighbors because of the possibility of overflows when going negative. For instance, to get the neighbor that is up and to the left of the current cell, I would have to something like this:

let row: usize = 0;
let col: usize = 0;
if let Some(row) = row.checked_sub(1) {
    if let Some(col) = col.checked_sub(1) {
        if let Some(cell) = grid.get(row, col) {
            dbg!(cell);
        }
    }
}

This was a lot of code. Ideally, I could just do this:

let row: isize = 0;
let col: isize = 0;
if let Some(cell) = grid.get(row - 1, col - 1) {
    dbg!(cell);
}

Unfortunately, .get and .get_mut do not accept anything but usize, so this would cause an overflow warning.

I started adding this to my rust projects:

trait GetChecked<T> {
    fn get_checked(&self, row: isize, col: isize) -> Option<&T>;
    fn get_checked_mut(&mut self, row: isize, col: isize) -> Option<&mut T>;
}

impl<T> GetChecked<T> for Grid<T> {
    fn get_checked(&self, row: isize, col: isize) -> Option<&T> {
        self.get(row.try_into().ok()?, col.try_into().ok()?)
    }

    fn get_checked_mut(&mut self, row: isize, col: isize) -> Option<&mut T> {
        self.get_mut(row.try_into().ok()?, col.try_into().ok()?)
    }
}

This trait and implementation allowed me to add new methods called get_checked and get_checked_mut to do what I wanted: pass in an isize or any other thing that could be converted into a number. If it was less than 0, it would return None, which is perfect: there wouldn't be a value there anyway!

let row: isize = 1;
let col: isize = 1;
if let Some(cell) = grid.get_checked(row - 1, col - 1) {
    dbg!(cell);
}

After implementing this a couple times, I figured that it was useful enough to contribute to this library, hence this PR. (I also cleaned up some of the clippy linter warnings I was getting! And added implentation for From which makes more sense than adding a from_vec method, making it possible to potentially remove that function one day)

… that guarantees conversion into a usize works This way get_unchecked and get_unchecked_mut are a little more versatile, but still won't randomly panic on you.

…tems from a 2d and (1d, size) types

becheran · 2023-12-16T10:39:56Z

Looks good to me and very useful. Had some mergers before this one which now causes merge conflicts. Could you please rebase to main? Then I will merge this as well.

Einliterflasche · 2023-12-16T11:12:50Z

I don't know whether this is intentional but I want to note that having a single generic type for both arguments (e.g. fo foo<U: impl Into<usize>(col: U, row: U)) prevents calling the function with two different types even if both implement Into<usize>. This could be allowed if we were to use seperate generic types, like this: fn get_checked(col: impl Into<usize>, row: impl Into<usize>)

pcbowers · 2023-12-16T18:23:46Z

I don't know whether this is intentional but I want to note that having a single generic type for both arguments (e.g. fo foo<U: impl Into<usize>(col: U, row: U)) prevents calling the function with two different types even if both implement Into<usize>. This could be allowed if we were to use seperate generic types, like this: fn get_checked(col: impl Into<usize>, row: impl Into<usize>)

Good call! That was not intentional. I'll implement your suggestion when I rebase!

…rics instead of one

pcbowers · 2023-12-16T18:39:30Z

Looks good to me and very useful. Had some mergers before this one which now causes merge conflicts. Could you please rebase to main? Then I will merge this as well.

@becheran, Should be ready to go! I realized I implemented a merge commit instead of a rebase (force of habit, sorry!). I can undo those commits if you feel strongly about rebasing.

pcbowers added 4 commits December 13, 2023 11:22

feat: ✨ Improve get and get_mut signatures

7fa00e4

chore: 🚨 Fix clippy lint warnings

3c4c7e2

feat: ✨ Improved signature for get_unchecked_mut and get_unchecked so…

db87ad8

… that guarantees conversion into a usize works This way get_unchecked and get_unchecked_mut are a little more versatile, but still won't randomly panic on you.

feat: ✨ Add implementations for From to improve ability to create i…

cc60b7a

…tems from a 2d and (1d, size) types

pcbowers changed the title ~~Allow All values that implement TryInto<usize> as parameters to get and get_mut~~ Improve get and get_mut method signature, Add From implementations for Vec<Vec<T>> and (Vec<T>, usize) Dec 13, 2023

pcbowers and others added 2 commits December 16, 2023 13:27

Merge branch 'master' into master

990167a

feat: ✨ Add support for different types in get methods via two gene…

0abdd03

…rics instead of one

style: 🎨 Run cargo format on code

2b72fb7

becheran approved these changes Dec 18, 2023

View reviewed changes

becheran merged commit 8c07d46 into becheran:master Dec 18, 2023
3 checks passed

henryiii mentioned this pull request Dec 19, 2023

refactor: remove CheckedGet henryiii/aoc2023#5

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve `get` and `get_mut` method signature, Add From implementations for Vec<Vec<T>> and (Vec<T>, usize) #41

Improve `get` and `get_mut` method signature, Add From implementations for Vec<Vec<T>> and (Vec<T>, usize) #41

pcbowers commented Dec 13, 2023 •

edited

Loading

becheran commented Dec 16, 2023

Einliterflasche commented Dec 16, 2023 •

edited

Loading

pcbowers commented Dec 16, 2023

pcbowers commented Dec 16, 2023

Improve get and get_mut method signature, Add From implementations for Vec<Vec<T>> and (Vec<T>, usize) #41

Improve get and get_mut method signature, Add From implementations for Vec<Vec<T>> and (Vec<T>, usize) #41

Conversation

pcbowers commented Dec 13, 2023 • edited Loading

becheran commented Dec 16, 2023

Einliterflasche commented Dec 16, 2023 • edited Loading

pcbowers commented Dec 16, 2023

pcbowers commented Dec 16, 2023

Improve `get` and `get_mut` method signature, Add From implementations for Vec<Vec<T>> and (Vec<T>, usize) #41

Improve `get` and `get_mut` method signature, Add From implementations for Vec<Vec<T>> and (Vec<T>, usize) #41

pcbowers commented Dec 13, 2023 •

edited

Loading

Einliterflasche commented Dec 16, 2023 •

edited

Loading