If statements not working as intended

Question

Basically I'm currently trying to make a reversi game for Android and my if statements are causing me a bit of a headache, it seems if conditions are right for more than one it's only going through the motions on one of the statements and just leaving the other one. My code looks like:

if (check[position] == 0
                            && (check[position - 8] == 2
                                    || check[position + 8] == 2
                                    || check[position + 1] == 2
                                    || check[position - 1] == 2
                                    || check[position - 9] == 2
                                    || check[position + 9] == 2
                                    || check[position - 7] == 2 || check[position + 7] == 2)) {

                        if (check[position + 8] == 2) {
                            for (int i = position; i < 56; i += 8) {
                                if (check[i] == 1) {
                                    for (int j = position; j < i; j += 8) {
                                        check[j] = 1;
                                    }
                                    playerno = 2;
                                    break;
                                } else
                                    break;
                            }
                        } else if (check[position - 8] == 2) {
                            for (int i = position; i > 8; i -= 8) {
                                if (check[i] == 1) {
                                    for (int j = position; j > i; j -= 8) {
                                        check[j] = 1;
                                    }
                                    playerno = 2;
                                    break;
                                } else
                                    break;
                            }
                        } else if (check[position + 1] == 2) {
                            for (int i = position; i < board.length; i++) {
                                if (check[i] == 1) {
                                    for (int j = position; j < i; j++) {
                                        check[j] = 1;
                                    }
                                    playerno = 2;
                                    break;
                                }
                                if (i == 7 || i == 15 || i == 23 || i == 31
                                        || i == 39 || i == 47 || i == 55
                                        || i == 63) {
                                    break;
                                }
                            }
                        } else if (check[position - 1] == 2) {
                            for (int i = position; i > 0; i--) {
                                if (check[i] == 1) {
                                    for (int j = position; j > i; j--) {
                                        check[j] = 1;
                                    }
                                    playerno = 2;
                                    break;
                                }
                                if (i == 0 || i == 8 || i == 16 || i == 24
                                        || i == 32 || i == 40 || i == 48
                                        || i == 56) {
                                    break;
                                }
                            }
                        }

Check is just an int array that notes what player holds that particular piece on the board Now for some reason if I have a position that satisfies two of these conditions it only goes through one of the if statements and more often than not this yields the game treating it as an invalid move, I was wondering how I can get around that?

Have you tried using recursion to solve this? Just scan outwards. — , Apr 18 '13 at 22:30
If your position is at the top or bottom of the board, these conditionals will overflow the array. — Edward Falk, Apr 18 '13 at 22:37
Sorry but I'm not even going to attempt this one. Please do **not** use a one-dimensional array to simulate a two-dimensional board. Fix that then come back and ask again what is wrong. — OldCurmudgeon, Apr 18 '13 at 22:42
There's a reason for the use of a one-dimensional array, if you read the question you'd see I was programming it for Android and I've always been taught that it's best to try and use a one-dimensional array where possible in Android so it's how I've learnt to do it. — Coombes, Apr 18 '13 at 22:46
@Coombes I don't quite know where you learned that, but that's really not true — , Apr 18 '13 at 22:48
@Coombes - Whoever taught you that should be ... something very unpleasant. **Always** use a nd array for a nd problem. Reversi can be played in 3d too - I know - I'used to play it. Do you really think you could code that in a 1d array? — OldCurmudgeon, Apr 18 '13 at 22:48
At the very least, abstract the 1d array by wrapping it in functions that take row,column arguments. In fact, you should implement the board as a class all its own and make the storage private. Access it only through methods that take row,column arguments. — Edward Falk, Apr 19 '13 at 01:20

score 1 · Answer 1 · answered Apr 18 '13 at 22:48

if I have a position that satisfies two of these conditions it only goes through one of the if statements

Are you referring to this statement ?

if (conditionA) {
  BlockA
} else if (conditionB) {
  BlockB
}  else if (conditionC) {
  BlockC
}  else if (conditionD) {
  BlockD
}

If you do, it's no wonder only one of the if blocks is executed. Only the block of the first condition evaluated to true is executed.

If you want to allow more than one block to be executed, change it to :

if (conditionA) {
  BlockA
} 
if (conditionB) {
  BlockB
} 
if (conditionC) {
  BlockC
}
if (conditionD) {
  BlockD
}

score 0 · Answer 2 · answered Apr 18 '13 at 22:32

0

First, your conditions are extremely complicated and difficult to read. Try to simplify them. Try to separate one complex condition onto several simpler expressions. And use debugger.

answered Apr 18 '13 at 22:32

AlexR

114,158
16
130
208

2

Shouldn't this be a comment? – jlordo Apr 18 '13 at 22:34

Edward Falk · Answer 3 · 2013-04-19T01:25:33.020

AlexR is right, your logic is too convoluted, and the formatting makes your code extremely hard to read, which makes it hard to debug.

I'm not going to solve the whole thing for you, but here are some suggested changes. Mostly, you should break up your logic into bite-sized chunks.

Edit: per my comment above, implement the board as a class:

class Board {
    private final int[] check = new int[BOARD_WIDTH*BOARD_HEIGHT];
    public Board() { for (int i=0; i < BOARD_WIDTH*BOARD_HEIGHT; check[i++] = 0); }
    public final int get(int x, int y) { return check[y*BOARD_WIDTH + x]; }
    public final void set(int x, int y, int val) { check[y*BOARD_WIDTH+x] = val; }

    /**
     * Return true if this square is free
     */
    public final boolean isFree(int x, int y) {
      if (x < 0 || x >= BOARD_WIDTH || y < 0 || y >= BOARD_HEIGHT) return false;
      int position = y*BOARD_WIDTH + x;
      return check[position] == 0;
    }

    /**
     * Return true if this square is occupied by opponent
     */
    public final boolean isOccupiedBy2(int x, int y) {
      if (x < 0 || x >= BOARD_WIDTH || y < 0 || y >= BOARD_HEIGHT) return false;
      int position = y*BOARD_WIDTH + x;
      return check[position] == 2;
    }

    /**
     * Return true if any neighboring square is occupied by opponent
     */
    final boolean isNeighborOccupied(int x, int y) {
      for (int i=x-1; i >= x+1; ++i)
        for (int j=y-1; j >= y+1; ++j)
          if ((i != x || j != y) && isOccupiedBy2(i,j)) return true;
      return false;
    }
    // etc.
}

Now from there, re-write your logic above:

if (board.isFree(x,y) && board.isNeighborOccupied(x,y)) {
    if (board.isOccupiedBy2(x,y+1)) {
        ...
    }
    else if (board.isOccupiedBy2(x,y-1)) {
        ...
    }
    else if (board.isOccupiedBy2(x+1,y)) {
        ...
    }
    else if (board.isOccupiedBy2(x-1,y)) {
        ...
    }
}

See how much easier this is to read? It will be that much easier to debug as well.

Finally, see if Eran's post doesn't solve your bug. Only one of those four conditionals will be executed. Since you're only testing four of the eight neighbor squares, I'm guessing you meant to be testing up, left, down, right, so perhaps the second "else" was a mistake.

If statements not working as intended

3 Answers3