Extremely Object Oriented Tic-Tac-Toe in Java

Please criticize the classes GameJudgeImpl and GameInfo so that I could make them clearer.

public interface GameJudge {
    public GameInfo gameInfo();
}

public enum GameResult {
    UNKNOWN, DRAW, PLAYER_WINS, OPPONENT_WINS
}

public enum Cell {
    EMPTY, PLAYER, OPPONENT
}

↓

public class GameInfo {
    private final GameResult gameResult;
    private final List cellsOnFire;

    public static GameInfo unknownResult() {
        return new GameInfo(GameResult.UNKNOWN, new ArrayList());
    }

    public static GameInfo drawResult() {
        return new GameInfo(GameResult.DRAW, new ArrayList());
    }

    public GameInfo(GameResult gameResult, List cellsOnFire) {
        this.gameResult = gameResult;
        this.cellsOnFire = cellsOnFire;
    }

    public GameResult gameResult() {
        return gameResult;
    }

    public List cellsOnFire() {
        return cellsOnFire;
    }

    public boolean resultIsKnown() {
        return gameResult != GameResult.UNKNOWN;
    }
}

↓

```
public class GameJudgeImpl implements GameJudge {
private final Matrix gameBoard;
private final int gameBoardDimension;

public GameJudgeImpl(Matrix gameBoard) {
this.gameBoard = gameBoard;
this.gameBoardDimension = gameBoard.rows;
}

@Override
public GameInfo gameInfo() {
GameInfo gameInfo = checkAllRowsColumns();
if (gameInfo.resultIsKnown()) {
return gameInfo;
}
gameInfo = checkDiagonals();
if (gameInfo.resultIsKnown()) {
return gameInfo;
}
if (gameBoard.contains(Cell.EMPTY)) {
return GameInfo.unknownResult();
}
return GameInfo.drawResult();
}

private GameInfo checkAllRowsColumns() {
for (int i = 0; i cellsPositionsOnRow(int row) {
List cells = new ArrayList(gameBoardDimension);
for (int column = 0; column cellsPositions) {
Matrix.Position firstCellOnLinePosition = cellsPo

-
Interface, GameJudge, doesn't seem to make sense.

-
the method gameInfo(), doesn't have an action verb so it's hard to determine what exactly it is doing. getGameInfo() perhaps?

-
An interface is a contract. So if another class were to implement this interface, would the return type, GameInfo, and the method gameInfo() make sense? i.e. for chess. If not, perhaps you'd want to rename your interface to something more specific to tic-tac-toe.

-
The Enum, GameResult, looks pretty good but needs some tweaking.

-
Since this is the result of the game, suggest that it only have 3 states: Player wins, Opponent wins, and Draw.

-
The value of Unknown leads me to believe that you will be using it to check the game's state. If that is the case, rename the class to GameState

-
The Enum, Cell, is fine. It will contain the state of the Cell.

A few things jump out of me are:

The use of static methods

-
In general, static methods make your code harder to test. See (Mikso Hevery). Since you always want to have tests with your code, try to avoid static methods until you have a better grasp of when to use them and when not to. This will help you with your OO design skills.

-
In this case, you're are using it to return a new instance (factory pattern) of GameInfo. The problem is that the static methods are in the same class you are returning. You would be better off creating a separate class that owns the responsibility of creating GameInfo objects.

-
I would argue that for any single game, you should only have a single GameInfo object. So you probably don't even need that factory method since you can create a new object during the start of the game and then return the same one when the game is finished.

A large number of private variables

• When you see a single public method and a large number of private methods, it could be an indication of that class having too much responsibility. Investigate your private methods and see if you can find common patterns/usage. If so, you can move those methods to the new class so that it can handle the responsibility.

For example, you have a lot of methods that start with 'check'. Could you make a class called, BoardChecker?

Keep repeating the process and breakdown your class into other classes when applicable.

Too many return statements

A large number of return statements can be confusing (and hard to debug if the method is large). Try to only have one or two return statements.

See method, gameInfo()

Naming Convention

What is Matrix.Position anyway? This seems very strange to me. Please ollow Java naming conventions for your class names.

StackExchange Code Review Q#29367, answer score: 6