patternjavaMinor
Connect Four game without diagonals check or validation
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]
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:
Similarly, instead of initializing
Code duplication
This snippet is duplicated in
It would be better to move this to a helper method.
Unnecessary execution
When a player wins,
you could return immediately from
no need to continue checking all rows and columns.
The helper method I mentioned in the previous section could return a
with a
When the returned value is
the caller in
Code organization
Instead of
You could still have a
but there's no reason for these two independent aspects to be implemented in the same method.
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.