A basic game of Tic-Tac-Toe

Question

import java.util.Scanner;


public class ticTacToe {

    public static Scanner input;


    public static void main(String[] args) {
        input = new Scanner(System.in);
        char p1 = ' ';
        char p2 = ' ';
        boolean gameOver = false;
        char[][] board = new char[3][3];
        int pos1 = 0, pos2 = 0;

        // Welcome the user to the game
        System.out.println("----- Welcome to Tic-Tac-Toe! -----");
        System.out.println("Would you like to play as x's or o's?");
        p1 = input.next().charAt(0);
        System.out.println("Would you like to go first or first?");
        String first = input.next();

        while (checkLetter(p1) == false) {
            System.out.println("Invalid input. Please choose either x or o.");
            p1 = input.next().charAt(0);
        }
        p2 = setCompLetter(p1);

        // game loop
        while (gameOver == false) {
            // Player 1 turn
            player1(p1, p2, board);
            // Check P1 victory
            if (checkP1Victory(board, p1) == true) {
                gameOver = true;
                System.out.println("Player 1 wins");
                break;
            }
            // Player 2 turn
            player2(p1, p2, board);
            // Check p2 victory
            if (checkP2Victory(board, p2) == true) {
                gameOver = true;
                System.out.println("Player 2 wins!");
                break;
            }
            // Check for draw
            if(checkDraw(p1, p2, board) == true){
                System.out.println("It is a draw!");
                break;
            }

        }

    }

    // Checks to see if the user input is valid when they select a letter
    public static boolean checkLetter(char letter) {
        if (letter != 'x' && letter != 'o') {
            return false;
        }
        return true;
    }

    // Sets the computer letter based on the user input
    public static char setCompLetter(char p1) {
        if (p1 == 'x') {
            return 'o';
        } else {
            return 'x';
        }

    }

    // Prints the board to the console
    public static void printBoard(char[][] board) {
        for (int i = 0; i < board.length; i++) {
            for (int j = 0; j < board[i].length; j++) {
                System.out.print("[ " + board[i][j] + " ]");
            }
            System.out.println();
        }
    }

    // Player 1 turn sequence
    public static void player1(char p1, char p2, char[][] board) {
        //System.out.println("Player 1 turn.");
        System.out.println("Enter the position: ");
        int pos1 = input.nextInt();
        int pos2 = input.nextInt();

        // Checks to see if the user entered a position that is in bounds
        while ((pos1 >= 3) || (pos2 >= 3)) {
            System.out.println("Out of bounds index, re-enter a position");
            pos1 = input.nextInt();
            pos2 = input.nextInt();
        }

        // Checks to see if there is an x or o in the position
        while ((board[pos1][pos2] == p1) || (board[pos1][pos2] == p2)) {
            System.out.println("Already something in that position, enter another.");
            pos1 = input.nextInt();
            pos2 = input.nextInt();
        }
        board[pos1][pos2] = p1;
        printBoard(board);
    }

    // Player 2(CPU) sequence
    public static void player2(char p1, char p2, char[][] board) {
        int pos1 = (int) (Math.random() * 3);
        int pos2 = (int) (Math.random() * 3);
        while ((board[pos1][pos2] == p1) || (board[pos1][pos2] == p2)) {
                pos1 = (int) (Math.random() * 3);
                pos2 = (int) (Math.random() * 3);
            }
        board[pos1][pos2] = p2;
        System.out.println("");
        printBoard(board);
    }
    // Checks the p1 victory
    public static boolean checkP1Victory(char[][] board, char p1) {
        // Checks player 1 row victory
        for (int i = 0; i < board.length; i++) {
            if ((board[i][0] == p1) && (board[i][1] == p1) && (board[i][2] == p1)) {
                return true;
            }
        }
        // Checks Player 1 verticle victory
        for (int j = 0; j < board.length; j++) {
            if ((board[0][j] == p1) && (board[1][j] == p1) && (board[2][j] == p1)) {
                return true;
            }
        }
        // Checks Player 1 diagonal victory(left to right)
        if ((board[0][0] == p1) && (board[1][1] == p1) && (board[2][2] == p1)) {
            return true;
        }
        // Checks Player 1 diagonal victory(right to left)
        if ((board[0][2] == p1) && (board[1][1] == p1) && (board[2][0] == p1)) {
            return true;
        } else {
            return false;
        }
    }

    public static boolean checkP2Victory(char[][] board, char p2) {
        // Checks player 2 row victory
        for (int i = 0; i < board.length; i++) {
            if ((board[i][0] == p2) && (board[i][1] == p2) && (board[i][2] == p2)) {
                return true;
            }
        }
        // Checks Player 2 verticle victory
        for (int j = 0; j < board.length; j++) {
            if ((board[0][j] == p2) && (board[1][j] == p2) && (board[2][j] == p2)) {
                return true;
            }
        }
        // Checks Player 2 diagonal victory(left to right)
        if ((board[0][0] == p2) && (board[1][1] == p2) && (board[2][2] == p2)) {
            return true;
        }
        // Checks Player 2 diagonal victory(right to left)
        if ((board[0][2] == p2) && (board[1][1] == p2) && (board[2][0] == p2)) {
            return true;
        } else {
            return false;
        }
    }
    public static boolean checkDraw(char p1, char p2, char[][] board){
        int count = 0;
        for(int i = 0; i < board.length; i++){
            for(int j = 0; j < board[i].length; j++){
                if(board[i][j] == 'x' || board[i][j] == 'o' ){
                    count++;
                    if(count == 9){
                        return true;
                    }
                }
            }
        }
        return false;
    }

}

Answer 1

Use conventional names.

Class names in Java use the CamelCaps notation, i.e. the class name should be TicTacToe, not ticTacToe.

Encapsulate / hide fields, and consider making them final.

Your field input is public, which means every other class could access it. It's not final, which means it could be changed from anywhere in the code. You might consider using private static final Scanner input = new Scanner(System.in); instead.

Encapsulate / hide methods

All of your methods are public, but the way they are written, they should (apart from unit tests) never be accessed from anywhere else. In such a case, making them public might not make sense.

Consider using OO

All your data is passed around between methods via arguments. Consider creating an instance of your TicTacToe class and turning parameters which are passed around frequently, like the board, into instance fields.

The input validation in player1() is bogus

I can enter a position which is out-of-bounds by first entering a position at which there already is something, and then enter a position which is out-of-bounds.

I can also enter a position which is out-of-bounds and crashes the program by entering negative numbers.

Spelling

vertical is neither a beer or body part ;) (change verticale and verticle to vertical).

Redundant code

Did you notice that the contents of checkP1Victory() and checkP2Victory() are identical? You need only one of these two methods, and can make it generic for any player. This also seems more logical from the point of view of tic tac toe rules: The rules for winning are the same for player1 and player2.

Extract methods

In checkP1Victory() / checkP2Victory(), I'd extract 3 methods. I'd extract the first loop into checkHorizontalVictory(), the second loop into checkVerticalVictory(), and the two if statements into checkDiagonalVictory(). This would simplify the code significantly, and you could get rid of comments.

Avoid check in method names.

check is a bad word to use in methods. There is a concept called CQS - Command-Query-Separation that we want to follow with methods. Methods should be either commands or queries but not both. Commands are void methods that don't return anything, but they have side-effects. Queries return something but usually do not have side-effects. The name of a method should tell whether it is a command or a query, and with check that's problematic. It sounds like a command, but in your case it is a query.

How about hasPlayerWon instead of checkPlayerVictory() (resp. checkP1Victory() / checkP2Victory())?

Notice how nice that reads in if-statements:

if (hasPlayerWon(p2)) { ... }

Food for thought: If you go for a little more OO and delegation, you could even end up with code which reads as nicely as:

if (player2.hasWon(game)) { ... }

Truth doesn't need to be compared

Instead of while (gameOver == false) use while (!gameOver). Instead of if (xyz == true) use if (xyz).

Unused variables and values.

In method main(), the variables pos1 and pos2 are unused and can be removed.

In method main(), the initial values ' ' of variables p1 and p2 are unused and can be removed.

In method main(), the variable first is unused, and because it's initialized from input.next(), I believe it's a bug.

Method length

Consider splitting method main(), it is very long. Good method lenghts in Java are 1-3 lines, sometimes a little bit longer, but hardly ever more than 6-7 lines.

Consider changing or even removing checkDraw method.

This is a suggestion to the algorithm. Currently you determine whether the game is a draw by detecting that the all 9 fields of the board contain either 'x' or 'o'.

You could also find out the inverse faster, by finding out that one of the fields is not 'x' or 'o'. Then you would not have to inspect the entire board, but could stop looking after you found the first ' '. It would also get rid of the count variable.

Alternatively, you could also count the number of moves in the main() method. If it reaches 9, it's a draw.

Consider using constants.

Mistyping 'x' as 'X' or 'o' as 'O' or '0' is all too easy and can lead to long long debugging sessions if you do not have unit tests (and it seems you do not have unit tests).

You could introduce constants

private static final char PLAYER1 = 'x';
private static final char PLAYER2 = 'o';

Then replace all other occurrences of 'x' and 'o' with PLAYER1 and PLAYER2. This can make the code easier to read, and it also makes the code easier to maintain in case your requirements change, i.e. because your customer / client / teacher / whoever wants to have something else instead of 'x' and 'o'.

Consider using an enum instead of char for your board.

Your board fields can only ever have three valid states: * ' ' * 'o' * 'x' However, the type char allows not 3 but 65536 different states, leaving each field potentially with 65533 invalid states. Parts of your code will inevitably be busy with preventing these 65533 invalid states from ever happening.

An enum is a class with a predefined set of instances. Here are two options:

1. You could have an enum with two values, and represent the empty field with null.
2. You could have an enum with three values, one of them being the empty field.

Here's the enum for the first option:

enum Player {
    PLAYER1('x'),
    PLAYER2('o');
    private final char displayChar;
    Player(char displayChar) {
        this.displayChar = displayChar;
    }
    public char getChar() {
        return displayChar;
    }
}

Both approaches have pros and cons, and enum might be new to you, so I'm not sure whether you really want to consider it already. But if you do, or even in the failed attempt of doing so, you might actually learn a lot, so it might be worth trying.

SRP violations

The SRP is the Single Responsibility Principle, and your methods violate it: They contain I/O and business logic at the same time. I/O and business logic are separate concerns and thus should be separated from each other. Imagine you want to create a graphical version, much of your code couldn't be reused without modification. And it turns out that reusable is a synonym to testable, and both are a subset of maintainable.

More Ideas for playing with the code further to learn more.

• Allow human vs human and pc vs pc games.
• Allow players to pick their symbols.
• Split your program into multiple classes.

Consider unit testing, consider TDD

Title says it all.

Answer 2

It looks like you are well aware of your IDEs formatting feature - the code is very well formatted.

DRY

checkP1Victory and checkP2Victory can be condensed into a single function, checkVictory. The logic is exactly the same, the only difference is the player token being checked.

Conditionals

Favor using the not (!) operator over checking explicitly for false.

Printing

Favor building strings and only using System.out.println on the result of a method call. For example, printBoard should return a String. Use a StringBuilder.

Separate concerns

Try to extract the idea of a TicTacToe game out into a class. Don't have any I/O in the class itself (no prints!) - you should be able to create a Driver (main class) that uses this TicTacToe game. The driver could use plain console I/O, like it is now, or possibly use a GUI approach. It could even simulate a game between two computers. Separate the logic from the input method, and you'll have more robust code.

Enum

An enumeration would be a better choice for marking cells, instead of using a character. You would have X, O, and then Empty. There is a good argument to be made to use something like PlayerOne, PlayerTwo and Empty, since it is the driver which would control how things are displayed. Perhaps your client wants to use Dogs and Cats, not Xs and Os.