patternjavaMinor
Paint fill function
Viewed 0 times
paintfunctionfill
Problem
Implement the "paint fill" function of image editing programs: Given a
screen point and new color fill the surrounding area.
Any comments on the code below?
Time complexity: \$\mathcal{O}(N^2)\$
Space complexity: \$\mathcal{O}(N^2)\$
screen point and new color fill the surrounding area.
Any comments on the code below?
Time complexity: \$\mathcal{O}(N^2)\$
Space complexity: \$\mathcal{O}(N^2)\$
static int N = 6;
static boolean[][] colored = new boolean[N+2][N+2];
static int old_color;
public static void fillP(int[][] array, int x, int y, int color){
System.out.println("Before");
printArray2D(array);
old_color = array[x][y];
fillPaint(array, x, y, color);
System.out.println("After");
printArray2D(array);
}
public static void fillPaint(int[][] array, int x, int y, int color){
if (x N || y N) return;
if (array[x][y] != old_color) return;
array[x][y] = color;
colored[x+1][y+1] = true;
for (int i = -1; i < 2; i += 2){
for (int j = -1; j < 2; j += 2){
if (!colored[x+i+1][y+1]){
fillPaint(array, x+i, y, color);
}
if (!colored[x+1][y+j+1]){
fillPaint(array, x, y+j, color);
}
}
}
}
public static void printArray2D(int[][] array){
int n = array.length;
int m = array[0].length;
for (int i = 0; i < n; i++){
for (int j = 0; j < m; j++){
if (j == m-1){
System.out.print(array[i][j] + " ");
System.out.println();
}
else{
System.out.print(array[i][j] + " ");
}
}
}
}
}Solution
You should avoid using
The names of your variables are a bit confusing. What does
Why is
I'd try to introduce some different data types to better model the problem your're working on. What about using a
Going back to your static variables, I thing you should get rid of them and instead have two separate functions. One to determine what pixels you need to color and another to actually change their colors. Something like:
I'm not sure if you want to use
static variables. They're going to cause you troubles and make your code untestable and very difficult to maintain.The names of your variables are a bit confusing. What does
colored mean? Why do you need to keep track of old_color? At a first glance at the code I cannot tell what they are for.Why is
fillP doing two different things? I would completely get rid of all the printing it does, I don't think it is really useful. Actually I'd get rid of it entirely and use only fillPaint instead.I'd try to introduce some different data types to better model the problem your're working on. What about using a
Canvas class instead of int[][], a Coordinate class instead of a pair of int and a Color class instead of an int? That could help making your program much better structured.Going back to your static variables, I thing you should get rid of them and instead have two separate functions. One to determine what pixels you need to color and another to actually change their colors. Something like:
public void fillPaint(int[][] canvas, int x, int y, int color)
{
for(int i = -1; i < 2; i += 2)
{
for(int j = -1; j < 2; j += 2)
{
setColor(canvas, x + i, y + j, color);
}
}
}I'm not sure if you want to use
colored to obtain a particular shape, but you can surely do it within fillPaint adding some conditions before the execution of setColor and you surely don't need to use a global variable. Deciding the shape of the area you want to color is exactly the responsibility of fillPaint.Code Snippets
public void fillPaint(int[][] canvas, int x, int y, int color)
{
for(int i = -1; i < 2; i += 2)
{
for(int j = -1; j < 2; j += 2)
{
setColor(canvas, x + i, y + j, color);
}
}
}Context
StackExchange Code Review Q#79632, answer score: 4
Revisions (0)
No revisions yet.