0

I have programmed a TicTacToe game. I created a list for checking if the game is over or still going on (checkGameOver). And the other one looks similiar but it is for checking who has won and it should print out, that player 1 or player 2 had won. When I run the game and when I play the top row fields 1 2 3. The computer print the message that the process is finished. But when I play the fiels 4 5 6 or anyother else the game do not finish.

Please let me know if you know the problem than I could send you the whole code.

   *public static boolean checkGameOver(char [][] gameBoard){
    List topRow = Arrays.asList(1, 2, 3);
    List midRow = Arrays.asList(4, 5, 6);
    List botRow = Arrays.asList(7, 8, 9);

    List leftCol = Arrays.asList(1, 4, 7);
    List midCol = Arrays.asList(2, 5, 8);
    List rightCol = Arrays.asList(3, 6, 9);

    List cross1 = Arrays.asList(1, 5, 9);
    List cross2 = Arrays.asList(7, 5, 3);

    List<List> gameOver = new ArrayList<List>();
    gameOver.add(topRow);
    gameOver.add(midRow);
    gameOver.add(botRow);
    gameOver.add(leftCol);
    gameOver.add(midCol);
    gameOver.add(rightCol);
    gameOver.add(cross1);
    gameOver.add(cross2);

    for(List l : gameOver) {
        if(playerPositions.containsAll(l) || computerPositions.contains(l)) {
            return true;
        } else {
            return false;
        }
    }

    return false;
}

public static String checkWinner(char [][] gameBoard) {

    List topRow = Arrays.asList(1, 2, 3);
    List midRow = Arrays.asList(4, 5, 6);
    List botRow = Arrays.asList(7, 8, 9);

    List leftCol = Arrays.asList(1, 4, 7);
    List midCol = Arrays.asList(2, 5, 8);
    List rightCol = Arrays.asList(3, 6, 9);

    List cross1 = Arrays.asList(1, 5, 9);
    List cross2 = Arrays.asList(7, 5, 3);

    List<List> gameOver = new ArrayList<List>();
    gameOver.add(topRow);
    gameOver.add(midRow);
    gameOver.add(botRow);
    gameOver.add(leftCol);
    gameOver.add(midCol);
    gameOver.add(rightCol);
    gameOver.add(cross1);
    gameOver.add(cross2);

    for(List l : gameOver) {
        if(playerPositions.containsAll(l)) {
            return "Glückwunsch du hast gewonnen!";
        } else if(computerPositions.contains(l)) {
            return "Computer hat gewonnen!";
        } else if(playerPositions.size() + computerPositions.size() == 9) {
            return "Unentschieden!";
        }
    }

    return "";
}*
Don_Angel
  • 17
  • 2
  • Remove the "`return false;`" in the else. – Andy Turner Sep 16 '21 at 12:07
  • More generally, if your current output does not match your desired output, and you don't know why then it's time to start debugging. If you're not sure how to go about doing this, then please check out [How to debug small programs](https://ericlippert.com/2014/03/05/how-to-debug-small-programs/). It won't solve your direct problem, but it will give you steps that you can follow that should help you solve it yourself, or even if that is not successful, then at least help you to better isolate your problem so that your question can be more focused and easier to answer. – Hovercraft Full Of Eels Sep 16 '21 at 12:08
  • Doing basic debugging where you step through your code, checking the state of variables as doing so, would quickly have revealed this bug to you. – Hovercraft Full Of Eels Sep 16 '21 at 12:09
  • Please, [don't use raw types](https://stackoverflow.com/questions/2770321/what-is-a-raw-type-and-why-shouldnt-we-use-it): `List topRow`, `List> gameOver` etc. – Andy Turner Sep 16 '21 at 12:09

1 Answers1

1

Remove the "return false;" in the else.

    for(List l : gameOver) {
        if(playerPositions.containsAll(l) || computerPositions.contains(l)) {
            return true;
        } else {
            return false;  // Remove this.
        }
    }

Assuming gameOver isn't empty, this loop only executes for the first element, because it always returns from within the loop body.


It's far from ideal that you duplicate the game over logic in the method for checking whether there is a winner, and the method for checking who is the winner.

You should define an enum as the return value of the checkGameOver method:

enum Winner {
  PLAYER, COMPUTER, NONE
}

Then you can do something like:

for(List l : gameOver) {
   if (playerPositions.containsAll(l)) return Winner.PLAYER;
   if (computerPositions.containsAll(l)) return Winner.COMPUTER;
}
return Winner.NONE;

Then you can check the return value of the method (e.g. with a switch) to print out the corresponding message.

Hovercraft Full Of Eels
  • 283,665
  • 25
  • 256
  • 373
Andy Turner
  • 137,514
  • 11
  • 162
  • 243
  • Your enum idea is excellent (1+) as is your simplification of the code, but I would extend it one bit further: rename it to `public enum Player{ USER, COMPUTER, NONE}` use it to declare who the winner is (as you're doing), but also change the `char [][] gameBoard` to `Player [][] gameBoard` to allow compiler type checking in other parts of the program. – Hovercraft Full Of Eels Sep 16 '21 at 14:13