-1

I've a matrix with size 7x7 that represents a game board. When a player makes a move, the program has to check the positions around the coordinates where the piece is, in order to detect another piece aside.

I use this function:

int check_position(COORDINATES coordinates, char board[7][7]) {
  int result = -1;

  if (board[coordinates.x][coordinates.y] != 'O' && board[coordinates.x-1][coordinates.y] != 'O' && board[coordinates.x][coordinates.y-1] != 'O' && board[coordinates.x+1][coordinates.y] != 'O' && board[coordinates.x][coordinates.y+1] != 'O' && board[coordinates.x-1][coordinates.y-1] != 'O' && board[coordinates.x+1][coordinates.y+1] != 'O' && board[coordinates.x-1][coordinates.y+1] != 'O' && board[coordinates.x+1][coordinates.y-1] != 'O') {
    result = 1;
  }

  return result;
}

The first parameter are the coordinates of the player's piece as a struct, with members x and y. The second parameter is the board array.

The if statement doesn't work to well, and I don't know which alternative can I take.

Can you help me? Thanks!

too honest for this site
  • 12,050
  • 4
  • 30
  • 52
mariec
  • 213
  • 3
  • 9
  • 4
    You probably need more `if` statements (or rather `else if`) and in them check that you're not indexing out of range. For example, think about what would happen if e.g. `coordinates.x` is `0` or `6`. You *can* to it as a single expression, but it will be very hard to read and maintain. – Some programmer dude Jun 21 '15 at 17:14
  • Is that for C or C++. They are different languages, pick one. There might be different answers for C++. – too honest for this site Jun 21 '15 at 17:35

2 Answers2

1

You forgot about your coordinates overflowing at the borders. You can either test for this, or:

Hint: Make the array two rows and columns larger than the board and fill the border with "empty" marker. The active board will the have coordinates 1...7 This way your coordinates cannot wrap (1 - 1 and 7 + 1 are still within the array) and you do not have to care about the borders.

Note: If you just want to return a boolean value, it would be better to use stdbool.h and return a bool result. That way, the caller can directly use that function as a condition:

#include <stdbool.h>

...

bool check_position(COORDINATES coordinates, const char board[9][9]) {
    int x = coordinates.x - 1 
    for ( int xc = 0 ; xc < 3 ; xc++ ) {
        int y = coodinates.y - 1;
        for ( int yc = 0 ; yc < 3 ; yc++ ) {
            if ( board[x][y] != '0' )
                return true;
            y++;
        }
        x++;
    }
    return false;
}

Note: as you only need one one non-empty field, you can terminate instantly if you found one. That is identical to the multiple conditions. Of course, that also works for your original int result.

Note2: I modified the type of board to being const, as it is not changed inside the function.

too honest for this site
  • 12,050
  • 4
  • 30
  • 52
-1

You could also solve the edge overflow like this. Edit improved after discussion with @Olaf

#define BOARD 7

int check_position(COORDINATES coordinates, char board[BOARD][BOARD]) {
    int result = -1;
    int left   = coordinates.x == 0 ? 0 : coordinates.x - 1;
    int top    = coordinates.y == 0 ? 0 : coordinates.y - 1;
    int right  = coordinates.x == BOARD-1 ? coordinates.x : coordinates.x + 1;
    int bottom = coordinates.y == BOARD-1 ? coordinates.y : coordinates.y + 1;

    if (board[left]         [top] != 'O' && 
        board[coordinates.x][top] != 'O' &&
        board[right]        [top] != 'O' &&
        board[left]         [coordinates.y] != 'O' &&
        board[coordinates.x][coordinates.y] != 'O' && 
        board[right]        [coordinates.y] != 'O' &&
        board[left]         [bottom] != 'O' &&
        board[coordinates.x][bottom] != 'O' &&
        board[right]        [bottom] != 'O' && )
    {
        result = 1;
    }

    return result;
}
Weather Vane
  • 33,872
  • 7
  • 36
  • 56
  • 1
    That will not work with unsigned coordinates. We do not know their scalar types. – too honest for this site Jun 21 '15 at 17:41
  • @Olaf it does work. When `unsigned x = 0` and `int left = x - 1` the value of `left` is `-1`. – Weather Vane Jun 21 '15 at 17:45
  • 1
    Just take the first condition: (assume `x == 0`) => `(int)left = UINT_MAX` is UB, as the `int` cannot hold that value (same rank). – too honest for this site Jun 21 '15 at 18:09
  • @Olaf, the value of `x` is converted to `int` before the `1` is subtracted. But it's hypothetical since you are only guessing about something OP didn't post. – Weather Vane Jun 21 '15 at 18:10
  • @Olaf yes I now read that the `unsigned` type has precedence in the subtraction, but there can only be confusion in comparisons. `unsigned 0 - 1` => `0xFFFFFFFF` which is then assignd to `int` which is `-1`. This is well defined, see here http://stackoverflow.com/questions/7221409/is-unsigned-integer-subtraction-defined-behavior – Weather Vane Jun 21 '15 at 18:22
  • 1
    `unsigned int` is _not_ converted to int, as both have the same rank. If that were, any arithmetics on unsigned relying on wrap would become UB, but wrapping is actually well defined. We both are speculating. That's why I prefer to play safe and do not assume it is `int` as you do. (Neither do I assume it is _not_ - heck, it could be `char` or `unsigned long long`). – too honest for this site Jun 21 '15 at 18:22
  • @Olaf I commented just ahead of you. – Weather Vane Jun 21 '15 at 18:23
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/81127/discussion-between-olaf-and-weather-vane). – too honest for this site Jun 21 '15 at 18:23