"Minesweeper" in C, trouble on counting surrounding mines are near a certain spot in the board

Question

I have to recreate a simple version of the game "Minefield" in C, but I'm having trouble on counting how many mines are near a certain spot in the board. My piece of code runs throughout the array (board) searching for mines. When it finds a mine, it should add up +1 in all the surrounding spots of the mine. My code only works to the surrounding spots of T[0][0]. m and n stand for the number of lines and columns on the board. Please, help!!

void mine_counting (int T[MMAX][NMAX], int m, int n)
{
int cont, i, j;

for (i = 0 ; i < m ; i++)
{
    printf("\ni = %d", i);
    for (j = 0 ; j < n ; j++)
        printf("\nj = %d", j);
        {
        if (T[i][j] == -3)
            {
            if (i == 0)
                {
                printf("i=0");
                if (j == 0)
                    {
                    if (T[i][j+1] != -3)
                        T[i][j+1] = T[i][j+1] + 1;
                    if (T[i+1][j] != -3)
                        T[i+1][j] = T[i+1][j] + 1;
                    if (T[i+1][j+1] != -3)
                        T[i+1][j+1] = T[i+1][j+1] + 1;
                    }
                if (j == (n-1))
                    {
                    printf("j = n-1");
                    if (T[i][j-1] != -3)
                        T[i][j-1] = T[i][j-1] + 1;
                    if (T[i+1][j] != -3)
                        T[i+1][j] = T[i+1][j] + 1;
                    if (T[i+1][j-1] != -3)
                        T[i+1][j-1] = T[i+1][j-1] + 1;
                    }
                if (j != 0 && j!= (n-1))
                    {
                    if (T[i+1][j] != -3)
                        T[i+1][j] = T[i+1][j] + 1;
                    if (T[i+1][j+1] != -3)
                        T[i+1][j+1] = T[i+1][j+1] + 1;
                    if (T[i+1][j-1] != -3)
                        T[i+1][j-1] = T[i+1][j-1] + 1;
                    if (T[i][j+1] != -3)
                        T[i][j+1] = T[i][j+1] + 1;
                    if (T[i][j-1] != -3)
                        T[i][j-1] = T[i][j-1] + 1;
                    }
                }
            if (i == (m-1))
                {
                if (j == 0)
                    {
                    if (T[i][j+1] != -3)
                        T[i][j+1] = T[i][j+1] + 1;
                    if (T[i-1][j] != -3)
                        T[i-1][j] = T[i-1][j] + 1;
                    if (T[i-1][j+1] != -3)
                        T[i-1][j+1] = T[i-1][j+1] + 1;
                    }
                if (j == (n-1))
                    {
                    if (T[i-1][j] != -3)
                        T[i-1][j] = T[i-1][j] + 1;
                    if (T[i-1][j-1] != -3)
                        T[i-1][j-1] = T[i-1][j-1] + 1;
                    if (T[i][j-1] != -3)
                        T[i][j-1] = T[i][j-1] + 1;
                    }

                if (j != 0 && j!= (n-1))
                    {
                    if (T[i-1][j] != -3)
                        T[i-1][j] = T[i-1][j] + 1;
                    if (T[i-1][j+1] != -3)
                        T[i-1][j+1] = T[i-1][j+1] + 1;
                    if (T[i-1][j-1] != -3)
                        T[i-1][j-1] = T[i-1][j-1] + 1;
                    if (T[i][j+1] != -3)
                        T[i][j+1] = T[i][j+1] + 1;
                    if (T[i][j-1] != -3)
                        T[i][j-1] = T[i][j-1] + 1;
                    }
                }
            if (j == 0 && i != 0 && i!= (m-1))
                {
                if (T[i-1][j] != -3)
                    T[i-1][j] = T[i-1][j] + 1;
                if (T[i-1][j+1] != -3)
                    T[i-1][j+1] = T[i-1][j+1] + 1;
                if (T[i][j+1] != -3)
                    T[i][j+1] = T[i][j+1] + 1;
                if (T[i+1][j+1] != -3)
                    T[i+1][j+1] = T[i+1][j+1] + 1;
                if (T[i+1][j] != -3)
                    T[i+1][j] = T[i+1][j] + 1;
                }
            if (j == (n-1) && i != 0 && i!= (m-1))
                {
                if (T[i-1][j] != -3)
                    T[i-1][j] = T[i-1][j] + 1;
                if (T[i-1][j-1] != -3)
                    T[i-1][j-1] = T[i-1][j-1] + 1;
                if (T[i][j-1] != -3)
                    T[i][j-1] = T[i][j-1] + 1;
                if (T[i+1][j-1] != -3)
                    T[i+1][j-1] = T[i+1][j-1] + 1;
                if (T[i+1][j] != -3)
                    T[i+1][j] = T[i+1][j] + 1;
                }
            if ((i != 0) && (i != (m-1)) && (j != 0) && (j != (n-1)))
                {
                if (T[i-1][j] != -3)
                    T[i-1][j] = T[i-1][j] + 1;
                if (T[i-1][j-1] != -3)
                    T[i-1][j-1] = T[i-1][j-1] + 1;
                if (T[i][j-1] != -3)
                    T[i][j-1] = T[i][j-1] + 1;
                if (T[i+1][j-1] != -3)
                    T[i+1][j-1] = T[i+1][j-1] + 1;
                if (T[i+1][j] != -3)
                    T[i+1][j] = T[i+1][j] + 1;
                if (T[i+1][j+1] != -3)
                    T[i+1][j+1] = T[i+1][j+1] + 1;
                if (T[i][j+1] != -3)
                    T[i][j+1] = T[i][j+1] + 1;
                if (T[i-1][j+1] != -3)
                    T[i-1][j+1] = T[i-1][j+1] + 1;
                }

for ease of readability and understanding: 1) use meaningful variable names. variable names should indicate `content` or `usage` (or better, both). 2) consistently indent the code. indent after every opening brace '{'. unindent before every closing brace '}'. Suggest using 4 spaces for each indent level. 3) separate code blocks (for, if, else, while, do...while, switch, case, default) via a single blank line. — user3629249, Jul 02 '17 at 03:35
when posting a question about a run time problem, as this question is doing, post code that cleanly compiles, post actual input, post expected output, post actual output. To meet these criteria, there will be a `main()` function and the appropriate header files will be included. — user3629249, Jul 02 '17 at 03:37
rather than using meaningless variable names like 'T', 'm', 'n', 'i', 'j', the code would be much more understandable by use humans if names like: 'board', 'currentRow', 'currentCol', 'maxRows', 'maxCols' were used — user3629249, Jul 02 '17 at 03:40
Please follow the axiom: *only one statement per line and (at most) one variable declaration per statement.* — user3629249, Jul 02 '17 at 03:41
the posted code references 'MMAX' and 'NMAX' but the related '#define' statements are missing — user3629249, Jul 02 '17 at 03:43
the posted code seems to be missing several lines at the end of the 'mine_counting()' function. — user3629249, Jul 02 '17 at 03:44
what are you expecting this code: `for (j = 0 ; j < n ; j++) printf("\nj = %d", j); {` to do? — user3629249, Jul 02 '17 at 03:47
the code can be cleanup quite a bit by replacing this kind of line: `board[row-1][col] = board[row-1][col] + 1;` with `board[row-1][col]++;` — user3629249, Jul 02 '17 at 04:01
how is the code handling the edges and corners of the board? Since they have less surrounding spaces to be checked? a corner had only 3 adjacent spaces to check. a edge has 5 adjacent spaces to check, the interior of the board has 8 adjacent spaces to check. Personally I would 1) have separate code for each corner 2) separate code for top edge 3) separate code for bottom edge 4) separate code for left edge 5) separate code for right edge 6) separate code for all the interior spaces. — user3629249, Jul 02 '17 at 04:05

score 1 · Accepted Answer · answered Jun 30 '17 at 19:56

Your approach can probably be fixed to work, but it's already produced too many lines of code.

First, the data structure which holds both mines and adjacency counts is a little contrived. It does represent the visible board, but it's jamming two different kinds of information into the int datatype.

Second, listing the adjacent spots is less elegant than finding them automatically with a loop. The edges get special cases (literal edge cases!) but you could avoid this by padding them with zeroes.

So, for instance,

// Parameter "mines" is the minefield, plus a ring of zero values around the edge.
// Parameter "adjacence" must be pre-filled with zeroes. It has no such padding.
void mine_counting (const bool mines[MMAX+2][NMAX+2], int adjacence[MMAX][NMAX],
                    int m, int n) {
    for ( int di = -1; di <= 1; ++ di ) {
        for ( int dj = -1; dj <= 1; ++ dj ) {
            for ( int i = 0; i != m; ++ i ) {
                for ( int j = 0; j != n; ++ j ) {
                    adjacence[i][j] += mines[i + di + 1][j + dj + 1];
                }
            }
        }
    }
}

If you wanted to make the coordinate systems of the two arrays match exactly, you could add the same padding to adjacence — it would be harmless. Then the +1's in the innermost loop would disappear.

Why add a border? All that good memory going to waste... it also adds an extra level of confusion to the entire application. — Michaël Roy, Jun 30 '17 at 20:27
@MichaëlRoy If the question weren't specifically to build an adjacency table, I'd suggest to write the function to find the adjacency value of a single cell (as you did) and *not* to cache those values but to call as needed. Then, changing `int` array to `bool` would be a net memory savings… though with `char` you could have it both ways. — Potatoswatter, Jun 30 '17 at 20:29
It's not an adjacency table. The OP's function initializes a 'mine sweeper' board, as stated. The results are stored on the board itself. — Michaël Roy, Jun 30 '17 at 20:43
@MichaëlRoy It's not an adjacency *matrix*, it's a table which counts adjacent mines. Pardon the verbiage. — Potatoswatter, Jun 30 '17 at 21:07

Michaël Roy · Answer 2 · 2017-06-30T20:37:08.683

You are really making it more complex than it is..

Why not use loops? They are made for doing these jobs. Cutting the task into smaller functions makes coding much easier and faster. It will also make your code easier to read and maintain.

[edit] I've added universal all-purpose, min and max macros, they're a must have in every C programmer's toolbox. Thanks @bolov for the suggestion.

#define MINE (-3)

// lower-case min, max are defined in <windows.h>, hence the choice 
// and the check.

#ifndef min
#define min(a, b) (((a) < (b)) ? (a) : (b))
#endif
#ifndef max
#define max(a, b) (((a) > (b)) ? (a) : (b))
#endif

int mine_detector(int board[M][N], int x, int y)
{
  int result, i, j, xmin, xmax, ymin, ymax;

  xmin = min(x - 1, 0);    // compute your boundaries beforehand
  xmax = max(x + 1, M - 1);
  ymin = min(y - 1, 0);
  ymax = max(y + 1, N - 1);

  if (board[x][y] == MINE)
    return MINE;

  result = 0;

  for (i = xmin; i <= xmax; ++i)
    for (j = ymin; j <= ymax; ++j)
      if (board[i][j] == MINE)
        ++result;

  return result;
}

void count_mines(int board[M][N])
{
  // returns detected mines on board[][] in board[][]
  int i, j;
  for (i = 0; i < M; ++i)
    for (j = 0; j < N; ++i)
       board[i][j] = mine_detector(board, i, j);
}

suggest improvement `xmin = max_int(0, x - 1)` where `int max_int(int a, int b) {...}` — bolov, Jun 30 '17 at 20:26
I personally would prefer: `xmin = clamp_int(x -1, 0, M - 1)` etc. — bolov, Jun 30 '17 at 20:28

"Minesweeper" in C, trouble on counting surrounding mines are near a certain spot in the board

2 Answers2