7
\$\begingroup\$

I am currently self-studying Java using the resources available on the internet (Stanford's cs106a introduction to programming lecture). I am still a beginner, and I hope that I can improve my coding skill after receiving some feedback here.

This is a program that plays 2-player Tic Tac Toe (I might add an AI feature in the future). The basic algorithm for this code is using 2 dimensional array (3x3) to keep track of the grid while using an array list to keep track of player move (to avoid duplicate and to check whether or not the grid is full).

How to play: In turn, each player will type in 2 numbers (row and column) where the O or the X will be placed.

I believe that there are no bug in the code (CMIIW). However, I feel that the code is too lengthy. I would appreciate it if anyone could give some feedback for my code.

Note: grid = game board

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

public class Play {

private static char[][] grid = new char[3][3];  
private static int player;
private static Scanner input = new Scanner(System.in);
private static ArrayList<int[]> moveMemory = new ArrayList<int[]>();

public static void main(String[] args) {

    while(true){

        setUp(); //set up the game (give instruction, reset board)

        for (player = 1; player <= 2; player++){ //looping for turn between player 1 and player 2           

                putObject(player, grid); //player (player) turn to put an O or an X
                printGrid(grid); //print the current board
                if (isWin()) break; //check whether anyone win or not
                if (gridIsFull()){ //check whether or not the board is full
                    player = 0; //player 0 means no one win
                    break;
                }

          //additional looping code (after player 2 turn, player 1 plays)
            if (player == 2){ 
                player = 0;
            }
        }   

        //congratulate winner
        winner(player);

        //restart or exit?
        if (!wantToRestart()){
            System.out.println("thanks for playing!");
            break;
        }

    }

    input.close();
}

private static void setUp(){
    resetGrid();
    resetMoveMemory();
    instruction();

}

private static void instruction(){

    System.out.println("Welcome to a 2-player tic tac toe game.");
    System.out.println("How to play: In turn, each player will type in 2 numbers ");
    System.out.println("(row and column) where the O or the X will be placed.");
    System.out.println("Player 1 will be an O and Player 2 will an X");
    System.out.println("Example: When player 1 typed in \"2 2\", an O will be placed in row 2 and column 2");
    System.out.println("Note: type \"grid\" to show the current grid");
    System.out.println("");
    printGrid(grid);
}

private static void resetGrid(){

        for (int i = 0; i < 3; i++){
            for (int j = 0; j < 3; j++){
                grid[i][j] = '-';
            }
        }
}

private static void resetMoveMemory(){
    moveMemory.clear();
}

//ask player about the location to put his X or O, and put it there
private static void putObject(int player, char[][] grid){
    char object = '\u0000';

    if (player == 1){
        object = 'O';
    } else if (player == 2){
        object = 'X';
    }

    int[] move = getPlayerMove(player);
    int row = move[0];
    int column = move[1];

    grid[row][column] = object;
    updateMoveMemory(row, column);
}

//return an array of an size 2 {row, column} 
private static int[] getPlayerMove(int player){

    int[] move = new int[2];

    while (true){
        System.out.println("");
        System.out.println("Player " + player + " turn. Select your move:" );
        String moveString = input.nextLine();

            if (moveString.equals("grid")){
                printGrid(grid);
            }
            else if (isValidInput(moveString)){
                move = convertToMoveArray(moveString);

                    if (!moveIsDuplicate(move)){                    
                        break;  
                    }

            } else {
                System.out.println("invalid move");
            }




    }

    return move;
}

private static void updateMoveMemory(int row, int column){
    int[] myArray = new int[2];
    myArray[0] = row;
    myArray[1] = column;
    moveMemory.add(myArray);
}

private static boolean isValidInput(String str){
    StringTokenizer st = new StringTokenizer(str);
        for (int i = 0; i < 2; i++){

            try
              { int myNumber = Integer.parseInt(st.nextToken()); 
                if (!(myNumber >= 1 && myNumber <= 3)){
                    return false;
               }
              }

             catch(NumberFormatException er)
              { return false; }

        }

        if (st.hasMoreTokens()) {
            return false;
        }
        return true;

}

private static boolean moveIsDuplicate(int[] move){
    for (int i = 0; i < moveMemory.size(); i++){
        if (moveMemory.get(i)[0] == move[0] && moveMemory.get(i)[1] == move[1]){
            System.out.println("grid is taken");
            return true;
        }
    }


    return false;

}

private static int[] convertToMoveArray(String str){
    int [] move = new int[2];
    StringTokenizer st = new StringTokenizer(str);

    for (int i = 0; i < 2; i++){
        move[i] = Integer.parseInt(st.nextToken());
        move[i]--;
    }

    //tokenize str here

    return move;        
}

private static void printGrid(char[][] grid){

    for (int i = 0; i < 3; i++){
        System.out.println("");
        for (int j = 0; j < 3; j++){
            System.out.print(grid[i][j] + " ");
        }
    }

    System.out.println("");

}

private static boolean isWin(){

    if (checkRow()) return true;
    if (checkColumn()) return true;
    if (checkDiagonal()) return true;
    return false;

}

private static boolean checkRow(){
    for (int i = 0; i < 3; i++){
        if (grid[i][0] == grid [i][1] && grid [i][0] == grid[i][2]){
            if (grid[i][0] != '-') return true; //because char '-' is empty
        }
    }

    return false;
}

private static boolean checkColumn(){
    for (int i = 0; i < 3; i++){
        if (grid[0][i] == grid [1][i] && grid [0][i] == grid[2][i]){
            if (grid[0][i] != '-') return true;
        }
    }

    return false;
}

private static boolean checkDiagonal(){

    if ((grid[1][1] == grid[0][0] && grid[1][1] == grid[2][2]) ||
            (grid[1][1] == grid[0][2] && grid [1][1] == grid[2][0])){
        if (grid[1][1] != '-') return true;
    }

    return false;
}

private static boolean gridIsFull(){
    if (moveMemory.size() == Math.pow(3, 2)){
        return true;
    }       
    return false;
}

private static boolean wantToRestart(){

    while(true){

        System.out.println("type (restart) to play again or type (exit) to quit:");

        String str = input.nextLine();

        if (str.equals("restart")){
            return true;
        }
        else if (str.equals("exit")){
            return false;
        } else {
            System.out.println("invalid command");
        }

    }
}

private static void winner(int player){
    if (player != 0){
        System.out.println(player + " win");
    } else {
        System.out.println("draw");
    }

}


}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Welcome to Code Review! I hope you get some great answers from our Java pros! \$\endgroup\$
    – Phrancis
    May 30, 2015 at 0:32

1 Answer 1

5
\$\begingroup\$

Avoid magic numbers and strings

The number 3 is scattered everywhere. It would be better to put that in a constant.

The same goes for 'O', 'X', '-'. This latter should be named EMPTY, so that you don't have to explain what it is in comments.

Don't keep state in static variables

Static variables are global state, which is error-prone and dangerous. Static variables can be fine if final, non-modifiable. It's highly suspicious when a static variable can change, avoid doing that as much as possible.

Use boolean expressions directly

This is tedious:

    if (st.hasMoreTokens()) {
        return false;
    }
    return true;

You can write simply as:

    return !st.hasMoreTokens();

Refer to types by interfaces

Instead of:

ArrayList<int[]> moveMemory = new ArrayList<int[]>();

Write like this:

List<int[]> moveMemory = new ArrayList<int[]>();

Naming

Is "Play" a good name for such class? What is really a "Play" object? "Play" sounds like an action. If your class was named "TicTacToe", and it had a "play" method, then TicTacToe.play() would be quite idiomatic.

Functions are best named with verbs, or verb + noun. "instruction" is not a good name. printInstructions would be better.

putObject is a where poor name. What kind of object is it? Put it where? Since the method is about making a player make a move, something like makePlayerMove would be better.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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