Function returning a number of 2D array locations in C doesn't work

Question

I'm new to c and trying to create the minesweeper game. I think my code has some problems with the data type conversion but I don't understand why. Checked this link

How to cast or convert an unsigned int to int in C?

and tried to implement the info in my code.

I'm trying to implement a function that returns the number of neighboring locations of a x,y coordinated 2D array that have mines.

int neighbours(const Field *f, unsigned int x, unsigned int y)
{
    int z = 0;
    int i,j;
    unsigned int a = y-1;
    j =(int)a;
    unsigned int b =x-1;
    i = (int)b;

    for(; j<=(a+2); j++){
        if(j>=0 && j<=f->ysize){
            for (; i<=(b+2); i++){
                if ( i>=0 && i<=f->xsize && (f->places[j][i] == UNKNOWN_MINE ||f->places[j][i] == KNOWN_MINE)){
                    z++;
                 }

             }
            i = b;
        }  
    }
    return z;
 }

if `x` or `y` are ever 0, then `x-1` and `y-1` will not give you `-1` because they are unsigned. Instead, it will roll around to become `INT_MAX` — AndyG, Mar 22 '14 at 12:07
Can you please elaborate? What kind of problem do you have? Have you tried stepping though the code line by line in a debugger? — Some programmer dude, Mar 22 '14 at 12:07
When I take my car to the shop, I never say "it doesn't work, please fix it". I say "It doesn't work. When I step on the breaks, it doesn't slow down. Please fix it." Applied here, "It doesn't work. When I input X, the result is supposed to be Y but instead I get Z." — mah, Mar 22 '14 at 12:07

Gábor Buella · Answer 1 · 2014-03-22T14:21:38.710

The code

unsigned a = y-1;

gives you a == UINT_MAX if y == 0

the condition

  j >= 0

is just saying "if this non-negative integer is not negative", so not very helpful.

A good idea would be to store the data with some padding, where the rectangle map is padded with one width of integers on each side, all having a uniqe value you would call something like PADDING_FIELD

This way your function would simpler, more effecient:

int neighbours(const Field *f, unsigned int x, unsigned int y)
{
  int z = 0;

  assert(x > 0 && x < UINT_MAX);
  assert(y > 0 && y < UINT_MAX);

  for(unsigned int j = y-1; j <= y+1); j++) {
        for (unsigned int i = x-1; i <= x+1; i++) {
            if ((f->places[j][i] == UNKNOWN_MINE ||f->places[j][i] == KNOWN_MINE)){
                z++;
            }
        }
  }
  return z;
}

This way you don't need to check being out of bounds, if you would address something outside, you would just get f->places[j][i] == PADDING_FIELD , which doesn't match anything. Also no problem with type conversions, you know x or y can't be zero or UINT_MAX.

EDIT

You could also eliminate one of the checks in the little loop, if you choose the values of UKNOWN_MINE and KNOWN_MINE carefully. Could do something like:

    static const field mine = 4;
    static const field known = 8;

    inline bool is_mine(field f)
    {
            return (f & mine) != 0;
    }

    inline bool is_knownmine(field f)
    {
            return (f & mine & known) != 0;
    }

    inline bool is_unknownmine(field f)
    {
            return is_mine(f) && !is_knownmine(f);
    }

    int neighbors(....)
    {
     .....
      if (is_mine(f->places[j][i]) {
          ++z;
      }
     .....
    }

Function returning a number of 2D array locations in C doesn't work

1 Answers1