If and if else not picking up trigger conditions

Question

So I'm working on a tic-tac-toe program, and I am working on a method that will detect whether or not a player has won.

The game is programmed like this: The game board is an int array that holds 9 integers. Player 1 will be able to fill in a specified part of the array with a "1", and Player 2 fills a specified part of the array with a "2".

Now, in my method to check for a victory, I'm using an if/else statement that is quite long and convoluted, but in the end there are no compilation errors and the program runs correctly, but when certain spaces are filled up in a way that would trigger a victory condition, my method is not printing the line that it is supposed to.

for example:

run:
Player 1, please enter the number of the square that you want to mark (1 - 9) 
1
[1, 0, 0, 0, 0, 0, 0, 0, 0]
Player 2, please enter the number of the square that you want to mark (1 - 9) 
6
[1, 0, 0, 0, 0, 2, 0, 0, 0]
Player 1, please enter the number of the square that you want to mark (1 - 9) 
2
[1, 1, 0, 0, 0, 2, 0, 0, 0]
Player 2, please enter the number of the square that you want to mark (1 - 9) 
8
[1, 1, 0, 0, 0, 2, 0, 2, 0]
Player 1, please enter the number of the square that you want to mark (1 - 9) 
3
[1, 1, 1, 0, 0, 2, 0, 2, 0]
Player 2, please enter the number of the square that you want to mark (1 - 9)

So as you can see in the last line, the first 3 integers in the array have been filled up by player one, which should have triggered a message saying that Player 1 has won, but no such message comes up.

Here is the method that is supposed to pick up the victory condition:

 public void checkWin() {
    int row1 = board[0] + board[1] + board[2];
    int row2 = board[3] + board[4] + board[5];
    int row3 = board[6] + board[7] + board[8];

    int column1 = board[0] + board[3] + board[6];
    int column2 = board[1] + board[4] + board[7];
    int column3 = board[2] + board[5] + board[8];

    int cross1 = board[0] + board[4] + board[8];
    int cross2 = board[2] + board[4] + board[6];

    int square0 = board[0];
    int square1 = board[1];
    int square2 = board[2];
    int square3 = board[3];
    int square4 = board[4];
    int square5 = board[5];
    int square6 = board[6];
    int square7 = board[7];
    int square8 = board[8];

    if (row1 == 3 | row1 == 6 | row2 == 3 | row2 == 6 | row3 == 3 | row3 == 6|
            column1 == 3 | column1 == 6 | column2 == 3 | column2 == 6 | column3 == 3 | column3 == 6|
            cross1 == 3 | cross1 == 6 | cross2 == 3 | cross2 == 6) {
    } else if ((square0 == 1 && square1 == 1 && square2 == 1) || 
            (square3 == 1 && square4 == 1 && square5 == 1) ||
            (square6 == 1 && square7 == 1 && square8 == 1) ||
            (square0 == 1 && square3 == 1 && square6 == 1) ||
            (square1 == 1 && square4 == 1 && square7 == 1) ||
            (square2 == 1 && square5 == 1 && square8 == 1) ||
            (square0 == 1 && square4 == 1 && square8 == 1) ||
            (square2 == 1 && square4 == 1 && square6 == 1)) {
        System.out.println("Player 1 has won this game!");
    } else if ((square0 == 2 && square1 == 2 && square2 == 2) || 
            (square3 == 2 && square4 == 2 && square5 == 2) ||
            (square6 == 2 && square7 == 2 && square8 == 2) ||
            (square0 == 2 && square3 == 2 && square6 == 2) ||
            (square1 == 2 && square4 == 2 && square7 == 2) ||
            (square2 == 2 && square5 == 2 && square8 == 2) ||
            (square0 == 2 && square4 == 2 && square8 == 2) ||
            (square2 == 2 && square4 == 2 && square6 == 2)) {
        System.out.println("Player 2 has won this game!");
    }



}

I know it's quite convoluted, but I couldn't quite find a way to shorten it down while still illustrating how the method is supposed to work.

I'd really appreciate it if anyone would point out what I'm missing or doing wrong.

Thanks guys!

@Idos It is valid in Java. [java - Why do we usually use `||` not `|`, what is the difference? - Stack Overflow](http://stackoverflow.com/questions/7101992/why-do-we-usually-use-not-what-is-the-difference) — MikeCAT, May 15 '16 at 06:45
Oh wow I learned something new. But glad I always used `||` makes more sense to short-circuit ... — Idos, May 15 '16 at 06:46
Would you mind posting a [Minimal, Complete, and Verifiable example](http://stackoverflow.com/help/mcve)? — MikeCAT, May 15 '16 at 06:51
@MikeCAT Do you want me to post my main class and the other two methods I have? They really don't do much besides asking the user to input some numbers and filling the spaces on the array. — Anthropologist, May 15 '16 at 06:56
@ChetSpalsky *Minimum* and *Complete* example is good. Minimum: you won't need to post code for beautiful input and output. What I expect is a code that initlaizes `board` and calls `checkWin`. Complete: Current code is not complete because neither what is `board` nor what is is it is unknown to us. — MikeCAT, May 15 '16 at 07:14

Amer Qarabsa · Answer 1 · 2016-05-15T09:45:57.457

The guys replied to you the reason of the failure yet i think you can write it in better way.

public void checkWin() {

   List<ResultChecker> resultCheckerList = new ArrayList<ResultChecker>();
   int row1 = board[0] + board[1] + board[2];
   resultCheckerList.add(new ResultChecker(row1/3,row1%3));

   int row2 = board[3] + board[4] + board[5];
   resultCheckerList.add(new ResultChecker(row2/3,row2%3));
   int row3 = board[6] + board[7] + board[8];
   resultCheckerList.add(new ResultChecker(row3/3,row3%3));

   int column1 = board[0] + board[3] + board[6];
   resultCheckerList.add(new ResultChecker(column1/3,column1%3));
   int column2 = board[1] + board[4] + board[7];
   resultCheckerList.add(new ResultChecker(column2/3,column2%3));
   int column3 = board[2] + board[5] + board[8];
   resultCheckerList.add(new ResultChecker(column3/3,column3%3));
   int cross1 = board[0] + board[4] + board[8];
   resultCheckerList.add(new ResultChecker(cross1/3,cross1%3));
   int cross2 = board[2] + board[4] + board[6];
   resultCheckerList.add(new ResultChecker(cross2/3,cross2%3));

   for(ResultChecker rc:resultCheckerList) {
    if(rc.isWin()) {
        if(rc.isFirstPlayer()) {
            System.out.println("Player 1 has won this game!");
        } else {
            System.out.println("Player 2 has won this game!");
        }
        break;
    }
  }




 }

 private Class ResultChecker {
   private int divResult;
   private int modResult;
   public ResultChecker(int divResult,int modResult) {
    this.divResult = divResult;
    this.modResult = modResult;
   }

   public boolean isWin() {
    return this.modResult == 0;
  }

  public boolean isFirstPlayer() {
    return this.divResult == 1;
  }
}

moving `/3` `%3` **into** the constructor would make the rewrite even better IMHO, and you probably miss a "break", once a win is found — , May 15 '16 at 09:24

score 0 · Answer 2 · answered May 15 '16 at 06:59

0

Hmm, it seems like if row1 == 3, then you just keep going without checking for victory. I may misunderstand, but for me what you're doing is:

if A {
    // does nothing
} else if B {
    // Player 1 won
} else if C {
    // Player 2 won
}

Where A checks if there is a winning combination, and B and C check which player has actually won. In my sense, it should rather be:

if A {
    // Winning game, checks winner
    if B {
        // Player 1 won
    } else {
        // Player 2 won
    }
}

answered May 15 '16 at 06:59

T. Claverie

11,380
1
17
28

1

Simply removing A should be good because it is just long, make mistakes (ex. `board[0]=1, board[1]=2, board[2]=0`) and the following checking is enough. – MikeCAT May 15 '16 at 07:01
True, didn't think of this case. – T. Claverie May 15 '16 at 07:11

If and if else not picking up trigger conditions

2 Answers2