7
\$\begingroup\$

I've been learning Java for 4 weeks now twice a week and I wanted to test myself to see what I could do. I'd love to see what you think and how I can make it better.

public static void main(String[] args) {

    Scanner keyboard = new Scanner(System.in);
    char a1 = ' ';
    char a2 = ' ';
    char a3 = ' ';
    char b1 = ' ';
    char b2 = ' ';
    char b3 = ' ';
    char c1 = ' ';
    char c2 = ' ';
    char c3 = ' ';
    int turn = 1;
    int y = 0;

    while (y <= 9) {
        if (turn == 1) {
            System.out.println("player 1 it is your turn. your spot you wou"
                    + "ld like to enter.\n Ex: top,botton,middle left,right,middle");
            String input = keyboard.nextLine();

            if (input.equals("top left")) {
                if (a1 == ' ') {
                    a1 = 'x';
                    turn++;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("top middle")) {
                if (b1 == ' ') {
                    b1 = 'x';
                    turn++;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("top right")) {
                if (c1 == ' ') {
                    c1 = 'x';
                    turn++;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("middle left")) {
                if (a2 == ' ') {
                    a2 = 'x';
                    turn++;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("middle middle")) {
                if (b2 == ' ') {
                    b2 = 'x';
                    turn++;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("middle right")) {
                if (c2 == ' ') {
                    c2 = 'x';
                    turn++;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("bottom left")) {
                if (a3 == ' ') {
                    a3 = 'x';
                    turn++;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("bottom middle")) {
                if (b3 == ' ') {
                    b3 = 'x';
                    turn++;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("bottom right")) {
                if (c3 == ' ') {
                    c3 = 'x';
                    turn++;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else {
                System.out.println("You have entered an invalid sapce"
                        + "please try again.");
            }
             System.out.printf(" %c | %c | %c \n", a1, b1, c1);
            System.out.printf("____________\n");
            System.out.printf(" %c | %c | %c \n", a2, b2, c2);
            System.out.printf("____________\n");
            System.out.printf(" %c | %c | %c \n", a3, b3, c3);
            if (a1 == 'x') {
                if (a2 == 'x') {
                    if (a3 == 'x') {
                        System.out.println("Player 1 wins!!!");
                        System.exit(1);
                    }
                }
            }
             if (b1 == 'x') {
                if (b2 == 'x') {
                    if (b3 == 'x') {
                        System.out.println("Player 1 wins!!!");
                        System.exit(1);
                    }
                }
            }
              if (c1 == 'x') {
                if (c2 == 'x') {
                    if (c3 == 'x') {
                        System.out.println("Player 1 wins!!!");
                        System.exit(1);
                    }
                }
            }
               if (a1 == 'x') {
                if (b2 == 'x') {
                    if (c3 == 'x') {
                        System.out.println("Player 1 wins!!!");
                        System.exit(1);
                    }
                }
            }
                if (a1 == 'x') {
                if (b1 == 'x') {
                    if (c1 == 'x') {
                        System.out.println("Player 1 wins!!!");
                        System.exit(1);
                    }
                }
            }
                 if (a2 == 'x') {
                if (b2 == 'x') {
                    if (c2 == 'x') {
                        System.out.println("Player 1 wins!!!");
                        System.exit(1);
                    }
                }
            }
                  if (a3 == 'x') {
                if (b3 == 'x') {
                    if (c3 == 'x') {
                        System.out.println("Player 1 wins!!!");
                        System.exit(1);
                    }
                }
            }
                   if (c1 == 'x') {
                if (b2 == 'x') {
                    if (a3 == 'x') {
                        System.out.println("Player 1 wins!!!");
                        System.exit(1);
                    }
                }
            }


            if(y == 9)
            {
                System.out.println("the game ends in a draw.");
                System.exit(1);
            }

        }
        if (turn == 2) {
            System.out.println("player 2 it is your turn. your spot you"
                    + " would like to enter.\n Ex: top,botton,middle left,right,middle");
            String input = keyboard.nextLine();

            if (input.equals("top left")) {
                if (a1 == ' ') {
                    a1 = 'o';
                    turn--;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("top middle")) {
                if (b1 == ' ') {
                    b1 = 'o';
                    turn--;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("top right")) {
                if (c1 == ' ') {
                    c1 = 'o';
                    turn--;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("middle left")) {
                if (a2 == ' ') {
                    a2 = 'o';
                    turn--;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("middle middle")) {
                if (b2 == ' ') {
                    b2 = 'o';
                    turn--;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("middle right")) {
                if (c2 == ' ') {
                    c2 = 'o';
                    turn--;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("bottom left")) {
                if (a3 == ' ') {
                    a3 = 'o';
                    turn--;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("bottom middle")) {
                if (b3 == ' ') {
                    b3 = 'o';
                    turn--;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else if (input.equals("bottom right")) {
                if (c3 == ' ') {
                    c3 = 'o';
                    turn--;
                    y++;
                } else {
                    System.out.println("you cant pick that space its"
                            + " already been taken");
                }
            } else {
                System.out.println("You have entered an invalid sapce"
                        + "please try again.");
            }
            System.out.printf(" %c | %c | %c \n", a1, b1, c1);
            System.out.printf("____________\n");
            System.out.printf(" %c | %c | %c \n", a2, b2, c2);
            System.out.printf("____________\n");
            System.out.printf(" %c | %c | %c \n", a3, b3, c3);
            if (a1 == 'o') {
                if (a2 == 'o') {
                    if (a3 == 'o') {
                        System.out.println("Player 2 wins!!!");
                        System.exit(1);
                    }
                }
            }
             if (b1 == 'o') {
                if (b2 == 'o') {
                    if (b3 == 'o') {
                        System.out.println("Player 2 wins!!!");
                        System.exit(1);
                    }
                }
            }
              if (c1 == 'o') {
                if (c2 == 'o') {
                    if (c3 == 'o') {
                        System.out.println("Player 2 wins!!!");
                        System.exit(1);
                    }
                }
            }
               if (a1 == 'o') {
                if (b2 == 'o') {
                    if (c3 == 'o') {
                        System.out.println("Player 2 wins!!!");
                        System.exit(1);
                    }
                }
            }
                if (a1 == 'o') {
                if (b1 == 'o') {
                    if (c1 == 'o') {
                        System.out.println("Player 2 wins!!!");
                        System.exit(1);
                    }
                }
            }
                 if (a2 == 'o') {
                if (b2 == 'o') {
                    if (c2 == 'o') {
                        System.out.println("Player 2 wins!!!");
                        System.exit(1);
                    }
                }
            }
                  if (a3 == 'o') {
                if (b3 == 'o') {
                    if (c3 == 'o') {
                        System.out.println("Player 2 wins!!!");
                        System.exit(1);
                    }
                }
            }
                   if (c1 == 'o') {
                if (b2 == 'o') {
                    if (a3 == 'o') {
                        System.out.println("Player 2 wins!!!");
                        System.exit(1);
                    }
                }
            }                

        }

    }

}

}
\$\endgroup\$
  • \$\begingroup\$ You should start testing yourself to see not only what you could do, but also how you could do it! Welcome to Code Review, you've come to the right place! \$\endgroup\$ – Simon Forsberg Oct 1 '15 at 19:58
  • \$\begingroup\$ One word: Switch. \$\endgroup\$ – Zenohm Oct 2 '15 at 0:04
  • \$\begingroup\$ @Zenohm although normally I agree in switches and uses them it's not always the best choice. What's more is that in this scenario switch is almost the same in verbosity as using the if/else if/else statements. Lastly there are nested if statments inside those blocks making switch even less useful. Maybe you could show an example of what you mean and how it is a better choice \$\endgroup\$ – Robert Snyder Oct 2 '15 at 12:53
  • \$\begingroup\$ @RobertSnyder Oh wait, you only said "an example," yeah... I may have slightly overdone that. I also now agree with you that the switch is a worse choice after thinking on it a bit more. \$\endgroup\$ – Zenohm Oct 2 '15 at 22:31
4
\$\begingroup\$

I have a few recommendations to make. Note that, like you, I am also in the process of learning Java, however I am coming at this from a Python mindset.

This answer does not use any arrays, see h.j.k.'s answer for those. You really should be using them for a gridded game like Tic-Tac-Toe.

Those Nested if Statements

Currently, your code operates as such:

if (input.equals("top left")) {
    if (a1 == ' ') {
        a1 = 'x';
        turn++;
        y++;
    } else {
        System.out.println("you cant pick that space its"
                + " already been taken");
    }
} else if (input.equals("top middle")) {
    if (b1 == ' ') {
        b1 = 'x';
        turn++;
        y++;
    } else {
        System.out.println("you cant pick that space its"
                + " already been taken");
    }
//.
//.
//.
} else {
    System.out.println("You have entered an invalid sapce"
            + "please try again.");
}

This works, but there are ways that you could improve this and make it more readable or more compact.

Let me list the ways that I know,

  • Switch Statement + if statements
  • Switch Statement and factored out if statement
  • Combine the inline if statements

(There may be other, better ways.)

Let me give an example of the first way and list of its benefits

Switch + if

switch (input){
    case "top left":
        if (a1 == ' ') {
            a1 = 'x';
            turn++;
            y++;
        } else {
            System.out.println("you cant pick that space its"
                    + " already been taken");
        }
        break;

    case "top middle":
        if (b1 == ' ') {
            b1 = 'x';
            turn++;
            y++;
        } else {
            System.out.println("you cant pick that space its"
                    + " already been taken");
        }
        break;
    //.
    //.
    //.
    default:
        System.out.println("You have entered an invalid sapce"
                + "please try again.");
}

Advantages:

  • Clearly distinguishes where checks begin and end
  • Easier to type than the current solution
  • Easier to glance over than the current solution
  • Encloses the entire section of logic under one clearly defined statement
  • Keeps the advantage of the nested if, that being a unique response if a space is taken

Disadvantages:

  • Would require you to rewrite your code for the most part
  • Cannot combine check for space availability, as far as I know

(Either may be extended via comments)

Switch && Factored if

if ( available(input) ) { // Requires a user-defined function to check availability and validity of input.
    switch (input){
        case "top left":
            a1 = 'x'; break;

        case "top middle":
            b1 = 'x'; break;
        //.
        //.
        //.
        // No default, the input has already been verified by the available() function.
    }
    turn++;
    y++;
} else {
     System.out.println("That space is unavailable."
             + " Please choose another.");
}

Advantages:

  • More readable
  • Removes repetition

Disadvantages:

  • Requires you to write your own function that has no value outside this block of code
  • Really more trouble than it's worth
  • You lose the unique response if the spot is unavailable
  • Solves the repeating code by having you write a function that does pretty much the same thing.
  • If you ever wanted to extend this program to recognize more varied inputs, you'd have to change the available() function.
  • Simply relocates the nested code elsewhere

In retrospect: Don't do this, it's unnecessary and a bad long-term solution.

Combined if

validInput = true; // boolean validInput; has been declared at the top of the program.
if (input.equals("top left") && a1 == ' ') {
    a1 = 'x';
}
else if (input.equals("top middle") && b1 == ' ') {
    b1 = 'x';
}
//.
//.
//.
else { 
    System.out.println("That space is unavailable."
            + " Please choose another.");
    validInput = false;
}

if (validInput) {
    turn++;
    y++;
}

Advantages:

  • Far more readable
  • No more nesting
  • Removes repetition
  • Good long-term solution
  • Easy to extend
  • Easier to write
  • Easier to rewrite than alternatives
  • You can reuse the validInput flag for Player 2

Disadvantages:

  • You lose the unique response if the spot is unavailable
  • Uuuh... (Possibly help here from the comments)

Just looking at the possibilities that I have listed, the best solution to these sections of code is to combine your nested if statements and use a flag to determine whether or not to increment.

More Nested if Statements

Seems to be a common theme in your code. You use the following code to check for a single win condition.

if (a1 == 'x') {
    if (a2 == 'x') {
        if (a3 == 'x') {
            System.out.println("Player 1 wins!!!");
            System.exit(1);
        }
    }
}

This and all the statements like it can be compressed into a single if statement which reads as follows;

if (a1 == 'x' && a2 == 'x' && a3 == 'x') {
    System.out.println("Player 1 wins!!!");
    System.exit(1);
}

Alternatively, if you don't want to write so many &&'s, you could just convert the three characters into a String when you add them and check the result;

if (("" + a1 + a2 + a3).equals("xxx")) {
    System.out.println("Player 1 wins!!!");
    System.exit(1);
}

Repeating Code

You repeat the following code a lot inside your win conditions;

System.out.println("Player 1 wins!!!"); // or System.out.println("Player 2 wins!!!"); 
System.exit(1);

What I recommend is to have a String variable which stores the current winner in it. Let's say you declare it at the top of your program;

String winner = "Nobody";

Now, whenever a win condition is detected, you would have the code break out of the while loop and display who won. This could be done as follows;

String winner = "Nobody";
while (y <= 9) {
    // Play the game Player 1.
    if (("" + a1 + b1 + c1).equals("xxx")) { // Or something like this.
        winner = "Player 1";
        break;
    // Play the game Player 2.
    if (("" + a1 + b1 + c1).equals("ooo")) { // Or something like this.
        winner = "Player 2";
        break;
} // Note the following code will run when the while loop ends or is broken
System.out.println(winner + " wins!"); // You could check here if winner.equals("Nobody")  so you can display a unique message for draws.

Wrap-Up

Here is the code with all the changes that I have recommended and a few other minor fixes. (You could remove the braces on the single line if statements, but I left them.)

All of the win condition checking is done below with several if statements. Those can all be turned into one if statement, but I left them spread out so it's slightly easier to read.

import java.util.Scanner;

public class SimpleTicTacToe {

    public static void main(String[] args) {

        Scanner keyboard = new Scanner(System.in);
        String winner = "Nobody";
        boolean validInput;
        char a1 = ' ', b1 = ' ', c1 = ' ';
        char a2 = ' ', b2 = ' ', c2 = ' ';
        char a3 = ' ', b3 = ' ', c3 = ' ';
        int turn = 1;
        int y = 0;

        gameLoop: while (y <= 9 && winner.equals("Nobody")) {
            if (turn == 1) {
                System.out.println("Player 1, enter the spot you would like to occupy.\n"
                        + "Ex: top,middle,bottom, left,middle,right");
                String input = keyboard.nextLine();

                validInput = true;
                if (input.equals("top left") && a1 == ' ') {
                    a1 = 'x';
                } else if (input.equals("top middle") && b1 == ' ') {
                    b1 = 'x';
                } else if (input.equals("top right") && c1 == ' ') {
                    c1 = 'x';
                } else if (input.equals("middle left") && a2 == ' ') {
                    a2 = 'x';
                } else if (input.equals("middle middle") && b2 == ' ') {
                    b2 = 'x';
                } else if (input.equals("middle right") && c2 == ' ') {
                    c2 = 'x';
                } else if (input.equals("bottom left") && a3 == ' ') {
                    a3 = 'x';
                } else if (input.equals("bottom middle") && b3 == ' ') {
                    b3 = 'x';
                } else if (input.equals("bottom right") && c3 == ' ') {
                    c3 = 'x';
                } else {
                    System.out.println("That space is unavailable."
                            + " Please choose another.");
                    validInput = false;
                }

                if (validInput) {
                    turn++;
                    y++;
                }

                System.out.printf(" %c | %c | %c \n", a1, b1, c1);
                System.out.printf("____________\n");
                System.out.printf(" %c | %c | %c \n", a2, b2, c2);
                System.out.printf("____________\n");
                System.out.printf(" %c | %c | %c \n", a3, b3, c3);
                if (("" + a1 + a2 + a3).equals("xxx")) {
                    winner = "Player 1";
                }
                if (("" + b1 + b2 + b3).equals("xxx")) {
                    winner = "Player 1";
                }
                if (("" + c1 + c2 + c3).equals("xxx")) {
                    winner = "Player 1";
                }
                if (("" + a1 + b1 + c1).equals("xxx")) {
                    winner = "Player 1";
                }
                if (("" + a2 + b2 + c2).equals("xxx")) {
                    winner = "Player 1";
                }
                if (("" + a3 + b3 + c3).equals("xxx")) {
                    winner = "Player 1";
                }
                if (("" + a1 + b2 + c3).equals("xxx")) {
                    winner = "Player 1";
                }
                if (("" + c1 + b2 + a3).equals("xxx")) {
                    winner = "Player 1";
                }
                if (winner.equals("Player 1") || y==9) {
                    break gameLoop;
                }

            }
            if (turn == 2) {
                System.out.println("Player 2, enter the spot you would like to occupy.\n"
                        + "Ex: top,middle,bottom, left,middle,right");
                String input = keyboard.nextLine();

                validInput = true;
                if (input.equals("top left") && a1 == ' ') {
                    a1 = 'o';
                } else if (input.equals("top middle") && b1 == ' ') {
                    b1 = 'o';
                } else if (input.equals("top right") && c1 == ' ') {
                    c1 = 'o';
                } else if (input.equals("middle left") && a2 == ' ') {
                    a2 = 'o';
                } else if (input.equals("middle middle") && b2 == ' ') {
                    b2 = 'o';
                } else if (input.equals("middle right") && c2 == ' ') {
                    c2 = 'o';
                } else if (input.equals("bottom left") && a3 == ' ') {
                    a3 = 'o';
                } else if (input.equals("bottom middle") && b3 == ' ') {
                    b3 = 'o';
                } else if (input.equals("bottom right") && c3 == ' ') {
                    c3 = 'o';
                } else {
                    System.out.println("That space is unavailable."
                            + " Please choose another.");
                    validInput = false;
                }

                if (validInput) {
                    turn--;
                    y++;
                }

                System.out.printf(" %c | %c | %c \n", a1, b1, c1);
                System.out.printf("____________\n");
                System.out.printf(" %c | %c | %c \n", a2, b2, c2);
                System.out.printf("____________\n");
                System.out.printf(" %c | %c | %c \n", a3, b3, c3);
                if (("" + a1 + a2 + a3).equals("ooo")) {
                    winner = "Player 2";
                }
                if (("" + b1 + b2 + b3).equals("ooo")) {
                    winner = "Player 2";
                }
                if (("" + c1 + c2 + c3).equals("ooo")) {
                    winner = "Player 2";
                }
                if (("" + a1 + b1 + c1).equals("ooo")) {
                    winner = "Player 2";
                }
                if (("" + a2 + b2 + c2).equals("ooo")) {
                    winner = "Player 2";
                }
                if (("" + a3 + b3 + c3).equals("ooo")) {
                    winner = "Player 2";
                }
                if (("" + a1 + b2 + c3).equals("ooo")) {
                    winner = "Player 2";
                }
                if (("" + c1 + b2 + a3).equals("ooo")) {
                    winner = "Player 2";
                }

            }

        }

        System.out.println(winner + " wins!");

    }

}

Really though, you should be using arrays in this case. They compress a lot of the code. The changes that I've recommended above essentially just change the way you've written your code so that it's easier to read.

\$\endgroup\$
  • \$\begingroup\$ It's never a bad idea to break out the code with more methods. Python differs from java significantly in its use of methods. In the java world it is generally better to use more methods to clearly express what your code is trying to do. Your tip on cleaning up the nested if's though is spot on! \$\endgroup\$ – Robert Snyder Oct 5 '15 at 11:48
  • \$\begingroup\$ @RobertSnyder Where does Java draw the line when writing more methods? In this case, for example, the use of the available() method would require the OP to update not only his original code, but also that method if he ever added more functionality to the inputs (Like being able to reverse the order that they are entered in). Is this a poor choice because the method complicates modification, or is it a good choice because it makes the code more readable? Does Java favor readability over malleability or simplicity? \$\endgroup\$ – Zenohm Oct 5 '15 at 13:24
  • 3
    \$\begingroup\$ Java does favor readability. Before this turns into a huge flame war I will say that simplicity is subjective. Those who are used to procedural/functional languages (probably such as yourself) you see a long list of code as easy to understand and simple. OO people (such as myself) would prefer seeing design patterns and small classes/methods. I suppose that is why we are told to learn multiple languages to get a good balance of the two. \$\endgroup\$ – Robert Snyder Oct 5 '15 at 14:13
6
\$\begingroup\$

Arrays

char a1 = ' ';
char a2 = ' ';
char a3 = ' ';
char b1 = ' ';
char b2 = ' ';
char b3 = ' ';
char c1 = ' ';
char c2 = ' ';
char c3 = ' ';

These nine variables can be replaced by a 'compact' 3x3 char[][] array. The main benefit of an array is that it makes looping much easier:

// for simplicity, just to demonstrate if there is any winning *row*
private static boolean isWinningRow(char[][] grid, char player) {
    char[] winning = new char[]{ player, player, player };
    for (char[] row : grid) {
        if (Arrays.equals(row, winning)) {
            return true;
        }
    }
    return false;
}

// for simplicity, just to demonstrate if there is any winning *column*
private static boolean isWinningColumn(char[][] grid, char player) {
    char[] winning = new char[]{ player, player, player };
    for (int i = 0; i < grid.length; i++) {
        // there is probably a better way of doing this
        char[] column = new char[]{ grid[0][i], grid[1][i], grid[2][i] };
        if (Arrays.equals(column, winning)) {
            return true;
        }
    }
    return false;
}

Methods

You should significantly cut down on code repetition in order to standardize most of the logic in your game. The main benefit of using more methods is that any bugs can be fixed within one method, instead of having to manually copy-and-paste the fixes throughout the codebase.

For example, the following code block is repeated:

if (b1 == ' ') {
    b1 = 'x';
    turn++;
    y++;
} else {
    System.out.println("you cant pick that space its"
            + " already been taken");
}

A possible method replacement for it might be (which ties in neatly with the array usage):

private static boolean update(char[][] grid, int x, int y, char player) {
    if (grid[x][y] != ' ') {
        return false;
    }
    grid[x][y] = player;
    return true;
}

The return type of the method - a boolean here - is useful to also indicate whether the move was valid or not, and to keep re-prompting the player:

// inside the main() method
int[] position = translateInput(scanner);
while (!update(grid, position[0], position[1], player)) {
    // note: spelling fixes too
    System.out.println("You can't pick that space, it's taken.");
    position = translateInput(scanner);
}
// grid is updated, proceed to next player

In another example, instead of repeating the lines for displaying the grid:

System.out.printf(" %c | %c | %c \n", a1, b1, c1);
System.out.printf("____________\n");
System.out.printf(" %c | %c | %c \n", a2, b2, c2);
System.out.printf("____________\n");
System.out.printf(" %c | %c | %c \n", a3, b3, c3);

Use a method:

private static void displayGrid(char[][] grid) {
    for (int i = 0; i < grid.length; i++) {
        System.out.printf(" %c | %c | %c \n", grid[i][0], grid[i][1], grid[i][2]);
        if (i < grid.length - 1) {
            System.out.println("____________");
        }
    }
}

Other comments

If you are on Java 7 and above, you should use try-with-resources on your Scanner instance for proper handling of its underlying I/O-based resource.

A non-zero value for System.exit(int) usually indicates 'abnormal termination'. Second, you should favor relying on a terminal condition of your loop to know when the game has ended:

private static void main(String[] args) {
    char[][] grid = createGridOfBlanks();
    char player = 'x'; // start with X
    boolean isWinning = false;
    try (Scanner scanner = new Scanner(System.in)) {
        do {
            int[] position = translateInput(scanner);
            while (!update(grid, position[0], position[1], player)) {
                System.out.println("You can't pick that space, it's taken.");
                position = translateInput(scanner);
            }
            displayGrid(grid);
            isWinning = isWinning(grid, player);
            if (!isWinning) {
                player = player == 'x' ? 'o' : 'x';
            }
        } while (!isTie(grid) && !isWinning);
    }
    if (isWinning) {
        // show player has won
    } else {
        // show tie
    }
}
\$\endgroup\$
  • 2
    \$\begingroup\$ IMO a single array is a better choice (albeit it slightly more taxing on the brain) but a method to reference a specific index using row and column would make that trivial. Single dimension arrays perform better as well... might just be personal preference \$\endgroup\$ – Robert Snyder Oct 2 '15 at 3:16
  • 3
    \$\begingroup\$ @RobertSnyder any performance improvements for the purpose of a console-based tic-tac-toe game is unlikely to be significant. ;) \$\endgroup\$ – h.j.k. Oct 2 '15 at 3:34
  • \$\begingroup\$ @robertsnyder so how exactly would i go about using the method for outputting the grid. I'm having trouble figuring out how to actually call it after making it a method. \$\endgroup\$ – Dan Nordin Oct 5 '15 at 7:38
  • \$\begingroup\$ @DanNordin When I get home from work tonight I'll finish up some of my thoughts and put it in an answer. To hopefully push you in the right direction draw a grid and number each cell incrementally. Then use the math to return the index. For instance if I was doing it I would return something like return (columns.length*row) + column. Naturally I'd want to unit test it first as I think there is a logical problem with that, but I think that is close. \$\endgroup\$ – Robert Snyder Oct 5 '15 at 14:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.