views:

222

answers:

2
+2  A: 

You should also outline your algorithm along with the code to give out a better idea of how you are approaching the problem. It's really difficult to understand this code without an accompanying algorithm.

You could outline your algorithm in plain english, doesn't have to be pseudocode. It will help you understand the problem and solution better. For instance, something like this:

  1. look at each mine
  2. increment all it's non-mine neighbors

I think you are over-complicating the way you check for boundary conditions. Let's assume if the minefield was infinitely large in each direction. If we then picked any random cell(i,j), it's 8 neighbours would be at:

cell(i-1, j-1) // top-left
cell(i-1, j)   // top
cell(i-1, j+1) // top-right
cell(i, j-1)   // left
cell(i, j+1)   // right
cell(i+1, j-1) // bottom-left
cell(i+1, j)   // bottom
cell(i+1, j+1) // bottom-right

Back to the actual finite-length case now. I'd suggest that instead of pre-calculating whether a cell is within bounds, you should always check all 8 neighbors, and discard the invalid ones.

A function that checks if a cell is valid is easy to write:

bool is_cell_valid(int x, int y, int rows, int cols) {
    if(x < 0 || x >= cols) {
        return false;
    }
    if(y < 0 || y >= rows) {
        return false;
    }
    return true;
}

I understand you algorithm now finally :)

  1. look at each non-mine cell
  2. change its value to the number of neighboring mines

The double checking is happening because you are not checking for all variables that are changing. For example, in this condition you are using iLEFT and jLEFT to go to the top-left cell. Since both variables are being used, you have to make sure that both of them are non-zero.

if(box[i-iLEFT][j-jLEFT]==-1)
{
    if(i-iLEFT!=i) // trying to avoid double checking
        count+=1;
}

The above should instead be

if(iLEFT != 0 && jLEFT != 0)
{
    if(box[i-iLEFT][j-jLEFT]==-1)
        count+=1;
}

And this check should be applied on each of the 8 cells, not just the diagonal cells.


Here's a tiny alternative to go through all 8 cells that are adjacent to some cell at (x, y) without using an if for each neighbor.

for(int i = -1; i < = 1; i++) {
    for(int j = -1; j <= 1; j++) {
        if(i == 0 && j == 0) {
            continue;
        }
        if(is_cell_valid(x + i, y + j, N, N);
            // cell is adjacent and within boundaries
            // do whatever calculations are needed
        }
    }
}
Anurag
Please see my algorithm
Leo
ok I understand your algorithm now, it's simple.. but the program is very complex.
Anurag
Hi,thank you for help, actualy i was change my algorithm :)For example if my board 4X4 i am used board 6X6.It is help me to avoid boundaries checking, because i am working only with internal board 4X4.
Leo
yeah, its better to make the algorithm flexible to handle any nxn board. that also makes you think about a general approach for solving the problem and the resulting solution is better in most cases.
Anurag
A: 

i second on including the illustration of your algorithm which will be alot easier to read any way i have made a minesweeper game before and this could be the outline of the generate function

  1. we will start with empty grid (full of zeros)
  2. insert each bomb randomly in empty place (doesn't already have a bomb)
  3. increment all the non-bombs neighbors

additional feature delay the generation of the bombs after the first click so you can make sure not to place a bomb on the first cell the user clicks

and here is a code snippet

var i = 0
while (i < bombsNum){
    //generate a random location for the bomb
    var col=Math.floor(Math.random()*cols);
    var row=Math.floor(Math.random()*rows);

    //if new location is already a bomb
    if (cellsData[row][col] == -1) continue;

    //if new location is near the start region    
    if (col == startCol-1 || col == startCol || col == startCol +1){
        if (row == startRow -1 || row == startRow || row == startRow+1)
            continue;
    }

    i++;
    cellsData[row][col] = -1;
    //increment all the neighbors cells and make sure to handle the special cases cells (cells at the corner)
}
Ahmed Kotb