4
\$\begingroup\$

To practice the MVC pattern and Unittesting in Java I decided to make a simple TicTacToe Console Application.

The features of this App are:

  • Multiplayer-Mode
  • Singleplayer-Mode (that should always result in a draw)

My questions are:

  • Have I applied the concept of the MVC pattern correctly?
  • I implemented Unittests for the SimpleAI class. Are my tests appropriate? Can i make them more dynamic(now they just test for a specific case)?
  • Are there any heavy no-noes in my code I should watch out for in the future?

Here is the Github link: https://github.com/Baumgartner-Lukas/TTT.git

Code:

View:

import controller.GameController;
import controller.SimpleAI;
import model.GameBoard;
import view.GameFieldView;

import java.io.IOException;

public class TicTacToe {

    public static void main(String[] args) throws IOException {
        GameBoard model = new GameBoard();
        GameFieldView view = new GameFieldView();
        SimpleAI sai = new SimpleAI();
        GameController controller = new GameController(model, view, sai);

        controller.play();
    }
}

Model:

Stones:

package model;

public enum Stone {
        X("X"), O("O"), NONE(" ");

        private final String stone;

        Stone(String stone){
            this.stone = stone;
        }

    @Override
    public String toString() {
        return stone;
    }
}

GameBoard:

package model;

public class GameBoard {
    public static final int SIZE = 3;
    public static final int TURNS = SIZE * SIZE;

    private Stone grid[][] = new Stone[SIZE][SIZE];

    //Fill the new GameBoard with NONE(" ") Stones
    public GameBoard(){
        for(int r = 0; r < SIZE; r++){
            for(int c = 0; c < SIZE; c++){
                grid[r][c] = Stone.NONE;
            }
        }
    }

    public Stone getStone(int row, int col) {
        return grid[row][col];
    }

    public void setStone(int row, int col, Stone stone) {
            grid[row][col] = stone;
    }
}

Controller:

package controller;

import model.GameBoard;
import model.Stone;
import view.GameFieldView;

import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;

public class GameController {
    private BufferedReader reader;
    private GameFieldView view;
    private GameBoard model;
    private SimpleAI simpleAI;
    private boolean SimpleAIisActive = false;


    public GameController(GameBoard model, GameFieldView view, SimpleAI sai) {
        this.model = model;
        this.view = view;
        this.simpleAI = sai;
        this.reader = new BufferedReader(new InputStreamReader(System.in));
    }

    private int counter = 0; //counter to determine which players turn it is and when the max. turns are reached

    public void play() throws IOException {
        int[] input = new int[2];
        System.out.println("Single- or Multiplayer (S | M): ");
        String opt = reader.readLine();
        if (opt.trim().toLowerCase().equals("s")) {
            SimpleAIisActive = true;
        }
        while (counter < GameBoard.TURNS) {
            //print Field
            view.printGameField(model);
            if (SimpleAIisActive && counter % 2 == 1) {
                simpleAI.updateGameBoard(model);
                if (hasWon()) {
                    System.out.printf("%n AI has won! You noob %n");
                    view.printGameField(model);
                    return;
                }
                System.out.printf("%n AI-Turn %n%n");
                view.printGameField(model);
                counter++;
            }
            //prompt players for their turn
            try {
                input = prompt();
                while (input[0] < 0 || input[0] > 2 || input[1] < 0 || input[1] > 2) {
                    System.err.printf("Row and Col must be between 0 - 2. %n");
                    input = prompt();
                }
                while (!isValidMove(input)) {
                    System.err.printf("This field is already taken! %n");
                    input = prompt();
                }
            } catch (IOException ioe) {
                System.err.println("Error reading input");
                ioe.printStackTrace();
            }
            placeStone(input);
            if (hasWon()) {
                view.printGameField(model);
                System.out.printf("%nPlayer %d has won! GG EZ %n", counter % 2 + 1);
                return;
            }
            counter++;
        }
        view.printGameField(model);
        System.out.println("Game finished with a draw!");
    }

    /**
     * For readability
     *
     * @return returns True if one of those conditions is true
     */
    private boolean hasWon() {
        return checkStraight() || checkDiagonal();
    }

    /**
     * Checks if there are 3 same Stones in a diagonal line
     *
     * @return returns true if 3 Stones are found. False if not.
     */
    private boolean checkDiagonal() {
        //middle Stone
        Stone s = model.getStone(1, 1);
        return s != Stone.NONE && (
                s == model.getStone(0, 0) && s == model.getStone(2, 2) ||
                        s == model.getStone(0, 2) && s == model.getStone(2, 0));
    }

    /**
     * Checks if there are 3 same Stones in a straight line
     *
     * @return returns true if 3 Stones are found. False if not.
     */
    protected boolean checkStraight() {
        int i = 0;
        while (i < 3) {
            Stone sCol = model.getStone(0, i);
            Stone sRow = model.getStone(i, 0);
            if (sCol == model.getStone(1, i) && sCol == model.getStone(2, i) && sCol != Stone.NONE) return true;
            if (sRow == model.getStone(i, 1) && sRow == model.getStone(i, 2) && sRow != Stone.NONE) return true;
            i++;
        }
        return false;
    }

    /**
     * Checks if the user puts a Stone on a valid (empty) position on the board
     *
     * @param input row and col of field where to set the stone
     * @return returns true if the input field is empty
     */
    protected boolean isValidMove(int[] input) {
        int row = input[0];
        int col = input[1];
        return (model.getStone(row, col) == Stone.NONE);
    }

    protected void placeStone(int[] input) {
        int row = input[0];
        int col = input[1];

        if (counter % 2 == 0) {
            model.setStone(row, col, Stone.X);
        } else {
            model.setStone(row, col, Stone.O);
        }
    }

    /**
     * Prompts the player for the position where to set the stone
     *
     * @return returns the inputarray [0] = row, [1] = col
     * @throws IOException Throws an Exception if the inputvalues are out of bound of the gamefield
     */
    private int[] prompt() throws IOException {
        int player;
        int[] input = new int[2];
        player = counter % 2 + 1;
        System.out.println("==========");
        System.out.printf("It is player %d's turn! %n", player);
        System.out.println("Give Row: ");
        input[0] = Integer.parseInt(reader.readLine());
        System.out.println("Give Col: ");
        input[1] = Integer.parseInt(reader.readLine());

        return input;
    }
}

Singleplayer "AI":

package controller;

import model.GameBoard;
import model.Stone;

import static java.util.concurrent.ThreadLocalRandom.current;

public class SimpleAI {
    int counter = 1;

    public SimpleAI() {
    }

    /**
     * public method to use in the GameController class
     * @param model model of the current game board
     */
    protected void updateGameBoard(GameBoard model) {
        alwaysDraw(model);
        counter++;
    }

    /**
     * Adds stones randomly on the field. Easiest difficulty.
     * @param model model of the current game board
     */
    private void addRandomStone(GameBoard model) {
        int row = getRandomNumber();
        int col = getRandomNumber();
        while (model.getStone(row, col) != Stone.NONE) {
            row = getRandomNumber();
            col = getRandomNumber();
        }
        model.setStone(row, col, Stone.O);
    }

    /**
     * Adds stones in a way to the board, that should always lead to a draw
     * @param model model of the current game board
     */
    private void alwaysDraw(GameBoard model) {
        //if there is no stone set in the middle, set a stone in the middle
        if (counter == 1) {
            if (model.getStone(1, 1) == (Stone.NONE) && counter == 1) {
                model.setStone(1, 1, Stone.O);
            } else {
                //if there is a stone in the middle, set the stone in one of the edges
                model.setStone(getRandomEvenNumber(), getRandomEvenNumber(), Stone.O);
            }
        } else {
            if (!checkDiagonal(model)) {
                if (!checkRows(model)) {
                    if (!checkCols(model)) {
                        if (!checkCorners(model)) {
                            checkStraights(model);
                        }
                    }
                }
            }
        }
    }

    /**
     * checks if there is a free space on any of the middle lanes(0:1, 2:1, 1:0, 1:2)
     * @param model current model of the game board
     */
    private void checkStraights(GameBoard model) {
        int r = getRandomNumber();
        int c = getRandomNumber();
        if(model.getStone(r, c) == Stone.NONE && (r + c > 0 && r + c < 4)) {
            model.setStone(r, c, Stone.O);
        }else{
            checkStraights(model);
        }

    }

    /**
     * checks if any of the corners of the game board is free to set a stone
     * @param model current model of the game board
     * @return  true if there was a free corner and a friendly stone was set
     *          false if no corner was empty
     */
    private boolean checkCorners(GameBoard model) {
        int cornerCount = 0;
        for (int r = 0; r < 2; r++) {
            for (int c = 0; c < 2; c++) {
                if (model.getStone(r * 2, c * 2) == Stone.X) {
                    cornerCount++;
                    if (cornerCount < 2 && model.getStone(r * 2, c * 2) == Stone.NONE) {
                        model.setStone(r * 2, c * 2, Stone.O);
                        return true;
                    }
                }
            }
        }
        return false;
    }

    /**
     * Checks if there are two enemy stones already in a diagonal position. If so, make the according counter move
     * If there is no enemy stone in the middle, skip that check.
     * @param model model of the current game board
     * @return  false if there is no enemy stone in the middle.
     *          true if there are two enemy stones in a diagonal pos and a counter move was made.
     */
    private boolean checkDiagonal(GameBoard model) {
        if (model.getStone(1, 1) != Stone.X) return false;
        if (model.getStone(1, 1) == Stone.X &&
                model.getStone(0, 0) == Stone.X &&
                model.getStone(2, 2) != Stone.O) {
            model.setStone(2, 2, Stone.O);
            return true;
        } else if (model.getStone(1, 1) == Stone.X &&
                model.getStone(0, 2) == Stone.X &&
                model.getStone(2, 0) != Stone.O) {
            model.setStone(2, 0, Stone.O);
            return true;
        } else if (model.getStone(1, 1) == Stone.X &&
                model.getStone(2, 0) == Stone.X &&
                model.getStone(0, 2) != Stone.O) {
            model.setStone(0, 2, Stone.O);
            return true;
        } else if (model.getStone(1, 1) == Stone.X &&
                model.getStone(2, 2) == Stone.X &&
                model.getStone(0, 0) != Stone.O) {
            model.setStone(0, 0, Stone.O);
            return true;
        }
        return false;
    }

    /**
     * Checks all rows if two enemy stones are in the same row
     * @param model model of the current game board
     * @return  false if there are no two enemy stones in the same row.
     *          true if there are two enemy stones in the same row and a counter move was made
     */
    private boolean checkRows(GameBoard model) {
        for (int r = 0; r < 3; r++) {
            int stoneCount = 0;
            for (int c = 0; c < 3; c++) {
                if (model.getStone(r, c) == Stone.X) {
                    stoneCount++;
                }else if(model.getStone(r,c) == Stone.O){
                    stoneCount--;
                }
            }
            if (stoneCount == 2) {
                counterMoveRow(model, r);
                return true;
            }
        }
        return false;
    }

    /**
     * Checks columns if for enemy stones
     * @param model model of the current game board
     * @return  false if threre are no two enemy stones in the same column
     *          true if there are two enemy stones in the same column and a counter move was made
     */
    private boolean checkCols(GameBoard model) {
        for (int c = 0; c < 3; c++) {
            int stoneCount = 0;
            for (int r = 0; r < 3; r++) {
                if (model.getStone(r, c) == Stone.X) {
                    stoneCount++;
                }else if(model.getStone(r,c) == Stone.O) stoneCount--;
            }
            if (stoneCount == 2) {
                counterMoveCol(model, c);
                return true;
            }
        }
        return false;
    }

    /**
     * Sets a friendly stone in the appropriate position
     * @param model model of the current game board
     * @param c column in which the two enemy stones were found
     */
    private void counterMoveCol(GameBoard model, int c) {
        for (int r = 0; r < 3; r++) {
            if (model.getStone(r, c) == Stone.NONE) model.setStone(r, c, Stone.O);
        }
    }

    /**
     * Sets a friendly stone in the appropriate position
     * @param model model of the current game board
     * @param r row in which the two enemy stones were found
     */
    private void counterMoveRow(GameBoard model, int r) {
        for (int c = 0; c < 3; c++) {
            if (model.getStone(r, c) == Stone.NONE) model.setStone(r, c, Stone.O);
        }
    }

    /**
     * generates a random integer number with a range between 0 and 2
     * @return random int between 0 and 2
     */
    private int getRandomNumber() {
        return current().nextInt(0, 3);
    }

    /**
     * generates an even random number (0 or 2)
     * used to setting a stone in one of the corners
     * @return random even int (0 or 2)
     */
    private int getRandomEvenNumber() {
        return current().nextInt(0, 2) * 2;
    }

}

Tests:

package controller;

import model.Stone;
import org.junit.Before;
import model.GameBoard;
import org.junit.Test;

import static org.junit.Assert.*;

public class SimpleAITest {
    private GameBoard model;
    private SimpleAI sai;

    @Before
    public void setUp() throws Exception {
        model = new GameBoard();
        sai = new SimpleAI();

        sai.counter = 2;
    }

    @Test
    public void stoneIsSetCorrectlyRowCheck() {
        setUpForRowCheck();

        sai.updateGameBoard(model);

        assertEquals(Stone.O, model.getStone(0, 2));
    }

    @Test
    public void stoneIsSetCorrectlyColCheck() {
        setUpForColCheck();

        sai.updateGameBoard(model);

        assertEquals(Stone.O, model.getStone(0, 2));
    }

    @Test
    public void stoneIsSetCorrectlyDiagonalCheck() {
        setUpForDiagonal();

        sai.updateGameBoard(model);

        assertEquals(Stone.O, model.getStone(2, 0));
    }

    @Test
    public void stoneIsSetCorrectlyStraightCheck() {
        setUpForStraight();

        sai.updateGameBoard(model);

        assertTrue(model.getStone(1,0) == Stone.O ||
                model.getStone(1,2) == Stone.O);
    }


    private void setUpForStraight() {
        model.setStone(0, 1, Stone.X);
        model.setStone(2, 1, Stone.X);

        model.setStone(1, 1, Stone.X);
    }

    private void setUpForRowCheck() {
        model.setStone(0, 0, Stone.X);
        model.setStone(0, 1, Stone.X);
        model.setStone(2, 0, Stone.X);
        model.setStone(2, 2, Stone.X);

        model.setStone(1, 1, Stone.O);
        model.setStone(2, 1, Stone.O);
    }

    private void setUpForColCheck() {
        model.setStone(2, 2, Stone.X);
        model.setStone(1, 2, Stone.X);

        model.setStone(1, 1, Stone.O);

    }

    private void setUpForDiagonal() {
        model.setStone(1, 1, Stone.X);
        model.setStone(0, 2, Stone.X);

        model.setStone(2, 2, Stone.O);
    }
}
\$\endgroup\$
2
\$\begingroup\$

Thanks for sharing your code.

Have I applied the concept of the MVC pattern correctly?

No.

In the MVC pattern thr controller manuputales the model and the view handles User interaction by displaying the models current state taking the user input and passing it to the controller.

In your implementation the controller does the user interaction.


I implemented Unittests for the SimpleAI class. Are my tests appropriate?

UnitTests have more that one goal:

  • UTs verify the desired behavior of the tested code
  • UTs document the current behavior of the tested code
  • UTs are examples of how to use the tested code

You might see yourself how well your code reaches each goal...


Can i make them more dynamic(now they just test for a specific case)?

UnitTest are meant to be specific.

Each test method verifies a single assumption about the behavior of the tested code. Therefore you cannot write "generic" test to be reused with other code to test.


Are there any heavy no-noes in my code I should watch out for in the future?

Naming

Finding good names is the hardest part in programming. So always take your time to think carefully of your identifier names.

Please read (and follow) the Java Naming Conventions.

Your variable SimpleAIisActive should start with a lower case letter and since it holds a boolean it should start with is, has, can or alike so it might be isSimpleAiActive.

avoid single character names

Since the number of characters is quite limited in most languages you will soon run out of names. This means that you either have to choose another character which is not so obviously connected to the purpose of the variable. And/or you have to "reuse" variable names in different contexts. Both makes your code hard to read and understand for other persons. (keep in mind that you are that other person yourself if you look at your code in a few month!)

On the other hand in Java the length of identifier names is virtually unlimited. There is no penalty in any way for long identifier names. So don't be stingy with letters when choosing names.

prefer OOish solutions over procedural approaches

There is nothing wrong with procedural approaches in general, but Java is an object oriented (OO) programming language and if you want to become a good Java programmer then you should start solving problems in an OO way.

But OOP doesn't mean to "split up" code into random classes.

The ultimate goal of OOP is to reduce code duplication, improve readability and support reuse as well as extending the code.

Doing OOP means that you follow certain principles which are (among others):

  • information hiding / encapsulation
  • single responsibility / separation of concerns
  • same level of abstraction
  • KISS (Keep it simple (and) stupid.)
  • DRY (Don't repeat yourself.)
  • "Tell! Don't ask."
  • Law of Demeter ("Don't talk to strangers!")

In you code the change of the current user is an example of a procedural approach. You have a counter variable and an calculate the current user based on that each time.

If you would consider the current user being an object represented by its Stone You could have it this way:

private static final int CURRENT_PLAYER = 0;
private final List<Stone> players = new ArrayList<>(Arrays.asList(Stone.X,Stone.O));
//...    
protected void placeStone(int[] input) {
    int row = input[0];
    int col = input[1];    
    model.setStone(row, col, players.get(CURRENT_PLAYER));        
}
//...    
protected void updateGameBoard(GameBoard model) {
    alwaysDraw(model);
    players.add(players.remove(CURRENT_PLAYER));
}
//...    
System.out.printf("%nPlayer %d has won! GG EZ %n", players.get(CURRENT_PLAYER));
\$\endgroup\$
  • \$\begingroup\$ Thanks for your answer! But could you please answer this question in more detail? "1: UnitTests have more than one goal: [...] You might see yourself how well your code reaches each goal..." No I honestly don't :D That's why I asked in the first place ;) \$\endgroup\$ – helloWorld Mar 1 '18 at 7:05
1
\$\begingroup\$

could you please answer this question in more detail? "1: UnitTests have more than one goal: [...]"

UTs verify the desired behavior of the tested code

Your unit test do that (sort of).

One important point about unit tests is that they test a units "public" behavior in isolation. This means any other code not belonging to the tested unit should be replaced by test doubles. The easiest way to create test doubles is to use a mocking framework (like Mockito).

"Public behavior" of a unit is the return values and its communication with other units (and not necessarily public methods). The mocking frameworks provide infrastructure to check this communication.

You code tests the unit SimpleAI. You verify the behavior of the tested unit by looking at the state change of another unit: GameBoard. This relies on the unit GameBoard to work correctly. This is OK for a different type of test: integration testing, which verifies that the units work together correctly. But for a unit test the consequence is that, if a test fails, you cannot immediately tell if it is because of a defect in unit SimpleAI or in unit GameBoard. You have to debug.

If your test was like this there would be no doubt:

public class SimpleAITest {        
    @Rule public MockitoRule mockitoRule = MockitoJUnit.rule(); 
    @Mock private GameBoard model;
    private SimpleAI sai;

    @Before
    public void setUp() throws Exception {
        doReturn(Stone.NONE).when(model).getStone(anyInt(), anyInt());
        sai = new SimpleAI();
        sai.counter=2;
   }

   @Test
   public void stoneIsSetCorrectlyRowCheck() {
        setUpForRowCheck();

        sai.updateGameBoard(model);

        Mockito.verify(model).setStone(0, 2, Stone.O));
   }
   private void setUpForRowCheck() {
        doReturn(Stone.X).when(model).getStone(0, 0);
        doReturn(Stone.X).when(model).getStone(0, 1);
        doReturn(Stone.X).when(model).getStone(2, 0);
        doReturn(Stone.X).when(model).getStone(2, 2);

        doReturn(Stone.O).when(model).getStone(1, 1);
        doReturn(Stone.O).when(model).getStone(2, 1);
    }
}

Since your current model is just a wrapper for an array one may argue that this "overkill" with no benefit, and your'e right.

But: this is an educational example.

What if your model is in a database? Using a real database in your tests would make them slow and they will fail if your database is not available for some reason (bad network, service not started and alike).

Or what if you model does some kind of validation? E.g.: you pre condition is invalid since Player X cannot have 2 more stones on the board then player O. If your board would check that you could not do this setup.

In short: your tests would fail for the wrong reason...

So for unit tests in your training projects you should always use mocks for dependencies just to get used to it. In your real projects you only mock dependencies that have business logic (which your GameBoard class does not).

UTs document the current behavior of the tested code

This starts with the name of the test method.

@Test
public void stoneIsSetCorrectlyRowCheck() {

This name leaves us with a question: What does "correct" mean?

The name should express the expectation verified as exact as possible. In contrast to method names in the production code test method names should be verbose:

@Test
public void preferesCornerOverEdgeToBlockOpponentsWin() {

UTs are examples of how to use the tested code

There is not much to say here.

The fact that you have unit tests reaches that goal.

But you should have a close look at the test:

  • is the name of the called method really expressing what it does?
  • does the test explain the meaning of the parameters
  • does the test explain the (for this test) relevant properties of the parameters?
\$\endgroup\$
  • \$\begingroup\$ Thank you very much for that detailed answer! I'm going to look into Mockito, didn't know such things exist :) With the naming of things. I'm not a native speaker, so finding descriptive names is not always that easy for me, but I'm going to work on that as well :) \$\endgroup\$ – helloWorld Mar 1 '18 at 9:12
  • \$\begingroup\$ @helloWorld "With the naming of things. I'm not a native speaker" Your test method names don't need to be "high literature". They just should describe the (part of the) business problem the code under test solves. \$\endgroup\$ – Timothy Truckle Mar 1 '18 at 11:33

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.