Minesweeper recursion, stackoverflow

Question

So, I've been working on some code for java for minesweeper. I'm working on trying to get the cells that are empty to reveal the coinciding cells next to them recursively. Here is the function that does this.

The "cell" is a button that I'm using for the cells in the game.

private void showCells(int x, int y) {

    //checks out of bounds
    if (x >= SIZE || y >= SIZE || x <= 0 || y <= 0) {
        return;
    }

    //function to look at each surrounding cell and see if it is a mine, 
    //has nMines as a global variable to keep track of the number of mines
    findMines(x, y);

    //if there are mines around the cell that was selected
    if (nMines > 0) {

        //set the text of that cell to the number of mines around it
        cell[x][y].setText(String.valueOf(nMines));

        //if cell is not already disabled, disable it
        if (!cell[x][y].isDisabled()) {
            cell[x][y].setDisable(true);
        }
    } else {

        //if there are no mines, recursively show the surrounding mines
        showCells(x + 1, y);
        showCells(x - 1, y);
        showCells(x, y + 1);
        showCells(x, y - 1);
    }

    //resets the mine count for the next search
    nMines = 0;
}

I know I have some other issues with the code as far as functionality goes, but I'm trying to figure out this recursive thing. What is happening as I debug, is that when I get to the end of the 'x' bound, it returns, but then immediately jumps into the next recursive call, which takes it to the same 'x' location.

showCells(x + 1, y);
showCells(x - 1, y);

I'm wondering what kind of qualifier I need and where I need to place it to make sure that it doesn't search in the same spot twice. Thanks in advance!

If you are just revealing cells surrounding an empty cell (only 4 of them) why are you using recursion? Currently your code seems to reveal all the cells. As far as a qualifier you could just use the .isDisabled() that you already have. — Bennett Yeo, Apr 28 '17 at 04:39
@BennettYeo I'm wanting for it to happen in a cascade so that, if there are other empty spaces, it will do the same thing for those cells also. — pianoman102, Apr 28 '17 at 04:42
You have to add a check if the cell is already disabled right after checking if x and y are within the bounds and if so return, otherwise you are creating an infinite loop. — maraca, Apr 28 '17 at 04:44
@pianoman102 Recursion is just going to unnecessarily complicate this problem. Recursion is always slower than iteration, especially in a non-tail call optimizing language (like java). We choose recursion when we want to make the code more "readable". I would suggest just doing a `for(cell c : cells)` that is empty and exposing the surrounding squares. You can decide whether you want to "boost" performance by making sure you aren't re-showing squares that are already shown. — Bennett Yeo, Apr 28 '17 at 04:45
Recursion may be slower, but it doesn't matter for small inputs and you almost never have to uncover more than 20 fields... — maraca, Apr 28 '17 at 04:47
@maraca I'm with you there. Thanks! I think my other issue is where to set it to disabled if it is empty also. Any thoughts on that? — pianoman102, Apr 28 '17 at 04:47
@pianoman102 you can always disable the cell right after checking if it wasn't disabled yet and remove the other disables. — maraca, Apr 28 '17 at 04:50

MyStackRunnethOver · Accepted Answer · 2017-04-28T04:50:20.233

You're creating an infinite loop, since each cell will recur to every neighboring cell, and then each of those that is empty will recur back to the original cell, etc.

You can fix this by adding a condition to your first if statement:

if (x >= SIZE || y >= SIZE || x <= 0 || y <= 0 || cell[x][y].isDisabled()) {
    return;
}

Because of a handy feature called short-circuiting, the check of isDisabled() won't even throw an error if x or y is out of bounds, because it will never be evaluated.

EDIT: to answer your follow-up about where to put the setDisabled(true) - you always want to disable a cell after it's been clicked on, right? So put it right under findMines(), before the if statement.

Great! I think I am missing a spot to be able to set it to disabled if it is also empty. Should that go in that last else statement to be able to meet this qualifier? — pianoman102, Apr 28 '17 at 04:49

Minesweeper recursion, stackoverflow

1 Answers1