Simple flood fill method causes StackOverFlow error

Question

My flood fill method:

public void fillNeighbours(int x, int y) {
    for(int i = -1; i < 2; i++) {
        for(int j = -1; j < 2; j++) {
            try {
                visible[x+i][y+j] = true;

                if(num[x+i][y+j] == 0) {
                    fillNeighbours(x+i, y+j);
                }
            } catch (ArrayIndexOutOfBoundsException ignored) {}
        }
    }
}

That catch (ArrayIndexOutOfBoundsException ignored) {} is there for avoiding x and/or y position from going outside of the array. Array size is 30 by 30. I'm making minesweeper like game. So you may know why I need this method and how it should work. If u don't know what minesweeper is then here is quick video about that game: Introduction to minesweeper

You should almost never ignore/swallow exceptions like that. — java-addict301, Oct 20 '17 at 16:24
1. compute your bounds, don't use arrayIndexOutofBoundsExceptions! 2. From your code snippet, we cannot see anything that would terminate your loop. You keep invoking fillNeighbours. Check whether your num[x+i][y+j] is changed to something else than 0. — Joris Kinable, Oct 20 '17 at 16:29

score 1 · Accepted Answer · answered Oct 20 '17 at 17:06

1

The code revisits fields which are set to visible already.

Try something like

if(!visible[x+i][y+j]){
  visible[x+i][y+j] = true;
  if(num[x+i][y+j] == 0) {
    fillNeighbours(x+i, y+j);
  }
}

answered Oct 20 '17 at 17:06

tevemadar

12,389
3
21
49

That or `if(!visible[x+i][y+j])` the OP code seems to check one variable but set another. That said if the area to fill is large the stack could still overflow. – Persixty Oct 20 '17 at 17:12
@Persixty the current data representation needs both checks. If the code would count neighbors on the fly then -1 (let's say) could encode visible=false, and 0...8 could represent visible=true. – tevemadar Oct 20 '17 at 21:45
You may be right. It's not exactly clear in the OP what the model is. – Persixty Oct 22 '17 at 15:28

score 0 · Answer 2 · answered Oct 20 '17 at 16:28

It looks like you're recursively calling fillNeighbours without any break out clause (base case) so the calls fill the stack.

From Wikistack

The tree laws of recursion are

A recursive algorithm must have a base case.
A recursive algorithm must change its state and move toward the base case.
A recursive algorithm must call itself, recursively.

score 0 · Answer 3 · answered Oct 20 '17 at 16:41

Once fillNeighbours finds a cell and calls itself, the next loop will always call another when i and j are equal to zero. So it will never exit, and crash once the stack is full.

Aside from that, it will produce a very deep tree as it is not keeping track of which cells have been recursed, and will call fillNeighbours on the same cell multiple times.

Simple flood fill method causes StackOverFlow error

3 Answers3