Java Tic Tac Toe implementation

Question

I am looking for some feedback regarding my Java Tic-Tac-Toe implementation.

Player.java

class Player {
    private String coordinate;
    private String name;


    public Player(String coordinate, String name ) {
        this.coordinate = coordinate;
        this.name = name;

    }

    public String getCoordinate() {
        return this.coordinate;
    }

    public String getName() {
        return this.name;
    }
}

TicTacToe.java

import java.util.Scanner;
import java.util.Set;
import java.util.HashSet;
import java.util.Hashtable;

public class TicTacToe {

    public static Hashtable<String,String> coordinates = new Hashtable<String, String>() {
        {
            put("X", "notselected");
            put("Y", "notselected");
        }
    };

    public static Scanner input = new Scanner(System.in);

    public static String[][] matrix = new String[][]{
        { "O", "O", "O" },
        { "O", "O", "O"},
        { "O", "O", "O" },
    };

    public static void printMatrix() {
        System.out.println("This is how the matrix looks like at the moment:\n");
        for(int i = 0 ; i < matrix.length; i ++ ) {
            for(int j = 0; j < matrix.length; j++ ) {
                System.out.print(matrix[i][j] + " " );
            }
            System.out.println();
        }
        System.out.println();
    }

    public static Player getPlayerInfo() throws InterruptedException {
        Thread.sleep(1000);
        System.out.println("Player name:");
        String playerName = input.next();

        if ( coordinates.get("X").equals("selected")) {
            System.out.println("Y is the only coordinate being left.");
            return new Player("Y", playerName);
        } else if ( coordinates.get("Y").equals("selected")) {
            System.out.println("X is the only coordinate being left.");
            return new Player("X", playerName); 
        } else {
            System.out.println("Enter your coordinate:");
            String coordinate = input.next();
            coordinates.put(coordinate, "selected");
            return new Player(coordinate, playerName);
        }
    }

    public static boolean hasAPlayerWon(Player player ) {
        if (isRowSolved(player.getCoordinate())) {
            System.out.println("Row is solved");
            return true;
        } else if (isColumnSolved(player.getCoordinate())) {
            System.out.println("Column is solved");
            return true;
        } else if (isLeftDiagonalSolved(player.getCoordinate())) {
            System.out.println("Diagonal is solved");
            return true;
        } else if (isRightDiagonalSolved(player.getCoordinate())) {
            System.out.println("Right diagona is solved");
            return true;
        }


        return false;
    }

    private static boolean isRowSolved(String coordinate ) {
        for(int i = 0; i < matrix.length; i ++) {
            Set<String> elements = new HashSet<String>();

            for(int j = 0; j < matrix[i].length; j++) {
                elements.add(matrix[i][j]);
            }

            if (elements.contains(coordinate) && elements.size() == 1) {
                return true;
            }

        }

        return false;
    }

    private static boolean isColumnSolved(String coordinate) {
        for(int i = 0; i < matrix.length; i ++) {
            Set<String> elements = new HashSet<String>();

            for(int j = 0; j < matrix[i].length; j++) {
                elements.add(matrix[j][i]);
            }

            if (elements.contains(coordinate) && elements.size() == 1) {
                return true;
            }

        }

        return false;
    }

    private static boolean isLeftDiagonalSolved(String coordinate) {
        int leftDiagonalIndex = 0;

        HashSet<String> leftDiagonalElements = new HashSet<String>(); 

        while(leftDiagonalIndex < matrix.length) {
            leftDiagonalElements.add(matrix[leftDiagonalIndex][leftDiagonalIndex]);
            leftDiagonalIndex += 1;

        }

        if(leftDiagonalElements.size() == 1 && matrix[leftDiagonalIndex-1][leftDiagonalIndex-1].equals(coordinate)) {
            return true;
        }  

        return false;
    }

    private static boolean isRightDiagonalSolved(String coordinate) {
        int rightDiagonalIndexRow = 0;
        int rightDiagonalIndexColumn = 2;

        HashSet<String> rightDiagonalElements = new HashSet<String>(); 

        while(rightDiagonalIndexRow < matrix.length) {
            rightDiagonalElements.add(matrix[rightDiagonalIndexRow][rightDiagonalIndexColumn]);
            rightDiagonalIndexRow += 1;
            rightDiagonalIndexColumn -= 1;

        }

        if(rightDiagonalElements.size() == 1 && matrix[rightDiagonalIndexRow-1][rightDiagonalIndexColumn+1].equals(coordinate)) {
            return true;
        }  

        return false;
    }

    public static boolean isMatrixUnsolvable() {
        for(int i = 0; i < matrix.length; i++) {
            for(int j = 0; j < matrix[0].length; j++) {
                if(matrix[i][j].equals("O")) {
                    return false;
                }
            }
        }

        return true;
    }

    public static void evaluateInput(Player player) throws InterruptedException {

        Thread.sleep(1000);
        System.out.println(player.getName() + " enter your indexes, seperated by comma:");
        String[] indexes = input.next().split(",");

        int row = Integer.valueOf(indexes[0]);
        int column = Integer.valueOf(indexes[1]);

        if(!matrix[row][column].equals("O")) {
            System.out.println("Sorry, you are missing your turn.");
            System.out.println("Either you already marked the square or you are tring to override a value");
            return;
        }

        matrix[row][column] = player.getCoordinate();

        if(hasAPlayerWon(player )) {
            System.out.println("Congratulations " + player.getName() + ", you WON!");
            System.exit(0);
        }

        if(isMatrixUnsolvable( )) {
            System.out.println("Matrix can't be resolved! Thank you for playing");
            System.exit(0);
        }

        printMatrix();
    }
    public static void main(String[] args) throws InterruptedException {
        printMatrix();

        Player firstPlayer = getPlayerInfo();
        Player secondPlayer = getPlayerInfo();

        while (true) {
            evaluateInput(firstPlayer);
            evaluateInput(secondPlayer);
        }
    }
}

My concerns are:

1. When using Thread.sleep() I prefer to declare the exception in the method declaration, because a try/catch is never going to fail. I mean I have simply 1 thread. Is this ok?

2. I know that a recommended SOLID principle for a method is to do a single task. Basically I was thinking to implement a method called checkIfRowOrColumnIsSolved, so that I do not do 2 separate methods. Is this a good name for a method? Does it not imply that the method is actually doing 2 things, instead of 1?

3. I'm looking for feedback how the game could be written in smaller chunks and with better logic. I am not looking for any Java 1.8/1.9 abstraction improvements at this point.

Answer 1

1) Since application has not defined another thread to cause the interrupt, it is OKAY to not catch InterruptedException.

2) For a trivial task, it's OKAY to have one method to do your row column check. One would even argue that both combined is actually one task. Note that over optimization or too much breaking of method into smaller individual methods is not always required. If you have a complex issue or difficulty in writing efficient unit tests then breaking up methods based on different logic makes sense.

Also for naming of methods, one should go with whatever one feels good about. There is not standard per se, but yeah it's definitely good to maintain a standard across one's own projects or in entire team of developers.

Just to add a suggestion to your method, I would name as isRowOrColumnSolved instead of checkIfRowOrColumnIsSolved if the method is returning a boolean.

3) Not sure what you mean by smaller chunks ? Are you asking about writing code with less number of lines or making it more modular with more classes ?

Answer 2

Here are my comments:

1. Replace literals with constants
   You chose "O" to signify an empty cell. You also have "X", "Y" as occupied cells. You use the String literals throughout the code. You should use either constants (static final variables) or better yet, use an Enum to describe all the possible values a cell can have. The most significant benefit is that the compiler will let you know if you mistyped a value somewhere. It also helps in readability.

2. Validate input
   You never validate that the user entered either "X" or "Y". You never validate that the user entered valid indexes in evaluateInput()

3. Single responsibility principle
   You mentioned this as a concern. So let us examine evaluateInput(): it is responsible for prompting the user and getting the input, it does partial input validation (checks that the cell pointed by the user is empty) and then delegates the rest of the processing to separate methods. If we want to follow Single responsibility principle, the above processing should be broken down to separate methods as well.

4. Hashtable
   is the same as HashMap with all operations synchronized. synchronization comes there with a performance cost and is needless in your program. Note: Even if you did want to have a thread safe HashMap, there are better alternatives to Hashtable, such as ConcurrentHashMap