Tic Tac Toe game in Java

Question

I've recently started programming and been self-studying on various online courses + official documentation of oracle / java. Code is quite messy and if you don't have time to understand it, I wouldn't even bother in your place. This is also not Object Oriented since I didn't see a reason to do it that way although I will later on try it as well. GUI is something I'll also do later on but I'd love to have the basics down first.

This is basically my first somewhat bigger application. I've also done a person management. Both took me about 3 hours. Is that reasonable or like way to bad?

The program works as follows:

First you get a menu, where you can chose what to do (I only coded 1 so far) so in this case you can play against a bot. You have the first turn, you get the board printed with the respective quadrants and then you can choose into which quadrant you will place your X by typing in 1-9. It also checks whether that quadrant has already been used or if your input type is wrong. Then the bot does his turn by simply generating a random number which represents the quadrant and then there is the same check if it has been used already. This continues until either wins.

I hope someone can take some time and analyze what I can do, to heavily improve.

import java.util.Scanner;

public class TickTack {

    public static void main(String[] args) {

        // This isn't really OOP and I should probably do that aswell later when
        // I finish this
        // Possible construction of OOP
        // Class : TickTackToe
        // Methods : constructor, findMatchingItem(), printCurrentBoard(),
        // testUserInput(),

        final int ROWS = 3;
        final int COLUMNS = 3;
        int filler = 2;
        Scanner in = new Scanner(System.in);
        int choice = 0;
        // winnerDecided is Variable to play game until a Winner is decided ->
        // draws don't count
        boolean winnerDecided = false;

        // Matrix
        //
        // 1 | 2 | 3
        // 4 | 5 | 6
        // 7 | 8 | 9

        int[][] tickTackToeFields = new int[ROWS][COLUMNS];

        // int[] arrayPlayerOne = new int[5];
        // int[] arrayPlayerTwo = new int[5];

        do {
            System.out.println("What do you want to do?");
            System.out.println(" 1. Play against Bot");
            // ToDo actually a decent bot not random generated
            // difficulties?
            System.out.println(" 2. Play against Human");
            // Bo3 or Bo5
            System.out.println(" 3. Stop Programm");
            System.out.println(""); // seperator
            choice = in.nextInt();
            System.out.println(""); // seperator
            switch (choice) {
            case 1:

                // Mechanism to Fill array with numbers indicating each quadrant
                // of the
                // Matrix
                // Dunno if int[][] tickTackToeFields = { {1,2,3} , {4,5,6] ,
                // {7,8,9} }
                // would even work
                for (int i = 0; i < ROWS; i++) {
                    for (int j = 0; j < COLUMNS; j++) {
                        // Filler Variable goes from 2-10 to Fill the Array with
                        // 2-10 --> As to why is mentionend at the printBoard
                        // method below
                        tickTackToeFields[i][j] = filler;
                        filler++;
                    }
                }
                // USER CHOICE VARIABLE FOR THE QUADRANTS
                int userChoiceInt = 0;
                // i is mechanism to choose which Players turn it is
                int i = 1;
                do {

                    printBoard(ROWS, COLUMNS, tickTackToeFields);

                    System.out.println(""); // seperator

                    if (i % 2 == 0 && i < 10) {
                        System.out.println("\nBot is choosing (O)");
                        // see if Field has already been used
                        boolean alreadyUsed = true;
                        do {
                            alreadyUsed = true;
                            // generating a random number then casting it to an
                            // int might use ROUND here since 9 is never gonna
                            // be used I guess?
                            userChoiceInt = (int) (1 + Math.round(Math.random() * 8));
                            // userChoiceInt = 1 + (int)(Math.random()*8);

                            for (int p = 0; p < ROWS; p++) {
                                for (int g = 0; g < COLUMNS; g++) {
                                    // don't forget -1 since it's 1 higher
                                    // explanation above
                                    // this is obviously botChoiceInt but why
                                    // would
                                    // you make a new Variable to waste
                                    // resources?
                                    if (tickTackToeFields[p][g] - 1 == userChoiceInt) {
                                        alreadyUsed = false;
                                    }
                                }
                            }

                        } while (alreadyUsed);

                        // actually doing the insert into the array
                        for (int p = 0; p < ROWS; p++) {
                            for (int g = 0; g < COLUMNS; g++) {
                                if (tickTackToeFields[p][g] - 1 == userChoiceInt) {
                                    tickTackToeFields[p][g] = 1;
                                }
                            }
                        }

                    } else if (i % 2 == 1 && i < 10) {
                        System.out.println("\nPlayer 1 is chosing (X) --> choose between 1-9");

                        boolean alreadyUsed = true;

                        // Check if user Input is correct and if it's not
                        // already used so you can't cheat
                        do {
                            // CHECKING IF USERINPUT CAN BE USED
                            try {
                                // when I used nextInt without the converting,
                                // and typed a letter or generally not a number
                                // it ended in a loop for some reason??
                                userChoiceInt = Integer.parseInt(in.next());
                            } catch (Exception e) {
                                System.out.println("Use a number between 1-9 Exception");
                                // so the whole thing restarts are same user
                                // without actually doing anything
                                // i--;

                            }

                            for (int p = 0; p < ROWS; p++) {
                                for (int g = 0; g < COLUMNS; g++) {
                                    // don't forget -1 since it's 1 higher
                                    // explanation above
                                    if (tickTackToeFields[p][g] - 1 == userChoiceInt) {
                                        alreadyUsed = false;
                                    }
                                }
                            }

                        } while (alreadyUsed);

                        // actually doing the insert into the array
                        for (int p = 0; p < ROWS; p++) {
                            for (int g = 0; g < COLUMNS; g++) {
                                if (tickTackToeFields[p][g] - 1 == userChoiceInt) {
                                    tickTackToeFields[p][g] = 0;
                                }
                            }
                        }
                    }

                    i++;

                    // I don't know how to make this easier, here it actually
                    // looks if you've 3 consecutive hits
                    // diagonal,horizontal,vertical I guess you could just check
                    // if they are equal but it's the same
                    if (tickTackToeFields[0][0] + tickTackToeFields[0][1] + tickTackToeFields[0][2] == 3
                            || tickTackToeFields[1][0] + tickTackToeFields[1][1] + tickTackToeFields[1][2] == 3
                            || tickTackToeFields[2][0] + tickTackToeFields[2][1] + tickTackToeFields[2][2] == 3
                            || tickTackToeFields[0][0] + tickTackToeFields[1][0] + tickTackToeFields[2][0] == 3
                            || tickTackToeFields[0][1] + tickTackToeFields[1][1] + tickTackToeFields[2][1] == 3
                            || tickTackToeFields[0][2] + tickTackToeFields[1][2] + tickTackToeFields[2][2] == 3
                            || tickTackToeFields[0][0] + tickTackToeFields[1][1] + tickTackToeFields[2][2] == 3
                            || tickTackToeFields[2][0] + tickTackToeFields[1][1] + tickTackToeFields[0][2] == 3) {
                        winnerDecided = true;

                        printBoard(ROWS, COLUMNS, tickTackToeFields);

                        System.out.println("\nBOT WON!!!!! CONGRATULATIONS\n");
                    } else if (tickTackToeFields[0][0] + tickTackToeFields[0][1] + tickTackToeFields[0][2] == 0
                            || tickTackToeFields[1][0] + tickTackToeFields[1][1] + tickTackToeFields[1][2] == 0
                            || tickTackToeFields[2][0] + tickTackToeFields[2][1] + tickTackToeFields[2][2] == 0
                            || tickTackToeFields[0][0] + tickTackToeFields[1][0] + tickTackToeFields[2][0] == 0
                            || tickTackToeFields[0][1] + tickTackToeFields[1][1] + tickTackToeFields[2][1] == 0
                            || tickTackToeFields[0][2] + tickTackToeFields[1][2] + tickTackToeFields[2][2] == 0
                            || tickTackToeFields[0][0] + tickTackToeFields[1][1] + tickTackToeFields[2][2] == 0
                            || tickTackToeFields[2][0] + tickTackToeFields[1][1] + tickTackToeFields[0][2] == 0) {
                        winnerDecided = true;

                        printBoard(ROWS, COLUMNS, tickTackToeFields);

                        System.out.println("\nPLAYER 1 WON!!!!! CONGRATULATIONS\n");
                    }

                    // ask user if they want to stop or not
                    if (winnerDecided != true && i == 10) {
                        System.out.println("Do you want to continue or stop? Y / N");
                        String userChoiceString = in.next();
                        // toLowerCase will ensure that any user choice will be
                        // accepted, includes yEs yES y ye es etc.. think this
                        // is the best solution
                        if ("yes".contains(userChoiceString.toLowerCase())) {
                            winnerDecided = true;
                        } else if ("no".contains(userChoiceString.toLowerCase())) {
                            winnerDecided = false;
                        } else {
                            System.out.println("Instructions unclear - going back to main Menu");
                            winnerDecided = true;
                        }
                    }

                } while (!winnerDecided);

                break;
            case 2:
                break;
            case 3:
                break;
            }

        } while (choice != 3);

        in.close();
    }

    public static void printBoard(int Rows, int Column, int Array1[][]) {
        for (int k = 0; k < Rows; k++) {
            for (int j = 0; j < Column; j++) {
                if ((k == 1 || k == 2) && j == 0) {
                    System.out.print("---+---+---");
                    System.out.println("");
                }

                if (Array1[k][j] == 0) {
                    System.out.print(" X");
                } else if (Array1[k][j] == 1) {
                    System.out.print(" O");
                } else {
                    // This is actually very weird, could be
                    // done
                    // alot more clever than this I think
                    // I had to fill Array with Numbers greater
                    // then
                    // 1 since the User choices will be 1 and 0
                    // depending on the answer
                    // And then I check the Array if it already
                    // has
                    // a 1 or 0 in it
                    // This wouldn't work if I had filled it
                    // with
                    // numbers from 1-9 it would say that the
                    // first
                    // quadrant has been used
                    // hope this somewhat explains my weird
                    // thought
                    // process in here - and sorry for bad
                    // explanation probably should've just used a random high
                    // number for user choices or constants
                    System.out.print(" " + (Array1[k][j] - 1));
                }

                if (j < 2) {
                    System.out.print(" |");
                }

            }
            System.out.println(""); // just a new line seperator
                                    // Maybe make a function
                                    // that
                                    // just makes a new line?
        }
    }

}

Answer 1

general coding approach

Your approach to the problem is purely procedural. This is not a problem as such.

But Java is a class based object oriented language.

This means you are encouraged to solve problems by using classes, objects and polymorphism. You should start with that as soon as possible. Otherwise you should ask yourself if Java is the right programming language for you.

detailed critic

comments

Your program contains lots of comments. Some of them are good because they describe why your code is like it is but most of your comments either repeat what the code already expresses or is there to cover a bad named identifier.

You also have comments to deactivate code. Don't do that. Use a Sourcecode Management System (like git or svn).

magic numbers

Your code contains some magic numbers which should be extracted to constants. E.g. when you detect the winner you compare with 0 and 3 which could be changed to

 static final int PLAYER = 0;
 static final int BOT = 3;

separation of concerns

As every program yours has (at least) 3 big parts: input, processing and output.

You created a separate method for the output, which is good. You should also have created a separate method to handle the user input.

Within your game logic there are also some parts which could have been separated to their own method, e.g.: the initialization and the content of the main loop.

naming

The hardest part in programming is finding good names. When ever you feel you should add a comment to explain a variable or a method take another 5 minutes to think of a better name that could make the comment obsolete.
E.g. your variable winnerDecided could better be named isGameWon and your variable i could be named playerSelector.

Avoid single letter variables and abbreviations. But if you chose single letter names use common ones. In your loops your iteration counters are g and p where i and j or x and y would be much more common.

Some of your identifiers have the type in their name (e.g. userChoiceInt). Don't do that, the compiler knows of the variables type and will assist you.

ROWS and COLUMNS are variables within the scope of the method main (and technically not constants). Therefor they should be spelled like variables (camelcase starting lowercase).
On the other hand you may have meant them to be constants. In this case you should move them between the start of the class and the first method (main in this case) and add the static keyword too so that the compiler also knows that this are constants.

Let boolean variables (and methods) start with is or has. (while (alreadyUsed); vs while (isAlreadyUsed);)

code douplication

your logic to detect the winner is a the same block of code, except the number you compare. the content of the if could be extracted to a method getting the number to compare as parameter and returning a boolean:

private static boolean isWinner(int player, int[][]  tickTackToeFields){
   return tickTackToeFields[0][0] + tickTackToeFields[0][1] + tickTackToeFields[0][2] == player
          || tickTackToeFields[1][0] + tickTackToeFields[1][1] + tickTackToeFields[1][2] == player
          || tickTackToeFields[2][0] + tickTackToeFields[2][1] + tickTackToeFields[2][2] == player
          || tickTackToeFields[0][0] + tickTackToeFields[1][0] + tickTackToeFields[2][0] == player
          || tickTackToeFields[0][1] + tickTackToeFields[1][1] + tickTackToeFields[2][1] == player
          || tickTackToeFields[0][2] + tickTackToeFields[1][2] + tickTackToeFields[2][2] == player
          || tickTackToeFields[0][0] + tickTackToeFields[1][1] + tickTackToeFields[2][2] == player
          || tickTackToeFields[2][0] + tickTackToeFields[1][1] + tickTackToeFields[0][2] == player;
}

Of cause we could simplify this a little more if we introduce some more parameterized methods:

private static int getRowOwner(int rowIndex, int[][] tickTackToeFields){
   int lineOwner =0;
   for(int linePosition=0;  linePosition<ROWS; linePosition++){
      lineOwner += tickTackToeFields[rowIndex, linePosition];
   }
   return lineOwner;
}
private static int getColumnOwner(int columnIndex, int[][] tickTackToeFields){
   int lineOwner =0;
   for(int linePosition=0;  linePosition<ROWS; linePosition++){
      lineOwner += tickTackToeFields[linePosition,columnIndex];
   }
   return lineOwner;
}
private static int getDiagonalOwner(int startColumn, int columnStep, int[][] tickTackToeFields){
   int lineOwner =0;
   for(int linePosition=0;  linePosition<ROWS; linePosition++){
      lineOwner += tickTackToeFields[linePosition,startColumn+ linePosition*columnStep];
   }
   return lineOwner;
}

Then the previous method changes to:

private static boolean isWinner(int player, int[][]  tickTackToeFields){
  // should be constants too:
  final int fromCell0x0=0;
  final int toRight = 1;
  final int fromCell0x2=2;
  final int toLeft = -1;
  return player == getRowOwner(0, tickTackToeFields)
      || player == getRowOwner(1, tickTackToeFields)
      || player == getRowOwner(2, tickTackToeFields)
      || player == getColumnOwner(0, tickTackToeFields)
      || player == getColumnOwner(1, tickTackToeFields)
      || player == getColumnOwner(2, tickTackToeFields)
      || player == getDiagonalOwner(fromCell0x0,toRight, tickTackToeFields)
      || player == getDiagonalOwner(fromCell0x2,toLeft, tickTackToeFields);

The part where you have the logic now would change to:

 if (isWinner(3, tickTackToeFields))
   winnerDecided = true;
   printBoard(ROWS, COLUMNS, tickTackToeFields);
   System.out.println("\nBOT WON!!!!! CONGRATULATIONS\n");
 } else if (isWinner(0, tickTackToeFields))
   winnerDecided = true;
   printBoard(ROWS, COLUMNS, tickTackToeFields);
   System.out.println("\nPLAYER 1 WON!!!!! CONGRATULATIONS\n");
 }

And if we also change the magic numbers to constants it would change to this:

 if (isWinner(BOT, tickTackToeFields))
   winnerDecided = true;
   printBoard(ROWS, COLUMNS, tickTackToeFields);
   System.out.println("\nBOT WON!!!!! CONGRATULATIONS\n");
 } else if (isWinner(PLAYER, tickTackToeFields))
   winnerDecided = true;
   printBoard(ROWS, COLUMNS, tickTackToeFields);
   System.out.println("\nPLAYER 1 WON!!!!! CONGRATULATIONS\n");
 }