1

I am creating a minesweeper like application and have some code that aims to check around a non mined space how many mines are around it. I have created something to this effect

int count = 0;
                if(model.get(i-1, j-1) == MinerGridCo.UNTURNED_MINE){ count++;}
                if(model.get(i, j-1) == MinerGridCo.UNTURNED_MINE){ count++;}
                if(model.get(i-1, j) == MinerGridCo.UNTURNED_MINE){ count++;}
                if(model.get(i+1, j) == MinerGridCo.UNTURNED_MINE){ count++;}
                if(model.get(i, j+1) == MinerGridCo.UNTURNED_MINE){ count++;}
                if(model.get(i-1, j+1) == MinerGridCo.UNTURNED_MINE){ count++;}
                if(model.get(i+1, j-1) == MinerGridCo.UNTURNED_MINE){ count++;}
                if(model.get(i+1, j+1) == MinerGridCo.UNTURNED_MINE){ count++;}
                String mineNum = String.valueOf(count);
                cell[i][j].setText(mineNum);

however, this produces errors when aim to get the mine number around the edges of the board. Any useful methods to avoid this?

After trying suggestions below. I am still getting the out of bounds errors. Anyone have any advice, here is the repo if anyone wants to compile it themselves https://github.com/phillolivercomp/MineSweeper.git

5
  • What data type is model? Commented Aug 29, 2014 at 17:26
  • 7
    Ignoring exceptions is always a bad idea. Write your code to handle edge (literally in this case) conditions instead. For example, write a method with the signature List<Cell> getNeighbors(Cell cell) that, given a cell, returns a list of valid neighbors to be examined. Then examine only those neighbors. Commented Aug 29, 2014 at 17:26
  • Well the model is a model element of a GUI and that is just an instance of a class "MinerGridCo" which has these UNTURNED_MINE elements which are just integers which are specifically declared. Commented Aug 29, 2014 at 17:27
  • You could always use try/catch, but it's dangerous if you don't understand what you're doing. (Try to imagine all life as you know it stopping instantaneously and every molecule in your body exploding at the speed of light. ) Commented Aug 29, 2014 at 17:36
  • By the way, have you thought of using loops? for (int x=-1; x <= 1; x++) { for (int y=-1; y <= 1; y++) { if( model.get( i+x, j+y )... You get the idea. Commented Aug 29, 2014 at 17:39

6 Answers 6

2

Do yourself a favor and create model.countUnturnedMinesAround(i,j). Then inside model you have some options.

You can create extra rows/columns that view does not see so that you can sfely handle off-by-one indexes.

Or you can write a function private Cell getCell(i,j) that would return default empty cell when i and j are out of bound.

Or you can have boolean hasUnturnedMine(i,j) that returns false when i or j are off the grid.

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

Comments

2

You can avoid duplication with loops:

for (int iOffset = -1; iOffset <= 1; iOffset++) {
    for (int jOffset = -1; jOffset <= 1; jOffset++) {
        if (iOffset != 0 || jOffset != 0) {
            if (isInGrid(i + iOffset, j + jOffset) && 
                model.get(i + iOffset, j + jOffset) == MinerGridCo.UNTURNED_MINE) {
                    count++;
            }
        }
    }
}

using the isInGrid function suggested by @Emrakul's answer (I'd actually combine the range check and the UNTURNED_MINE check into one hasUnturnedMine function, as mentioned in @Arkadiy's answer).

3 Comments

I implemented this and the value seems to reach an out of bounds error when it gets to 12 for some reason which is really confusing me. 12 is the maximum height and width that I have specified for the board.
I have added my github repo to the title as I cannot make any sense of why the code still is throwing these exceptions
@PhillOliver I noticed that @Emrakul's answer says if (i > WIDTH || j > HEIGHT) .. that should probably be if (i >= WIDTH || j >= HEIGHT). P.S. haven't looked at your repo yet.
1

Use short circuiting, and write a function isInGrid(int x, int y) that checks if a position is in the grid.

public boolean isInGrid(int x, int y) {
    //Check if a position is valid in the grid
    if(i < 0 || j < 0) return false;
    if(i >= WIDTH || j >= HEIGHT) return false;
    return true;
}

...

if(isInGrid(i-1, j-1) && model.get(i-1, j-1) == MinerGridCo.UNTURNED_MINE) count++;
if(isInGrid(i+1, j+1) && model.get(i+1, j+1) == MinerGridCo.UNTURNED_MINE) count++;
//Repeat for each location you want to check

If the first condition, isInGrid(i-1, j-1) isn't true, then the condition will exit without running the [condition] statement. In this way, you prevent [condition] from executing if it would otherwise fail.

This is called "short circuiting" - it's an optimization that causes if condition checks to exit if it's already known that the code won't run. You can place one of these isInGrid checks before each statement you want to evaluate, and if the location isn't in the grid, it won't execute get.

4 Comments

Only problem is some of the if statements I have included need to be called. If the object I am looking to check is in the top left it has to check the object to the right and below and diagonally below and right wheras this is different if the object is in the bottom right.
@Phill You can create as many of these statements as you'd like. Place the isInGrid() condition before each statement, and the get won't run if the location isn't in the grid.
Sorry I see what you mean. Good idea. Will implement this. Just as a side note your code in the above example has the variable names different between the ones you are passing to it and the ones you are using.
Shouldn't that be if(i >= WIDTH || j >= HEIGHT)? I assume the valid ranges should be 0 through WIDTH-1 and 0 through HEIGHT-1 inclusive.
0

Create a one line function:

boolean checkBounds(int i, int j) { //TODO }

Then check that before doing your model lookup

3 Comments

I thought of this but the only problem is I have to do a checkbounds for every individual case. For instance, the bottom right case needs to be within the 2D array and will need to be less than the maximum height and width for the count to be valid and the top left needs to be more than 0
i'm not sure what the problem is? checking bounds is not limited to covering only a single corner case.
@PhillOliver yes, you need to check every case
0

An alternative approach could be to have a method isBorder(int i, int j) that lets you know if you're on the bounds. If it is not a border, you know you can safely check all squares around (i,j). If it is on the border, you can calculate whether it's a left/right/top/bottom border or some combination of the 4 (no more than 2) and then check the appropriate boxes.

disclaimer: I am assuming you're already doing a check to make sure (i,j) is a valid location on the grid. You should be doing a check for this regardless of your implementation

Comments

0

There are better ways to do this, via structuring your code a little better. However Here's the answer to your specific question:

The way you ignore an exception in Java is with a try-catch block. This allows you to attempt to run a block of code, catch any exceptions which get thrown, and then decide what to do about them (including ignore them).

Assuming that your out of bounds exception is coming from the model.get() calls you could ignore them like this:

int count = 0;
try
{
    if(model.get(i-1, j-1) == MinerGridCo.UNTURNED_MINE){ count++;}
    if(model.get(i, j-1) == MinerGridCo.UNTURNED_MINE){ count++;}
    if(model.get(i-1, j) == MinerGridCo.UNTURNED_MINE){ count++;}
    if(model.get(i+1, j) == MinerGridCo.UNTURNED_MINE){ count++;}
    if(model.get(i, j+1) == MinerGridCo.UNTURNED_MINE){ count++;}
    if(model.get(i-1, j+1) == MinerGridCo.UNTURNED_MINE){ count++;}
    if(model.get(i+1, j-1) == MinerGridCo.UNTURNED_MINE){ count++;}
    if(model.get(i+1, j+1) == MinerGridCo.UNTURNED_MINE){ count++;}
}
catch (IndexOutOfBoundsException ex)
{
    // Do nothing
}
String mineNum = String.valueOf(count);
cell[i][j].setText(mineNum);

However, when an exception is encountered all subsequent code in the try block will be skipped. And this is definitely not what you want because you'll miss checking some adjacencies. Instead you will need to perform a try-catch for each call to model.get(). This is best implemented by creating a boolean function like:

// Change method visibility and signature accordingly to match your code
public static boolean tileHasMine(int x, int y)
{
    try
    {
        if (model.get(x, y) == MinerGridCo.UNTURNED_MINE)
        {
            return true;
        }
    }
    catch (IndexOutOfBoundsException ex)
    {
        // Do Nothing
    }
    return false;
}

Then your checks would look like:

int count = 0;
if(tileHasMine(i-1, j-1)){ count++;}
if(tileHasMine(i, j-1)){ count++;}
if(tileHasMine(i-1, j)){ count++;}
if(tileHasMine(i+1, j)){ count++;}
if(tileHasMine(i, j+1)){ count++;}
if(tileHasMine(i-1, j+1)){ count++;}
if(tileHasMine(i+1, j-1)){ count++;}
if(tileHasMine(i+1, j+1)){ count++;}
String mineNum = String.valueOf(count);
cell[i][j].setText(mineNum);

Alternatively, you could write a boolean function that first checks if the block is out of the bounds of the model prior to even trying to read from it, which may be a better approach. But I think this is what you were asking.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.