Coombes
Coombes

Reputation: 203

If statements not working as intended

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?

Upvotes: 1

Views: 257

Answers (3)

Edward Falk
Edward Falk

Reputation: 10063

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.

Upvotes: 0

Eran
Eran

Reputation: 393781

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
}

Upvotes: 1

AlexR
AlexR

Reputation: 115328

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.

Upvotes: 0

Related Questions