Java Tic Tac Toe console

Question

everyone!I have created a bit ugly looking Tic Tac Toe console game using Java. I'm learning how to code and I would love to hear some comments about my code. Because it's quite streatforward, I'm not using classes and objects ( my assignment was not to use any ). Thank you in advance!

package com.company;

import java.util.ArrayList;
import java.util.Scanner;

public class Main {
    public static Scanner scan = new Scanner(System.in);

    public static char[][] table = {
        {'1', '2', '3'},
        {'4', '5', '6'},
        {'7', '8', '9'}
    };

    public static ArrayList<Character> picksTillNow = new ArrayList<Character>();
    public static char playerOne = ' ';
    public static char playerTwo = ' ';

    public static void main(String[] args) {

    for(char i = '1'; i <= '9'; i++){
        picksTillNow.add(i);
    }

    printTable();

    while (true){
        playerOneChoice();
        endGame();
        playerTwoChoice();
        endGame();
    }
}

/**
 * Prints the table with no players choices made ( 1 - 9 )
 */
public static void printTable() {

    for (int i = 0; i < table.length; i++) {
        for (int j = 0; j < table[i].length; j++) {
            System.out.print(table[i][j] + " ");
        }
        if (table[i][2] != 9) {
            System.out.println();
        }
    }
}


/**
 * Prints the table with the players choice included
 * @param player - player 1 or player 2
 * @param choice - char from 1 - 9
 */
public static void printTable(int player, char choice){

    for (int i = 0; i < table.length ; i++) {
        for (int j = 0; j < table[i].length; j++) {
            if(choice == table[i][j] && player == 1){
                //X
                table[i][j] = 'X';
            }else if(choice == table[i][j] && player == 2){
                //O
                table[i][j] = 'O';
            }
            System.out.print(table[i][j] + " ");
        }
        if(table[i][2] != 9){
            System.out.println();
        }
    }
}


/**
 * Asks for player one choice method and prints the table, checks if that position has been chosen
 */
public static void playerOneChoice(){
    System.out.print("Играч 1: ");
    playerOne = scan.next().charAt(0);
    if(picksTillNow.contains(playerOne)){
        picksTillNow.remove(picksTillNow.indexOf(playerOne));
        printTable(1, playerOne);
    }else{
        playerOneChoice();
    }
}


/**
 * Asks for player one choice method and prints the table, checks if that position has been chosen
 */
public static void playerTwoChoice(){
    System.out.print("Играч 2: ");
    playerTwo = scan.next().charAt(0);
    if(picksTillNow.contains(playerTwo)){
        picksTillNow.remove(picksTillNow.indexOf(playerTwo));
        printTable(2, playerTwo);
    }else{
        playerTwoChoice();
    }
}

/**
 * Checks for a winner. If yes - exits program
 */
public static void endGame(){
    for(int i = 0; i < table.length; i++){
        if(table[i][0] == table[i][1] && table[i][1] == table[i][2] && table[0][i] != 'О'){
            System.out.println("Победа!");
            System.exit(0);
        }
    }

    for(int j = 0; j < table.length; j++){
        if(table[0][j] == table[1][j] && table[1][j] == table[2][j] && table[j][0] != 'O'){
            System.out.println("Победа!");
            System.exit(0);
        }
    }

    if(table[0][0] == table[1][1] && table[1][1] == table[2][2] && table[0][0] != 'O'){
        System.out.println("Победа!");
        System.exit(0);
    }

    if(table[0][2] == table[1][1] && table[1][1] == table[2][0] && table[0][2] != 'O'){
        System.out.println("Победа!");
        System.exit(0);
    }

    if(picksTillNow.isEmpty()) System.exit(0);
 }
}

Answer 1

Your formatting seems to be inconsistent, use an automatic code formatter (your IDE most likely has one).

public static Scanner scan = new Scanner(System.in);

The usage of the scanner (or streams in general) should most likely be scoped. Streams are rather easily associated with native resources which must be destroyed explicitly to be freed.

---

    public static char[][] table = {
        {'1', '2', '3'},
        {'4', '5', '6'},
        {'7', '8', '9'}
    };

Why is the table prefilled with values?

---

    public static ArrayList<Character> picksTillNow = new ArrayList<Character>();

Try to always use the most super-class you can use and get away with, makes it easier to make sure that you're coupling classes through methods of the actual implementation instead of the interface. In this case, declare the variable with List.

---

    public static char playerOne = ' ';
    public static char playerTwo = ' ';

You're preparing these, but you only use them in a very limited scope, you should declare them there.

---

    for(char i = '1'; i <= '9'; i++){
        picksTillNow.add(i);
    }

Autoboxing.

---

    while (true){

You could also have a boolean like running or playing or gameRunning and set it depending on the return value of `endGame().

Or, as the number of turns is fixed, you might do a mixture of both:

int turn = 0;

while (turn++ < 9 && nobodyHasWonYet) {
    // Logic.
}

---

        endGame();

That's a bad name for the method, as it does not always end the game.

---

    for (int i = 0; i < table.length; i++) {
        for (int j = 0; j < table[i].length; j++) {

I'm a persistent advocate that you're only allowed to use single-letter variable names when dealing with dimensions ("x", "y", "z"), and that excludes using i and j). In this case actually, using x and y as variable names would improve the readability of the code. Even better would be using row and column.

---

        if (table[i][2] != 9) {
            System.out.println();
        }

But the default value of this field is overwritten at some point, isn't it?

---

    }else{
        playerOneChoice();
    }

You're recursing here, so keeping to hit Return without entering something should at some point crash your game because of a stackoverflow.

---

public static void endGame(){
    for(int i = 0; i < table.length; i++){
        if(table[i][0] == table[i][1] && table[i][1] == table[i][2] && table[0][i] != 'О'){
            System.out.println("Победа!");
            System.exit(0);
        }
    }

    for(int j = 0; j < table.length; j++){
        if(table[0][j] == table[1][j] && table[1][j] == table[2][j] && table[j][0] != 'O'){
            System.out.println("Победа!");
            System.exit(0);
        }
    }

    if(table[0][0] == table[1][1] && table[1][1] == table[2][2] && table[0][0] != 'O'){
        System.out.println("Победа!");
        System.exit(0);
    }

    if(table[0][2] == table[1][1] && table[1][1] == table[2][0] && table[0][2] != 'O'){
        System.out.println("Победа!");
        System.exit(0);
    }

    if(picksTillNow.isEmpty()) System.exit(0);
 }

If I don't completely suck at math (and Tic Tac Toe), there should only be 8 winning positions, and even then there are only 4 and the other 4 are mirrored or rotated. So it might be more interesting and easier to maintain if these are hardcoded.

---

            System.exit(0);

Something to keep in mind is that System.exit is not "exit the application" but "kill the JVM process". When invoking this not even finally block may run. In this case it does not matter, but something to keep in mind.

---

If I read this right, you have the playing field with the numbers in it, which then get replaced with the player choices. Whether it is valid choice is validated through the remaining fields in your list. Instead I suggest you keep the state limited to only the playing field. Assuming your field definition, you can check whether a field is still free by calculating the position of the input and then checking whether there is an O or X placed, like this:

int fieldChoice = getChoice();
int row = fieldChoice % 3;
int column = fieldChoice / 3; // Might be other way around...never can remember.

char selectedField = playingField[row][column];

if (selectedField != 'X' && selectedField != 'O') {
    // Set it.
} else {
    // No dice.
}

playerOneChoice and playerTwoChoice can be rolled into one, by passing the wanted field player as parameter. Using my above example:

public static void playerChoice(char playerCharacter) {
    int fieldChoice = getChoice();
    int row = fieldChoice % 3;
    int column = fieldChoice / 3; // Might be other way around...never can remember.
    
    char selectedField = playingField[row][column];
    
    if (selectedField != 'X' && selectedField != 'O') {
        playingField[row][column] = playerCharacter;
    } else {
        // No dice.
    }
}

Having said that, your print function would then boil down to this:

for (int row = 0; row < 3; row++) {
    for (int column = 0; column < 3; column++) {
        System.out.println(playingField[row][column] + " ");
    }
    
    System.out.println();
}

Hardcoding the size here is not a bad thing, as long as you assume a Tic Tac Toe game with a 3x3 playing field. If you want to support larger playing fields, you'll have to adjust the rest of your logic anyway.

Coming around to checking whether somebody has won, the winning conditions would boil down to:

// Rows
playingField[0][0] == playingField[0][1] && playingField[0][1] == playingField[0][2];
playingField[1][0] == playingField[1][1] && playingField[1][1] == playingField[1][2];
playingField[2][0] == playingField[2][1] && playingField[2][1] == playingField[2][2];

// Columns
playingField[0][0] == playingField[1][0] && playingField[1][0] == playingField[2][0];
playingField[0][1] == playingField[1][1] && playingField[1][1] == playingField[2][1];
playingField[0][2] == playingField[1][2] && playingField[1][2] == playingField[2][2];

// Diagonal
playingField[0][0] == playingField[1][1] && playingField[1][1] == playingField[2][2];
playingField[0][2] == playingField[1][1] && playingField[1][1] == playingField[2][0];

You will notice, that is just as short as using your loops, and we can chain them with || directly for a return. However, that will not tell us who one. For that need some additional logic, namely an if on every line. But we could also assume that the winner is the player that last played, which is a fair assumption, actually. To make it more readable we can add us three helper functions:

public static boolean isWinningRow(int rowIndex) {
    return playingField[row][0] == playingField[row][1] && playingField[0][1] == playingField[row][2]; 
}

public static boolean isWinningColumn(int column) {
    return playingField[0][column] == playingField[1][column] && playingField[1][column] == playingField[2][column]; 
}

public static boolean isLeftRightDiagonalWin() {
    return playingField[0][0] == playingField[1][1] && playingField[1][1] == playingField[2][2]; 
}

public static boolean isRightLeftDiagonalWin() {
    return playingField[0][2] == playingField[1][1] && playingField[1][1] == playingField[2][0]; 
}

This does not simplify the overall complexity, but does improve the readability:

return isWinningRow(0)
        || isWinningRow(1)
        || isWinningRow(2)
        || isWinningColumn(0)
        || isWinningColumn(1)
        || isWinningColumn(2)
        || isLeftRightDiagonalWin()
        || isRightLeftDiagonalWin();

So you could rewrite your logic to something like this:

int turn = 0;
boolean running = true;

while (turn < 9 && running) {
    int currentPlayer = (turn % 2) + 1;
    
    playerChoice(currentPlayer);
    printPlayingField();
    
    if (hasWon()) {
        System.out.println("Winner: " + Integer.toString(currentPlayer);
        
        running = false;
    }
    
    turn++;
}

I'm a little bit unhappy with the code to choose the player, but it should work. And overall should give you a good idea where to start.

Answer 2

Adding to Bobby's very good answer, you could also explore alternative ways for checking the winning game.

You can think of noughts and crosses as 1s and 0s (true and false):

boolean[][] board = new boolean[][]{
     {false, false, false},
     {false, true, false},
     {false, false, true}
};

and therefore transform the board to an integer number:

int intboard = 0;
int exponent = 0;
for (int i = 0; i < board.length; i++) {
    for (int j = 0; j < board[0].length; j++) {
        intboard += board[i][j] ? Math.pow(2, exponent) : 0;
        exponent++;
    }
}

Then you can simply check intboard against a set of pre-calculated integers for all winning boards:

List<Integer> winningBoards = List.of(
        7, // Horizontal, row-0 --> 000000111
        56, // Horizontal, row-1 --> 7 << 3
        448, // Horizontal, row-2 --> 7 << 6
        73, // Vertical, column-0 --> 001001001
        146, // Vertical, column-1 --> 73 << 1
        292, // Vertical, column-2 --> 73 << 2
        273, // Diagonal-0 --> 100010001
        84 // Diagonal-1 --> (273 >> 2) | 16
);

Note that you can calculate some of these winning boards using bitwise operations too!

Finally checking for the winning cases is a matter of applying the winningBoards masks on intboard:

final int boardToCheck = intboard;
boolean youHaveWon = winningBoards.stream()
        .anyMatch(winningboard -> (boardToCheck & winningboard) == winningboard);

System.out.println("Nought has won: " + youHaveWon);

If you want to check for crosses you simply need to flip the bits of the winning boards using the complement operator ~:

youHaveWon = winningBoards.stream()
        .anyMatch(winningboard -> (~boardToCheck & winningboard) == winningboard);

System.out.println("Cross has won: " + youHaveWon);

The advantage of this technique is not clear for a game as simple as noughts and crosses, but if you are working on more complex games and need to check many many boards (even millions or billions!) when performing Minimax or equivalent, you can get a very nice speed up by changing the representation of your game from using objects to bits.