Tic-Tac-Toe in Java

Question

I am currently learning Java; I could really use some help from more experienced programmers.

How can I clean up my Tic-Tac-Toe code? What mistakes have I made? Can I use more exception handling? I tried to use exception handling, in isValidStepCheck and takenFieldCheck, but I don't think I used it efficiently.

public class TicTacToe {
    public static String[] bpos = {" 1 ", " 2 ", " 3 ", " 4 ", " 5 ", " 6 ", " 7 ", " 8 ", " 9 "};
    public static int turn = 0;
    public static int num;
    public static String numS;
    public static String winner = "n";
    public static int r = 0;

    public static void main(String[] args) {
        System.out.println("Welcome to the Tic-Tac-Toe game.");
        Board();
        System.out.println("X has the first step, enter a number.");
        try{TicTacToeGame();}
        catch (Exception e){
            System.out.println("Something went wrong. Are you sure you entered a number?");
        }
    }


    static boolean isValidStepCheck() {
        while (true) {
            Scanner sc = new Scanner(System.in);
            num = sc.nextInt();
            numToString();
            if (num < 1 || num > 9) {
                System.out.println("You have to enter a number between 1 and 9!");
            } else {
                return true;
            }
        }
    }

    static void isValidMove() {
        while (true) {
            if (isValidStepCheck() && !takenFieldCheck()) {
                if (turn % 2 == 0) {
                    turn++;
                    bpos[num - 1] = " x ";
                    Board();
                    break;
                } else if (turn % 2 == 1) {
                    turn++;
                    bpos[num - 1] = " o ";
                    Board();
                    break;
                }
            } else if (takenFieldCheck()) {
                System.out.println("This field is already taken!");
            }

        }
    }

    static void Board() {
        System.out.println(" ___________ ");
        System.out.println("|" + bpos[0] + "|" + bpos[1] + "|" + bpos[2] + "|");
        System.out.println("|-----------|");
        System.out.println("|" + bpos[3] + "|" + bpos[4] + "|" + bpos[5] + "|");
        System.out.println("|-----------|");
        System.out.println("|" + bpos[6] + "|" + bpos[7] + "|" + bpos[8] + "|");
        System.out.println(" ----------- ");
    }

    static boolean takenFieldCheck() {
        if (bpos[num - 1].equals(numS)) {
            return false;
        } else {
            return true;
        }
    }

    static void numToString() {
        switch (num) {
            case 1:
                numS = " 1 ";
                break;
            case 2:
                numS = " 2 ";
                break;
            case 3:
                numS = " 3 ";
                break;
            case 4:
                numS = " 4 ";
                break;
            case 5:
                numS = " 5 ";
                break;
            case 6:
                numS = " 6 ";
                break;
            case 7:
                numS = " 7 ";
                break;
            case 8:
                numS = " 8 ";
                break;
            case 9:
                numS = " 9 ";
                break;
        }
    }

    static void isWon() {
        String line = null;
        if (winner.equals("n")) {
            for (int i = 0; i < 8; i++) {
                switch (i) {
                    case 0:
                        line = bpos[0] + bpos[1] + bpos[2];
                        break;
                    case 1:
                        line = bpos[3] + bpos[4] + bpos[5];
                        break;
                    case 2:
                        line = bpos[6] + bpos[7] + bpos[8];
                        break;
                    case 3:
                        line = bpos[0] + bpos[3] + bpos[6];
                        break;
                    case 4:
                        line = bpos[1] + bpos[4] + bpos[7];
                        break;
                    case 5:
                        line = bpos[2] + bpos[5] + bpos[8];
                        break;
                    case 6:
                        line = bpos[0] + bpos[4] + bpos[8];
                        break;
                    case 7:
                        line = bpos[2] + bpos[4] + bpos[6];
                        break;
                }
                if (line.equals(" x  x  x ")) {
                    winner = "x";
                } else if (line.equals(" o  o  o ")) {
                    winner = "o";
                }
            }

        } else {
            winner = "n";
        }
    }

    static void TicTacToeGame() {
        while (true) {
            isValidMove();
            isWon();
            r++;
            if (winner.equals("x")) {
                System.out.println("Congrats to X, you won!");
                break;
            } else if (winner.equals("o")) {
                System.out.println("Congrats to O, you won!");
                break;
            }
            else {
                if(r==9){
                    System.out.println("Its a draw!");
                    break;
                }
            }
        }
    }
} 
```

Answer 1

Just a few quick things that jump out at me (I didn't go over all of it):

Using static for everything is awkward. Usually you want to avoid storing state in statics. I know this is a simple app without multiple classes so you can get away with it here. But following more typical rules of thumb, you would make all these variables and methods non-static.

If you see yourself using if/else to simply return true or false, you can almost always remove the if/else.

static boolean takenFieldCheck() {
    return !bpos[num - 1].equals(numS);
}

This method could be made much shorter like this:

static void numToString() {
    numS = " " + num + " ";
}

However, you only use numS in one spot. It's error prone to keep a copy of your game state in two different forms, because then you have to be absolutely sure you always keep them in sync. I would remove numS and change the method to this:

static String numAsString() {
    return " " + num + " ";
}

And use it in the one place you were comparing something to numS.

It would be better practice to catch your Exceptions from Scanner right from where they are thrown because then you can show the proper error message right there and let the user recover. Your top-level catch-all block doesn't really know what kind of exception was thrown, so it could be showing the wrong message. Not really in the case of this very simple game, though.

You're checking for a number out of range and let the user recover from that. Why not let them recover from anything, like this? Also, there's no reason for this method to return a Boolean, since it cannot possibly return false. I would rename it to describe what it actually does.

static void receiveValidMove() { // was isValidStepCheck
    Scanner sc = new Scanner(System.in);
    while (true) {
        int newNumber;
        try {
            newNumber = sc.nextInt();
        } catch (Exception e) {
            newNumber = -1; // input was invalid
        }
        if (number >= 1 && number <= 9){
            num = newNumber;
            numToString(); // omit if following above advice
            return;
        }
        System.out.println("You have to enter a number between 1 and 9!");
    }
    
}

Answer 2

Naming is important

When you're in the flow and writing code you'll typically remember what variables refer to and you'll often be able to hold the whole code context in your head, particularly with relatively small examples like this. However, as the code gets bigger / after you've stepped away from it for a while having good names really makes the code more approachable. bpos isn't really a meaningful name. It looks like some kind of abbreviation / acronym, possibly boardPosition? From the way you are using it, it looks like it's actually the state of the game. Perhaps gameBoard might be more descriptive. r is even less descriptive. One letter variables, if you want to use them, should really be confined to local variables, in code that fits on a typical screen. This reduced the impact of lost context. It's also a lot easier to search your codebase for something like roundNumber than r.

No, really, naming is important...

When developers approach your code, they bring expectations from other code that they've previously seen. You can obviously introduce your own approaches, but consider if they're actually adding anything. When I see methods that start with 'is' or 'has', I'm expecting them to return booleans. When you read code that's been written like this, it helps readability because it often comes out closer to natural language. So you might have something like:

if(isValidMove(position)) {
    makeMove(position);
} else {...}

Your isValidMove and isWon methods break this expectation. As has been pointed out by @Tenfour04 your other method isValidStepCheck kind of satisfies this, however it always returns true.

Generally methods in Java should follow camelCase and describe what they do, so instead of Board, consider printBoard.

Keep an eye out for redundancy

You isWon method contains:

if (winner.equals("n")) {
  // ...
} else {
    winner = "n";
}

If the if fails, then it's because winner is already "n", so there's no need to assign it in the else. In the same area, consider if you need to check if all three positions are 'x', or all three positions are 'y'. Would it simply be enough to consider if all three positions are the same?

There's a similar issue in isValidMove:

if (turn % 2 == 0) {
    turn++;
    bpos[num - 1] = " x ";
    Board();
    break;
} else if (turn % 2 == 1) {
    turn++;
    bpos[num - 1] = " o ";
    Board();
    break;
}

There's a couple of issue... the only difference between the two if clauses is that one is assigning " o " and the other " x ". If who was moving was worked out first, then you could simplify the code. You also don't need the second if condition. If turn % 2 isn't '0', then it has to be '1', you don't need to check for it.