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
– Bennett Yeo, Commented Apr 28, 2017 at 4: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
– pianoman102, Commented Apr 28, 2017 at 4: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
– maraca, Commented Apr 28, 2017 at 4: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
– Bennett Yeo, Commented Apr 28, 2017 at 4: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
– maraca, Commented Apr 28, 2017 at 4:47

MyStackRunnethOver · Accepted Answer · 2017-04-28 04:50:20Z

1

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.

edited Apr 28, 2017 at 4:50

answered Apr 28, 2017 at 4:47

MyStackRunnethOver

5,3652 gold badges34 silver badges46 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

pianoman102 Over a year ago

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?

Collectives™ on Stack Overflow

Minesweeper recursion, stackoverflow

1 Answer 1

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related