HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Tic-Tac-Toe classes according to SOLID principles

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
principlestoetictacclassessolidaccording

Problem

I have these classes for my Tic-Tac-Toe game project. I am very new to object oriented analysis and design. Can anyone help determine if SOLID principles are preserved in this code or if it is violating some of them?

```
public class InputReader {

private BufferedReader inputReader;

public InputReader() {
inputReader = new BufferedReader(new InputStreamReader(System.in));
}

public String getUserInput(String messagePrompt) {
String userInput = null;
System.out.println(messagePrompt);
try {
userInput = inputReader.readLine();
} catch (IOException e) {
e.printStackTrace();
}
return userInput;
}

public void close() {
try {
inputReader.close();
} catch (IOException e) {
e.printStackTrace();
}
}

}

public class Board {

private static final int SIZE = 3;

private int occupiedCells = 0;

private char[][] board = new char[SIZE][SIZE];

private Set validCells;

public Board() {
validCells = new HashSet();
for (int row = 0; row = 0; row--) {
System.out.print(row + " ");
for (column = 0; column . Eg. 20 for top left cell.");
board.checkForValidCell(playerInput);
markBoardCell(userMark, playerInput);
} catch (CellOccupiedException wrongCell) {
System.out.println(wrongCell.getErrorMessage());
playerInput = null;
} catch (InvalidInputException invalidInput) {
System.out.println(invalidInput.getErrorMessage());
playerInput = null;
}
return playerInput;

}

private void markBoardCell(char mark, String playerInput) {
board.markCell(mark, Integer.parseInt(playerInput.substring(0, 1)),
Integer.parseInt(playerInput.substring(1)));

}

private void printResult(String winnerOrDraw) {
board.print();
System.out.pri

Solution

This looks well designed and implemented. Here are a couple things I noticed:

Redundant checking:

In checkRow(int), you redundantly check whether the first square is ' ' every time. I would check this first, then check whether the rest of the cells are equal to it:

if (cell1 == ' ') { return false; }


Looping over a known range:

for loops are designed for looping over a known range, and while loops are designed for looping over an unknown range until a condition is met. So, instead of:

int column = 1;
while (column < SIZE) {
    if (cell1 == ' ' || cell1 != board[row][column]) {
        return false;
    }
    column++;
}


You can use a for loop:

for (int column = 1; column < SIZE; column++)
{
    if (cell1 != board[row][column]) {
        return false;
    }
}


But here you are iterating over an array, and could possibly make use of the enhanced for:

for (char cell : board[row])
{
    if (cell == ' ' || cell != board[row][0]) {
        return false;
    }
}


This way, you have the redundant check that each cell is not ' ', but otherwise, the syntax is cleaner. This is just something to consider.

Common Exceptions:

throw new InvalidInputException("Error..!!! Invalid Input");


Exceptions are supposed to be thrown over uncommon occurrences. If the user enters the input with the keyboard, incorrect values are going to be common occurrences and should not throw an exception. I would just loop without an exception until the user input the correct value.

Code Snippets

if (cell1 == ' ') { return false; }
int column = 1;
while (column < SIZE) {
    if (cell1 == ' ' || cell1 != board[row][column]) {
        return false;
    }
    column++;
}
for (int column = 1; column < SIZE; column++)
{
    if (cell1 != board[row][column]) {
        return false;
    }
}
for (char cell : board[row])
{
    if (cell == ' ' || cell != board[row][0]) {
        return false;
    }
}
throw new InvalidInputException("Error..!!! Invalid Input");

Context

StackExchange Code Review Q#105820, answer score: 4

Revisions (0)

No revisions yet.