Add flood fill in Rust #745
Conversation
berquist
added
the
Implementation
|
[lang: rust] |
There was a problem hiding this comment.
I've taken the liberty of adding a Cargo.toml file to your project, as we are moving to an automated build system. You could still see if a more recent version of ndarray works, though!
Overall it looks good, I don't really know ndarray and I think it's overkill to use it here.
Maybe you have a good reason to recommend it?
Otherwise just a few changes, clippy had a few warnings that need to be fixed.
| @@ -0,0 +1,151 @@ | |||
| use ndarray::prelude::*; // 0.13.1 | |||
There was a problem hiding this comment.
I don't think you need to pick from prelude directly, and you can remove the comment for the version
| use ndarray::prelude::*; // 0.13.1 | |
| use ndarray::*; |
| queue.push_back(*loc); | ||
|
|
||
| // color the initial location | ||
| color(canvas, &loc, old_val, new_val); |
There was a problem hiding this comment.
There is a clippy warning about needless borrow for *loc
| color(canvas, &loc, old_val, new_val); | |
| color(canvas, loc, old_val, new_val); |
| return; | ||
| } | ||
|
|
||
| color(canvas, &loc, old_val, new_val); |
There was a problem hiding this comment.
There is a clippy warning about needless borrow for *loc
| color(canvas, &loc, old_val, new_val); | |
| color(canvas, loc, old_val, new_val); |
|
|
||
| color(canvas, &loc, old_val, new_val); | ||
|
|
||
| let up_next = find_neighbors(canvas, &loc, old_val); |
There was a problem hiding this comment.
There is a clippy warning about needless borrow for *loc
| let up_next = find_neighbors(canvas, &loc, old_val); | |
| let up_next = find_neighbors(canvas, loc, old_val); |
|
|
||
| fn find_neighbors(canvas: &Array<usize, Ix2>, loc: &[usize; 2], val: usize) -> Vec<[usize; 2]> { | ||
| // find neighbors | ||
| let mut possible_neighbors: Vec<[usize; 2]> = vec![[loc[0] + 1, loc[1]], [loc[0], loc[1] + 1]]; |
There was a problem hiding this comment.
It's not necessary to be explicit about types here
| let mut possible_neighbors: Vec<[usize; 2]> = vec![[loc[0] + 1, loc[1]], [loc[0], loc[1] + 1]]; | |
| let mut possible_neighbors = vec![[loc[0] + 1, loc[1]], [loc[0], loc[1] + 1]]; |
| } | ||
|
|
||
| // set up the stack | ||
| let mut stack: Vec<[usize; 2]> = Vec::new(); |
There was a problem hiding this comment.
I don't think it's necessary to be explicit about the types here.
| let mut stack: Vec<[usize; 2]> = Vec::new(); | |
| let mut stack = Vec::new(); |
|
And you also need to solve conflicts in the |
|
It's very old PR. Are we still going to continue reviewing it ? author doesn't seems to be active on this PR for 2 years. |
|
If there are quick and obvious changes to make, we can split it off into another PR and merge that |
|
I don't particularly have time to go back through this PR and make changes. Feel free to supersede/cherry-pick. |
This implementation makes use of the
ndarraycrate, which is accessible from the Rust Playground.