CODEWITHSUNDEEP
error-handlingrustrefactoring
naiveai
naiveai

Reputation: 853

How can I simplify this horribly nested error handling code in Rust?

I'm writing a function to compute a Summed Area Table of a Vec<Vec<isize>> in Rust, without the use of any external crates. I'm trying to learn to do this as idiomatically as possible, but I'm running into some roadblocks with error handling.

Basically all I'm trying to do is this, which is mentioned on the Wikipedia page:

The summed-area table can be computed efficiently in a single pass over the image, as the value in the summed-area table at (x, y) is just:

where i provides values from the grid, and I from the previously computed values of the table. Obviously, if x or y is 0, then some of these won't exist, in which case they are replaced with 0.

However, of note is the fact that if I(x, y - 1) does not exist even if y - 1 exists, then the grid we're working with is in fact non-rectangular, and we want to return an NonRectError in that case.

With all that background, here's the code: I need to protect against overflow errors from the subtractions, and return the NonRectError in the special case:

fn compute_summed_area_table(grid: &Vec<Vec<isize>>) -> Result<Vec<Vec<isize>>, NonRectError> {
    let mut summed_area_table =
        vec![Vec::with_capacity(grid[0].len()); grid.len()];

    for (yi, row) in grid.iter().enumerate() {
        for (xi, &value) in row.iter().enumerate() {
            let (prev_row, prev_column_idx) = (
                yi.checked_sub(1).and_then(|i| summed_area_table.get(i)),
                xi.checked_sub(1)
            );

            let summed_values =
                value +
                // I(x, y - 1)
                match prev_row {
                    None => &0,
                    Some(prev_row_vec) => match prev_row_vec.get(xi) {
                        Some(v) => v,
                        None => return Err(NonRectError { xi, yi })
                    }
                } +
                // I(x - 1, y)
                (prev_column_idx
                     .and_then(|i| summed_area_table[yi].get(i))
                     .unwrap_or(&0)) -
                // I(x - 1, y - 1)
                (prev_row
                     .map_or(&0, |r| {
                         prev_column_idx
                             .and_then(|i| r.get(i))
                             .unwrap_or(&0)
                     }));

            summed_area_table[yi].push(summed_values);
        }
    }

    Ok(summed_area_table)
}

// Omitted the definition of NonRectError here, but it is defined.

This code is clearly the definition of sin itself, but I'm not sure which angle to approach simplifying this from - there are so many gosh darn edge cases!

Are there any built-in methods that'll allow me to escape this nested error-checking stuff? Can I return the NonRectError in some easier way than this?

Upvotes: 1

Views: 457

Answers (1)

Evan
Evan

Reputation: 2440

Here are some things you can try:

  1. Use an array, not nested Vecs. With an array, you can guarantee that all the rows have the same width, and NonRectError can't happen. (But maybe you have good reasons to use nested Vecs, so the rest of my examples use nested Vecs.)

  2. The block where you calculate summed_value is pretty long. I'd break it up like this:

    // I(x, y - 1)
let north = ...;

// I(x - 1, y)
let west = ...;

// I(x - 1, y - 1)
let northwest = ...;

let summed_values = value + north + west - northwest;

  3. Instead of checked subtraction, it's easier to check if xi and yi are nonzero. Also, .ok_or() is a good way to convert None to an error.

    let northwest = match (xi, yi) {
    (0, _) => 0,
    (_, 0) => 0,
    (_, _) => {
        // We know xi and yi are nonzero, so we can subtract without checks
        summed_area_table.get(yi - 1)
            .and_then(|row| row.get(xi - 1))
            .ok_or(NonRectError { xi, yi })?
    }
};

    You could also write that with an if/else chain. They're both idiomatic, it's just a matter of preference. I prefer match because it feels more concise.

    let northwest = if xi == 0 {
    0
} else if yi == 0 {
    0
} else {
    // same as above
};

Upvotes: 5

Related Questions