Reputation: 203
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
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
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
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