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

Connect Four game without diagonals check or validation

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

Problem

This is a simple implementation of Connect four in Java. It is working correctly so far.
What I haven't done yet are the diagonals check and input validation.

```
package game;

import java.util.Scanner;
//TODO check for diagonals, input validation

public class Game {
private char[][] gameBoard;
private State gameState;
private Turn turn;
private int[] possiblePositions;
private static final int BOARD_HEIGHT = 6;
private static final int BOARD_WIDTH = 7;

enum Turn {
PLAYER_ONE, PLAYER_TWO
}

enum State {
PLAYER_1_WIN, PLAYER_2_WIN, RUNNING
}

public Game() {
gameBoard = new char[BOARD_HEIGHT][BOARD_WIDTH];
for (int row = 0; row < BOARD_HEIGHT; row++) {
for (int col = 0; col < BOARD_WIDTH; col++) {
gameBoard[row][col] = '□';
}
}
turn = Turn.PLAYER_ONE;
gameState = State.RUNNING;
possiblePositions = new int[] { 5, 5, 5, 5, 5, 5, 5 };
}

public void playerMove(int column) {
switch (turn) {
case PLAYER_ONE:
gameBoard[possiblePositions[column]][column] = '■';
turn = Turn.PLAYER_TWO;
break;
case PLAYER_TWO:
gameBoard[possiblePositions[column]][column] = '⊠';
turn = Turn.PLAYER_ONE;
break;
}
possiblePositions[column]--;
}

public void printBoard() {
System.out.println("0 1 2 3 4 5 6");
for (int row = 0; row < BOARD_HEIGHT; row++) {
for (int col = 0; col < BOARD_WIDTH; col++) {
System.out.print(gameBoard[row][col] + " ");
}
System.out.println();
}
}

public void checkRowAndCol() {
int chainLength = 1;
for (int row = 0; row < BOARD_HEIGHT - 1; row++) {
for (int col = 0; col < BOARD_WIDTH - 1; col++) {
if (gameBoard[row][col] == gameBoard[row][col + 1] && gameBoard[row][col]

Solution

Magic values

The implementation is mostly fine, except for the widespread magic values that would be better as constants, for example:

private static final char SYMBOL_EMPTY = '□';
private static final char SYMBOL_PLAYER1 = '■';
private static final char SYMBOL_PLAYER2 = '⊠';


Similarly, instead of initializing possiblePositions with new int[] { 5, 5, 5, 5, 5, 5, 5 }, it would be better to derive the correct value from the BOARD_WIDTH and BOARD_HEIGHT constants.

Code duplication

This snippet is duplicated in checkRowAndCol:

if (chainLength == 4) {
    switch (gameBoard[row][col]) {
        case '■':
            gameState = State.PLAYER_1_WIN;
            break;
        case '⊠':
            gameState = State.PLAYER_2_WIN;
            break;
    }
}


It would be better to move this to a helper method.

Unnecessary execution

When a player wins,
you could return immediately from checkRowAndCol,
no need to continue checking all rows and columns.

The helper method I mentioned in the previous section could return a boolean,
with a true value if there is a winner.
When the returned value is true,
the caller in checkRowAndCol could return immediately.

Code organization

Instead of checkRowAndCol, it would be better to separate to checkRow and checkCol methods.
You could still have a checkRowAndCol method that calls the other two,
but there's no reason for these two independent aspects to be implemented in the same method.

Code Snippets

private static final char SYMBOL_EMPTY = '□';
private static final char SYMBOL_PLAYER1 = '■';
private static final char SYMBOL_PLAYER2 = '⊠';
if (chainLength == 4) {
    switch (gameBoard[row][col]) {
        case '■':
            gameState = State.PLAYER_1_WIN;
            break;
        case '⊠':
            gameState = State.PLAYER_2_WIN;
            break;
    }
}

Context

StackExchange Code Review Q#150774, answer score: 2

Revisions (0)

No revisions yet.