3
\$\begingroup\$

After reading about MVC pattern in Head First Design Patterns, I've decided to write a TicTacToe app.

I will not reveal the source code of the classes Matrix, Dimension, and Size because they do not relate to the topic. All source code can be found here.

Please criticize my code, along with its MVC pattern usage.

TicTacToeActivity.java

public class TicTacToeActivity extends Activity {

    @Override
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);

        GameModel model = new GameModelImpl(new Dimension(4, 4));
        GameController controller = new GameControllerAndroidImpl(model, this);
    }
}

GameController.java

public interface GameController {
    void onViewIsReadyToStartGame();
    void onPlayerMove(Matrix.Position movePos);
}

GameControllerImpl.java

abstract class GameControllerImpl implements GameController {

    private final GameModel model;

    public GameControllerImpl(GameModel model) {
        this.model = model;
    }

    protected abstract GameView getGameView();

    @Override
    public void onViewIsReadyToStartGame() {
        model.onViewIsReadyToStartGame();
    }

    @Override
    public void onPlayerMove(Matrix.Position movePos) {
        getGameView().blockMoves();
        model.onPlayerTurn(movePos);
        getGameView().unblockMoves();
    }
}

GameControllerAndroidImpl.java

public class GameControllerAndroidImpl extends GameControllerImpl {

    private final GameView gameView;

    public GameControllerAndroidImpl(GameModel model, Activity activity) {
        super(model);
        gameView = new GameViewAndroidImpl(this, model, activity);
    }

    @Override
    protected GameView getGameView() {
        return gameView;
    }
}

GameView.java

public interface GameView {
    void blockMoves();
    void unblockMoves();
    boolean movesBlocked();
}

GameViewImpl.java

public abstract class GameViewImpl implements GameView, OnCellClickListener,
                        OnOpponentMoveListener, OnGameFinishedListener {

    private final GameController controller;
    private final GameModel model;
    private boolean movesBlocked;
    private boolean gameFinished;

    public GameViewImpl(GameController controller, GameModel model) {
        this.controller = controller;
        this.model = model;
        model.addOnOpponentMoveListener(this);
        model.addOnGameFinishedListener(this);

        gameFinished = false;
        movesBlocked = false;
    }

    protected abstract GameBoard gameBoard();

    protected abstract GameResultDisplay gameResultDisplay();

    protected OnCellClickListener getOnCellClickListener() {
        return this;
    }

    @Override
    public void blockMoves() {
        movesBlocked = true;
    }

    @Override
    public void unblockMoves() {
        movesBlocked = false;
    }

    @Override
    public boolean movesBlocked() {
        return movesBlocked;
    }

    @Override
    public void onCellClick(Matrix.Position cellPos) {
        if (gameFinished) {
            gameFinished = false;
            gameBoard().clear();
            controller.onViewIsReadyToStartGame();
        } else if (model.emptyCell(cellPos) && !movesBlocked()) {
            gameBoard().showMove(cellPos);
            controller.onPlayerMove(cellPos);
        }
    }

    @Override
    public void onOpponentMove(Matrix.Position movePos) {
        gameBoard().showMove(movePos);
    }

    @Override
    public void onGameFinished(GameInfo gameInfo) {
        gameFinished = true;
        gameBoard().showFireLine(gameInfo.cellsOnFire());
        gameResultDisplay().show(gameInfo.gameResult());
    }
}

OnCellClickListener.java

public interface OnCellClickListener {
    void onCellClick(Matrix.Position pos);
}

GameViewAndroidImpl.java

public class GameViewAndroidImpl extends GameViewImpl {

    private final GameBoard gameBoard;
    private final GameResultDisplay gameResultDisplay;

    public GameViewAndroidImpl(GameController controller, GameModel model, Activity activity) {
        super(controller, model);
        gameResultDisplay = new GameResultDisplayAndroidToastImpl(activity);
        GameBoardCreator gameBoardCreator = new GameBoardCreatorAndroidImpl(activity);
        gameBoard = gameBoardCreator.createGameBoard(model.getDimension());
        gameBoard.setOnCellClickListener(super.getOnCellClickListener());
    }

    @Override
    protected GameBoard gameBoard() {
        return gameBoard;
    }

    @Override
    protected GameResultDisplay gameResultDisplay() {
        return gameResultDisplay;
    }
}

GameResultDisplay.java

public interface GameResultDisplay {
    void show(GameResult gameResult);
}

GameResultDisplayAndroidToastImpl.java

public class GameResultDisplayAndroidToastImpl implements GameResultDisplay {

    private final Activity activity;

    public GameResultDisplayAndroidToastImpl(Activity activity) {
        this.activity = activity;
    }

    @Override
    public void show(GameResult gameResult) {
        Toast.makeText(activity, gameResult.name(), Toast.LENGTH_LONG).show();
    }
}

GameBoard.java

public interface GameBoard {
    void setOnCellClickListener(OnCellClickListener onCellClickListener);
    void showMove(Matrix.Position pos);
    void showFireLine(List<Matrix.Position> positions);
    void clear();
}

GameBoardAndroidImpl.java

public class GameBoardAndroidImpl implements GameBoard {

    private final Matrix<ImageView> cells;
    private CellIcon currentIcon;

    public GameBoardAndroidImpl(Matrix<ImageView> cells) {
        this.cells = cells;
        clear();
    }

    @Override
    public void clear() {
        cells.forEach(new Matrix.OnEachHandler<ImageView>() {
            @Override
            public void handle(Matrix<ImageView> matrix, Matrix.Position pos) {
                clearCell(pos);
            }
        });
        currentIcon = CellIcon.X;
    }

    private void clearCell(Matrix.Position cellPos) {
        setCellImageResource(cellPos, android.R.color.transparent);
        setCellBackgroundResource(cellPos, R.drawable.empty);
    }

    @Override
    public void setOnCellClickListener(final OnCellClickListener onCellClickListener) {
        cells.forEach(new Matrix.OnEachHandler<ImageView>() {
            @Override
            public void handle(Matrix<ImageView> matrix, final Matrix.Position pos) {
                ImageView cell = cells.get(pos);
                cell.setOnClickListener(new View.OnClickListener() {
                    @Override
                    public void onClick(View view) {
                        onCellClickListener.onCellClick(pos);
                    }
                });
            }
        });
    }

    @Override
    public void showMove(Matrix.Position pos) {
        int iconId;
        if (currentIcon == CellIcon.X) {
            iconId = IconRandomizer.randomCrossIconId();
            currentIcon = CellIcon.O;
        } else {
            iconId = IconRandomizer.randomZeroIconId();
            currentIcon = CellIcon.X;
        }
        setCellBackgroundResource(pos, iconId);
    }

    @Override
    public void showFireLine(List<Matrix.Position> positions) {
        for (Matrix.Position pos : positions) {
            setCellImageResource(pos, IconRandomizer.randomFireIconId());
        }
    }

    private void setCellBackgroundResource(Matrix.Position cellPos, int resId) {
        cells.get(cellPos).setBackgroundResource(resId);
    }

    private void setCellImageResource(Matrix.Position cellPos, int resId) {
        cells.get(cellPos).setImageResource(resId);
    }
}

CellIcon.java

public enum CellIcon {
    X, O;
}

IconRandomizer.java

public class IconRandomizer {

    private static final int[] CROSS_ICONS_IDS = {
            R.drawable.cross_1, R.drawable.cross_2, R.drawable.cross_3
    };

    private static final int[] ZERO_ICONS_IDS = {
            R.drawable.zero_1, R.drawable.zero_2, R.drawable.zero_3
    };

    private static final int[] FIRE_ICONS_IDS = {
            R.drawable.fire_1, R.drawable.fire_2, R.drawable.fire_3,
            R.drawable.fire_4, R.drawable.fire_5, R.drawable.fire_6
    };

    public static int randomCrossIconId() {
        return randomElement(CROSS_ICONS_IDS);
    }

    public static int randomZeroIconId() {
        return randomElement(ZERO_ICONS_IDS);
    }

    public static int randomFireIconId() {
        return randomElement(FIRE_ICONS_IDS);
    }

    private static int randomElement(int[] array) {
        int randomIndex = Randomizer.randomPositiveInt() % array.length;
        return array[randomIndex];
    }
}

GameBoardCreator.java

public interface GameBoardCreator {
    GameBoard createGameBoard(Dimension dim);
}

GameBoardCreatorAndroidImpl.java

public class GameBoardCreatorAndroidImpl implements GameBoardCreator {

    private static final int SPACE_BETWEEN_CELLS = 2;

    private final Activity activity;

    public GameBoardCreatorAndroidImpl(Activity activity) {
        this.activity = activity;
    }

    @Override
    public GameBoard createGameBoard(Dimension dim) {
        return new GameBoardAndroidImpl(prepareCells(dim));
    }

    private Matrix<ImageView> prepareCells(Dimension dim) {
        Matrix<ImageView> cells = new Matrix<ImageView>(dim);
        LinearLayout verticalLayout = prepareVerticalLinearLayout(dim);
        for (int row = 0; row < dim.rows; ++row) {
            LinearLayout rowLayout = prepareHorizontalLinearLayout(dim);
            for (int column = 0; column < dim.columns; ++column) {
                ImageView cell = prepareCell();
                setHorizontalMargins(cell, column, dim.columns);
                rowLayout.addView(cell);
                cells.set(row, column, cell);
            }
            setVerticalMargins(rowLayout, row, dim.rows);
            verticalLayout.addView(rowLayout);
        }
        activity.setContentView(R.layout.tic_tac_toe_activity);
        FrameLayout gameBoardFrameLayout =
                (FrameLayout) activity.findViewById(R.id.gameBoardFrameLayout);
        gameBoardFrameLayout.addView(verticalLayout);
        return cells;
    }

    private LinearLayout prepareVerticalLinearLayout(Dimension dim) {
        return prepareLinearLayout(LinearLayout.VERTICAL, dim.rows);
    }

    private LinearLayout prepareHorizontalLinearLayout(Dimension dim) {
        return prepareLinearLayout(LinearLayout.HORIZONTAL, dim.columns);
    }

    private LinearLayout prepareLinearLayout(int orientation, int weightSum) {
        LinearLayout layout = new LinearLayout(activity);
        layout.setWeightSum(weightSum);
        layout.setOrientation(orientation);
        return layout;
    }

    private ImageView prepareCell() {
        LayoutInflater inflater = activity.getLayoutInflater();
        return (ImageView) inflater.inflate(R.layout.cell_image_view, null);
    }

    private void setHorizontalMargins(ImageView cell, int column, int columns) {
        int leftMargin = (column == 0) ? 0 : SPACE_BETWEEN_CELLS;
        int rightMargin = (column == columns - 1) ? 0 : SPACE_BETWEEN_CELLS;
        setMargins(cell, leftMargin, 0, rightMargin, 0);
    }

    private void setVerticalMargins(LinearLayout rowLayout, int row, int rows) {
        int topMargin = (row == 0) ? 0 : SPACE_BETWEEN_CELLS;
        int bottomMargin = (row == rows - 1) ? 0 : SPACE_BETWEEN_CELLS;
        setMargins(rowLayout, 0, topMargin, 0, bottomMargin);
    }

    private void setMargins(View view, int left, int top, int right, int bottom) {
        LinearLayout.LayoutParams params = createLinearLayoutParams();
        params.setMargins(left, top, right, bottom);
        view.setLayoutParams(params);
    }

    private LinearLayout.LayoutParams createLinearLayoutParams() {
        return new LinearLayout.LayoutParams(
            LinearLayout.LayoutParams.WRAP_CONTENT, LinearLayout.LayoutParams.WRAP_CONTENT, 1.0f
        );
    }
}

tic_tac_toe_activity.xml

<RelativeLayout xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:tools="http://schemas.android.com/tools"
    android:layout_width="match_parent"
    android:layout_height="match_parent"
    android:paddingLeft="@dimen/activity_horizontal_margin"
    android:paddingRight="@dimen/activity_horizontal_margin"
    android:paddingTop="@dimen/activity_vertical_margin"
    android:paddingBottom="@dimen/activity_vertical_margin"
    tools:context=".TicTacToeActivity">

    <TextView
            android:layout_width="match_parent"
            android:layout_height="wrap_content"
            android:gravity="center"
            android:text="margin"
            android:id="@+id/textView"
            android:layout_alignParentTop="true"
            android:layout_alignParentLeft="true"
            android:textSize="64sp"/>

    <TextView
            android:layout_width="wrap_content"
            android:layout_height="wrap_content"
            android:id="@+id/gameScoreTextView"
            android:textSize="64sp"
            android:layout_alignParentTop="true"
            android:layout_alignParentRight="true"/>

    <TextView
            android:layout_width="match_parent"
            android:layout_height="wrap_content"
            android:text="margin"
            android:id="@+id/adTextView"
            android:layout_alignParentBottom="true"
            android:layout_centerHorizontal="true"
            android:textSize="74sp"/>

    <FrameLayout
            android:id="@+id/gameBoardFrameLayout"
            android:background="@color/light_green"
            android:layout_width="match_parent"
            android:layout_height="match_parent"
            android:layout_below="@+id/textView"
            android:layout_above="@+id/adTextView"
            android:layout_alignParentRight="true"
            android:layout_alignParentLeft="true">
    </FrameLayout>
</RelativeLayout>

cell_image_view.xml

<?xml version="1.0" encoding="utf-8"?>

<ImageView xmlns:android="http://schemas.android.com/apk/res/android"
           android:scaleType="fitXY"
           android:layout_width="match_parent"
           android:layout_height="match_parent"
           android:layout_weight="1" >

</ImageView>

GameModel.java

public interface GameModel {
    boolean emptyCell(Matrix.Position pos);
    Dimension getDimension();
    void addOnOpponentMoveListener(OnOpponentMoveListener listener);
    void addOnGameFinishedListener(OnGameFinishedListener listener);
    void onPlayerTurn(Matrix.Position turnPosition);
    void onViewIsReadyToStartGame();
}

GameModelImpl.java

public class GameModelImpl implements GameModel {

    private final Dimension dimension;
    private final GameJudge gameJudge;
    private final List<OnGameFinishedListener> onGameFinishedListeners;
    private final List<OnOpponentMoveListener> onOpponentMoveListeners;
    private final Matrix<Cell> gameBoard;
    private final Opponent opponent;
    private boolean opponentMovesFirst;

    public GameModelImpl(Dimension gameBoardDimension) {
        dimension = gameBoardDimension;
        gameBoard = new Matrix<Cell>(gameBoardDimension);
        initGameBoardByEmpty();
        gameJudge = new GameJudgeImpl(gameBoard);
        onOpponentMoveListeners = new ArrayList<OnOpponentMoveListener>();
        onGameFinishedListeners = new ArrayList<OnGameFinishedListener>();
        opponent = new StupidAIOpponent(gameBoard);
        opponentMovesFirst = false;
    }

    private void initGameBoardByEmpty() {
        gameBoard.forEach(new Matrix.OnEachHandler<Cell>() {
            @Override
            public void handle(Matrix<Cell> matrix, Matrix.Position pos) {
                gameBoard.set(pos, Cell.EMPTY);
            }
        });
    }

    @Override
    public boolean emptyCell(Matrix.Position pos) {
        return gameBoard.get(pos) == Cell.EMPTY;
    }

    @Override
    public Dimension getDimension() {
        return dimension;
    }

    @Override
    public void addOnGameFinishedListener(OnGameFinishedListener listener) {
        onGameFinishedListeners.add(listener);
    }

    @Override
    public void addOnOpponentMoveListener(OnOpponentMoveListener listener) {
        onOpponentMoveListeners.add(listener);
    }

    @Override
    public void onPlayerTurn(Matrix.Position turnPosition) {
        gameBoard.set(turnPosition, Cell.PLAYER);
        if (gameNotFinished()) {
            opponentMove();
        }
        GameInfo gameInfo = gameJudge.gameResultInfo();
        if (gameInfo.resultIsKnown()) {
            onGameFinished(gameInfo);
        }
    }

    private boolean gameNotFinished() {
        return !gameJudge.gameResultInfo().resultIsKnown();
    }

    private void opponentMove() {
        Matrix.Position opponentMovePos = opponent.positionToMove();
        gameBoard.set(opponentMovePos, Cell.OPPONENT);
        notifyOnOpponentMoveListeners(opponentMovePos);
    }

    private void notifyOnOpponentMoveListeners(Matrix.Position opponentMovePos) {
        for (OnOpponentMoveListener each : onOpponentMoveListeners) {
            each.onOpponentMove(opponentMovePos);
        }
    }

    private void onGameFinished(GameInfo gameInfo) {
        opponentMovesFirst = defineOpponentMovesFirst(gameInfo.gameResult());
        notifyOnGameFinishedListeners(gameInfo);
        initGameBoardByEmpty();
    }

    private boolean defineOpponentMovesFirst(GameResult gameResult) {
        return (gameResult == GameResult.OPPONENT_WINS) ||
               (opponentMovesFirst && gameResult == GameResult.DRAW);
    }

    private void notifyOnGameFinishedListeners(GameInfo gameInfo) {
        for (OnGameFinishedListener each : onGameFinishedListeners) {
            each.onGameFinished(gameInfo);
        }
    }

    @Override
    public void onViewIsReadyToStartGame() {
        if (opponentMovesFirst) {
            opponentMove();
        }
    }
}

Cell.java

public enum Cell {
    EMPTY, PLAYER, OPPONENT
}

OnGameFinishedListener.java

public interface OnGameFinishedListener {
    void onGameFinished(GameInfo gameInfo);
}

OnOpponentMoveListener.java

public interface OnOpponentMoveListener {
    void onOpponentMove(Matrix.Position movePos);
}

GameInfo.java

public class GameInfo {
    private final GameResult gameResult;
    private final List<Matrix.Position> cellsOnFire;

    public static GameInfo unknownResultInfo() {
        return new GameInfo(GameResult.UNKNOWN, new ArrayList<Matrix.Position>());
    }

    public static GameInfo drawResultInfo() {
        return new GameInfo(GameResult.DRAW, new ArrayList<Matrix.Position>());
    }

    public GameInfo(GameResult gameResult, List<Matrix.Position> cellsOnFire) {
        this.gameResult = gameResult;
        this.cellsOnFire = cellsOnFire;
    }

    public GameResult gameResult() {
        return gameResult;
    }

    public List<Matrix.Position> cellsOnFire() {
        return cellsOnFire;
    }

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

GameResult.java

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

GameJudge.java

public interface GameJudge {
    public GameInfo gameResultInfo();
}

GameJudgeImpl.java

public class GameJudgeImpl implements GameJudge {
    private final Matrix<Cell> gameBoard;
    private final int gameBoardDimension;

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

    @Override
    public GameInfo gameResultInfo() {
        for (int i = 0; i < gameBoardDimension; ++i) {
            GameInfo resultInfo = rowColumnResultInfo(i);
            if (resultInfo.resultIsKnown()) {
                return resultInfo;
            }
        }
        GameInfo resultInfo = diagonalsResultInfo();
        if (resultInfo.resultIsKnown()) {
            return resultInfo;
        }
        return gameBoardContainsEmptyCell()
                ? GameInfo.unknownResultInfo()
                : GameInfo.drawResultInfo();
    }

    private GameInfo rowColumnResultInfo(int index) {
        GameInfo rowResultInfo = rowResultInfo(index);
        if (rowResultInfo.resultIsKnown()) {
            return rowResultInfo;
        } else {
            return columnResultInfo(index);
        }
    }

    private GameInfo rowResultInfo(int row) {
        List<Matrix.Position> rowCellsPositions = rowCellsPositions(row);
        return resultInfoByCellsPositions(rowCellsPositions);
    }

    private List<Matrix.Position> rowCellsPositions(int row) {
        List<Matrix.Position> cells = new ArrayList<Matrix.Position>();
        for (int column = 0; column < gameBoardDimension; ++column) {
            cells.add(new Matrix.Position(row, column));
        }
        return cells;
    }

    private GameInfo resultInfoByCellsPositions(List<Matrix.Position> cellsPositions) {
        Matrix.Position firstCellOnLinePosition = cellsPositions.get(0);
        Cell firstCellOnLine = gameBoard.get(firstCellOnLinePosition);
        if (firstCellOnLine == Cell.EMPTY) {
            return GameInfo.unknownResultInfo();
        }
        for (int i = 1; i < gameBoardDimension; ++i) {
            Matrix.Position currentPosition = cellsPositions.get(i);
            Cell currentCell = gameBoard.get(currentPosition);
            if (firstCellOnLine != currentCell) {
                return GameInfo.unknownResultInfo();
            }
        }
        return new GameInfo(cellToResult(firstCellOnLine), cellsPositions);
    }

    private GameResult cellToResult(Cell cell) {
        if (cell == Cell.PLAYER) {
            return GameResult.PLAYER_WINS;
        } else if (cell == Cell.OPPONENT) {
            return GameResult.OPPONENT_WINS;
        }
        throw new IllegalArgumentException("Input cell must be not empty!");
    }

    private GameInfo columnResultInfo(int column) {
        List<Matrix.Position> columnCellsPositions = columnCellsPositions(column);
        return resultInfoByCellsPositions(columnCellsPositions);
    }

    private List<Matrix.Position> columnCellsPositions(int column) {
        List<Matrix.Position> cells = new ArrayList<Matrix.Position>();
        for (int row = 0; row < gameBoardDimension; ++row) {
            cells.add(new Matrix.Position(row, column));
        }
        return cells;
    }

    private GameInfo diagonalsResultInfo() {
        GameInfo leftUpperDiagonalResultInfo = leftUpperDiagonalResultInfo();
        if (leftUpperDiagonalResultInfo.resultIsKnown()) {
            return leftUpperDiagonalResultInfo;
        } else {
            return rightUpperDiagonalResultInfo();
        }
    }

    private GameInfo leftUpperDiagonalResultInfo() {
        return resultInfoByCellsPositions(leftUpperDiagonalPositions());
    }

    private List<Matrix.Position> leftUpperDiagonalPositions() {
        List<Matrix.Position> positions = new ArrayList<Matrix.Position>();
        for (int i = 0; i < gameBoardDimension; ++i) {
            positions.add(new Matrix.Position(i, i));
        }
        return positions;
    }

    private GameInfo rightUpperDiagonalResultInfo() {
        return resultInfoByCellsPositions(rightUpperDiagonalPositions());
    }

    private List<Matrix.Position> rightUpperDiagonalPositions() {
        List<Matrix.Position> positions = new ArrayList<Matrix.Position>();
        for (int i = 0; i < gameBoardDimension; ++i) {
            positions.add(new Matrix.Position(i, gameBoardDimension - i - 1));
        }
        return positions;
    }

    private boolean gameBoardContainsEmptyCell() {
        for (int row = 0; row < gameBoardDimension; ++row) {
            for (int column = 0; column < gameBoardDimension; ++column) {
                if (gameBoard.get(row, column) == Cell.EMPTY) {
                    return true;
                }
            }
        }
        return false;
    }
}

Opponent.java

public interface Opponent {
    Matrix.Position positionToMove();
}

StupidAIOpponent.java

public class StupidAIOpponent implements Opponent {

    private final Matrix<Cell> gameBoard;

    public StupidAIOpponent(Matrix<Cell> gameBoard) {
        this.gameBoard = gameBoard;
    }

    @Override
    public Matrix.Position positionToMove() {
        for (int row = 0; row < gameBoard.rows; ++row) {
            for (int column = 0; column < gameBoard.columns; ++column) {
                if (gameBoard.get(row, column) == Cell.EMPTY) {
                    return new Matrix.Position(row, column);
                }
            }
        }
        throw new RuntimeException("There is not empty cells!");
    }
}
\$\endgroup\$
2
\$\begingroup\$

Note that the following line violates the principle of information hiding:

            if (gameBoard.get(row, column) == Cell.EMPTY) {

Express this as:

            if (gameBoard.isEmpty(row, column)) {

In the following:

    if (rowResultInfo.resultIsKnown()) {
        return rowResultInfo;
    } else {
        return columnResultInfo(index);
    }

The else is superfluous:

    if (rowResultInfo.resultIsKnown()) {
        return rowResultInfo;
    }

    return columnResultInfo(index);

This naturally leads to a single return statement:

    return rowResultInfo.resultIsKnown() ? rowResultInfo : columnResultInfo(index);

Much of the code breaks encapsulation, and very little of the code is extensible. I'd recommend you read about self-encapsulation to see how to develop code that is extensible (i.e., subscribes to the Open-Closed Principle).

A lot of information is passed between various classes that incurs duplication. For example, all of these are a form of repetition:

gameBoard.get(pos) == Cell.EMPTY
if (gameBoard.get(row, column) == Cell.EMPTY) {
if (firstCellOnLine == Cell.EMPTY) {
if (cell == Cell.PLAYER) {
} else if (cell == Cell.OPPONENT) {

These are all examples of thinking about programming in terms of functions, rather than in terms of objects. Knowledge of an object's state should stay as close to the object as possible. In the cases above, there is no reason to expose the inner workings of the cell. The first step to hiding the cell's state is by eliminating its get accessor method:

if( cell.isTakenBy( Cell.EMPTY ) )
if( cell.isTakenBy( Cell.OPPONENT ) ) 
if( cell.isTakenBy( Cell.PLAYER ) ) 

These can also be written as:

if( cell.isEmpty() )
if( cell.isOpponent() ) 
if( cell.isPlayer() ) 

I get the feeling that there should be no distinction between an "opponent" and a "player" -- there should be only players, one being the "active" player. This would allow for variations such as 4-player T-T-T. You could then write, for example:

if( cell.isTakenBy( currentPlayerToken ) )
if( !cell.isTakenBy( currentPlayerToken ) && !cell.isEmpty() )

For little effort with this approach, the game can use tokens to occupy board positions, rather than a "player" being in a cell. (That is, when playing TTT, the players themselves aren't actually occupying different spaces in the grid, rather their tokens are -- in this case an X or an O. With the model as developed, it strongly implies that a PLAYER or an OPPONENT is in a cell, which, frankly, is nonsensical for TTT, but would make sense for other games where the player physically occupies the board space.)

By making the design a little more generic, it can relatively easily apply to more games.

Note that IconRandomizer isn't a "class" per se. A class, strictly speaking, must have both attributes and behaviour.

\$\endgroup\$
0

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.