Flood-clearing routine for a JavaFx Minesweeper

Question

I really hate copy and pasting my code from one method to another. It signals something in my brain, "This could be done either more efficiently or more readable". This is used in a minesweeper program I am implementing with a JavaFx GUI (that's not very relevant, but it might help to know). Hopefully this is enough context!

@Contract(pure = true)
private int setMinX(int x){ return (x == 0 ? 0:(x-1)); }
@Contract(pure = true)
private int setMinY(int y){ return (y == 0 ? 0:(y-1)); }
@Contract(pure = true)
private int setMaxX(int x){ return (x == this.width-1 ? this.width-1 : x+1); }
@Contract(pure = true)
private int setMaxY(int y){ return (y == this.height-1 ? this.height-1 : y+1);}

private void adjacents(int x, int y){
    //calculate adjacents for the just placed mine
    int minX = setMinX(x);
    int minY = setMinY(y);
    int maxX = setMaxX(x);
    int maxY = setMaxY(y);

    if (this.mineField[minX][minY] != -1){ this.mineField[minX][minY] += 1; } //left top corner
    if (this.mineField[maxX][minY] != -1){ this.mineField[maxX][minY] += 1; } //right top corner
    if (this.mineField[maxX][maxY] != -1){ this.mineField[maxX][maxY] += 1; } // bottom right corner
    if (this.mineField[minX][maxY] != -1){ this.mineField[minX][maxY] += 1; } // bottom left corner

    if(x > 0 && x < this.width-1){
        if (this.mineField[x][minY] != -1){ this.mineField[x][minY] += 1; } // top-middle
        if (this.mineField[x][maxY] != -1){ this.mineField[x][maxY] += 1; } //bottom-middle
    }

    if(y > 0 && y < this.height-1){
        if (this.mineField[minX][y] != -1){ this.mineField[minX][y] += 1; } // left-middle
        if (this.mineField[maxX][y] != -1){ this.mineField[maxX][y] += 1; } //right-middle
    }
}

public void floodClear(int x, int y) {

    int minX = setMinX(x);
    int minY = setMinY(y);
    int maxX = setMaxX(x);
    int maxY = setMaxY(y);

    if (this.mineField[x][y] >= 0 && !this.liveGame[x][y]) {
        //IMPLEMENT WITH GUI
        //show(x,y);
        if (this.mineField[minX][minY] == 0) {
            floodClear(minX, minY);
        }
        if (this.mineField[maxX][minY] == 0) {
            floodClear(maxX, minY);
        }
        if (this.mineField[maxX][maxY] == 0) {
            floodClear(maxX, maxY);
        }
        if (this.mineField[minX][maxY] == 0) {
            floodClear(minX, maxY);
        }

        if (x > 0 && x < this.width - 1) {
            if (this.mineField[x][minY] == 0) {
                floodClear(x, minY);
            } // top-middle
            if (this.mineField[x][maxY] == 0) {
                floodClear(x, maxY);
            } //bottom-middle
        }

        if (y > 0 && y < this.height - 1) {
            if (this.mineField[minX][y] == 0) {
                floodClear(minX, y);
            } // left-middle
            if (this.mineField[maxX][y] == 0) {
                floodClear(maxX, y);
            }  //right-middle
        }
    }
}

Answer 1

Iterating through adjacents

There are two main approaches to take while iterating through the neighbors in Minesweeper.

The first and most common option is to use a nested for loop:

for (int y = -1; y <= 1; y++) {
    for (int x = -1; x <= 1; x++) {
        if (x == 0 && y == 0) {
            continue; // We don't want to consider the field itself as a neighbor
        }
        increaseAdjacentMineCounter(mineX + x, mineY + y);
    }
}

The second approach, and the one that I normally use and like better, is to use an array to define which fields should be used as your neighbors.

int[][] neighbors = {{ -1, -1 }, { -1, 0 }, { -1, 1 }, { 0, -1 },
    { 0, 1 }, { 1, -1 }, { 1, 0 }, { 1, 1 }};
for (int[] neighbor : neighbors) {
    int x = neighbor[0];
    int y = neighbor[1];
    increaseAdjacentMineCounter(mineX + x, mineY + y);
}

The reason for why I like the second approach better is because it becomes more data-oriented. You can change the way you consider neighbors without changing your code. Changing the way your neighbors are considered can be a bit entertaining but very confusing :)

Answer 2

this.mineField[minX][minY] != -1

This could be a method hasMineAt.

You could rewrite the entire method as a list of "increase adjacent mine counter":

private void increaseAdjacentMineCounter(int locationX, int locationY)
{
    if(withinBounds(locationX, locationY) && !hasMineAt(locationX, locationY)){
        this.mineField[locationX][locationY] += 1;
    }
}

Then your method could consist of 8 calls to increaseAdjacentMineCounter:

increaseAdjacentMineCounter(mineX - 1, mineY - 1);
increaseAdjacentMineCounter(mineX - 1, mineY);
increaseAdjacentMineCounter(mineX - 1, mineY + 1);

increaseAdjacentMineCounter(mineX, mineY - 1);

increaseAdjacentMineCounter(mineX, mineY + 1);

increaseAdjacentMineCounter(mineX + 1, mineY - 1);
increaseAdjacentMineCounter(mineX + 1, mineY);
increaseAdjacentMineCounter(mineX + 1, mineY + 1);

Maybe there's a way to make iterating that list a bit easier. I don't know.