You should avoid using static
variablesstatic
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
.