Code Review Asked by L292092 on December 31, 2021
I’ve included the files that I felt were relevant (i.e I haven’t included the individual pieces in the inheritance hierarchy). I’m particularly curious as to whether the system as a whole is constructed well. I’ve used MVC along with the Observer pattern to decouple the model and view.
Controller.java
package chess;
import java.io.File;
import java.io.FileInputStream;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.ObjectInputStream;
import java.io.ObjectOutputStream;
import java.util.Observable;
import java.util.Observer;
public class Controller implements Observer {
private Board board;
private final View view;
private final MinimaxAI ai;
private Position startOfPlayerMove;
private Position endOfPlayerMove;
private Team currentTeam;
public Controller() {
board = new Board();
view = new View();
setupBoardImages();
view.addObserver(this);
ai = new MinimaxAI(4, Team.WHITE);
}
// Main control method for entire program
public void run() {
currentTeam = Team.WHITE;
Move move;
GameStatus status;
boolean running = true;
while (running) {
// Check if there's a checkmate or stalemate. If there is, end of game
status = board.getGameStatus(currentTeam);
if (status == GameStatus.CHECKMATE || status == GameStatus.STALEMATE) {
view.gameOverMessage(status, currentTeam);
running = false;
continue;
}
move = getMove();
// Check if move follows the rules of Chess. If not, repeat turn
if (!board.isValidMove(move, currentTeam)) {
view.invalidMoveMessage(move);
continue;
}
// Attempt to make move. If move results in the mover being checked, repeat turn
if (!board.makeMove(move)) {
view.checkMessage(currentTeam);
continue;
}
// Update GUI and switch to next player
updateView(move);
view.moveMessage(move);
currentTeam = getNextTurn();
}
}
// Maps pieces on the board to the view
private void setupBoardImages() {
for (int row = 0; row < 8; row++) {
for (int column = 0; column < 8; column++) {
Position position = new Position(row, column);
if (board.pieceAt(position) != null)
view.updateTile(position, board.pieceAt(position).toString());
else
view.clearTile(position);
}
}
}
private Move getMove() {
if (currentTeam == Team.WHITE)
return ai.pickMove(board);
else
return pickPlayerMove();
}
private Move pickPlayerMove() {
while (startOfPlayerMove == null || endOfPlayerMove == null)
waitForValidInput();
Move ret = new Move(startOfPlayerMove, endOfPlayerMove);
resetMove();
return ret;
}
private void waitForValidInput() {
try {
Thread.sleep(10);
} catch (InterruptedException e) {
e.printStackTrace();
}
}
private Team getNextTurn() {
return Team.otherTeam(currentTeam);
}
// Update GUI with new state of board resulting from a move
private void updateView(Move move) {
String updateNewPiecePos = board.pieceAt(move.destination()).toString();
view.clearTile(move.start());
view.updateTile(move.destination(), updateNewPiecePos);
}
@Override
public void update(Observable gui, Object information) {
switch (view.getUpdateType()) {
case SAVE:
save(information);
break;
case LOAD:
load(information);
break;
case MOVE:
updatePlayerMove(information);
break;
default:
throw new AssertionError("Enum doesn't seem to match with any supported types");
}
}
private void updatePlayerMove(Object object) {
if (!(object instanceof Position))
throw new AssertionError("There doesn't seem to be a position here");
Position position = (Position) object;
if (isValidEndOfMove(position))
endOfPlayerMove = position;
else {
startOfPlayerMove = position;
endOfPlayerMove = null;
}
}
private boolean isValidEndOfMove(Position position) {
Piece selectedPiece = board.pieceAt(position);
return (selectedPiece == null || selectedPiece.getTeam() != currentTeam) && startOfPlayerMove != null;
}
private void save(Object object) {
if (!(object instanceof File))
throw new AssertionError("There doesn't seem to be a file here");
File file = (File) object;
try (FileOutputStream fileStream = new FileOutputStream(file);
ObjectOutputStream os = new ObjectOutputStream(fileStream)) {
os.writeObject(board);
} catch (IOException e) {
e.printStackTrace();
view.fileIOError();
}
}
private void resetMove() {
startOfPlayerMove = null;
endOfPlayerMove = null;
}
private void load(Object object) {
if (!(object instanceof File))
throw new AssertionError("There doesn't seem to be a file here");
File file = (File) object;
try (FileInputStream fileStream = new FileInputStream(file);
ObjectInputStream os = new ObjectInputStream(fileStream)) {
board = (Board) os.readObject();
resetMove();
setupBoardImages();
} catch (IOException | ClassNotFoundException e) {
e.printStackTrace();
view.fileIOError();
}
}
}
Board.java
package chess;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Stack;
public class Board implements Serializable {
private final Piece[][] board;
// Cache is used to save moves in case you want to reverse them.
private final Stack<Piece> deletedPieceCache;
private final Stack<Move> moveCache;
private final Stack<Position> pawnToQueenConversionCache;
// Maps a pieces string representation onto it's relative value
private final Map<String, Integer> heuristicMap;
public Board() {
board = new Piece[8][8];
deletedPieceCache = new Stack<>();
moveCache = new Stack<>();
pawnToQueenConversionCache = new Stack<>();
heuristicMap = new HashMap<>();
buildHeuristicMapping();
addPieces(0, 1, Team.WHITE);
addPieces(7, 6, Team.BLACK);
}
public void reverseLastMove() {
Move move = moveCache.pop();
Position start = move.start();
Position end = move.destination();
board[start.row()][start.column()] = pieceAt(end);
board[end.row()][end.column()] = deletedPieceCache.pop();
checkForReversePawnReplacement();
}
// Returns true if last move was successful, false if unsuccessful
public boolean makeMove(Move move) {
Position start = move.start();
Position end = move.destination();
Team team = pieceAt(start).getTeam();
cacheMove(move, end);
movePiece(start, end);
checkForPawnReplacement(start, end);
if (isChecked(team)) {
reverseLastMove();
return false;
}
return true;
}
private void movePiece(Position start, Position end) {
board[end.row()][end.column()] = pieceAt(start);
board[start.row()][start.column()] = null;
}
private void cacheMove(Move move, Position end) {
deletedPieceCache.push(pieceAt(end));
moveCache.push(move);
}
public GameStatus getGameStatus(Team team) {
for (Move move : generatePossibleMovesForTeam(team)) {
if (makeMove(move)) {
reverseLastMove();
return GameStatus.INPLAY;
}
}
// No moves can be made, game is either in checkmate or stalemate
if (isChecked(team))
return GameStatus.CHECKMATE;
else
return GameStatus.STALEMATE;
}
// Returns true if a move doesn't break the rules
public boolean isValidMove(Move move, Team team) {
if (pieceAt(move.start()) == null)
return false;
if (pieceAt(move.start()).getTeam() != team)
return false;
List<Move> possibleMoves = generatePossibleMovesForPiece(move.start());
return possibleMoves.contains(move);
}
public List<Move> generatePossibleMovesForTeam(Team team) {
List<Move> ret = new ArrayList<>();
for (Position pos : getPositionsOfPiecesForTeam(team))
ret.addAll(generatePossibleMovesForPiece(pos));
return ret;
}
// Adds piece objects to board for each team
private void addPieces(int backRow, int frontRow, Team team) {
board[backRow][0] = new Rook(team);
board[backRow][7] = new Rook(team);
board[backRow][1] = new Knight(team);
board[backRow][6] = new Knight(team);
board[backRow][2] = new Bishop(team);
board[backRow][5] = new Bishop(team);
board[backRow][3] = new Queen(team);
board[backRow][4] = new King(team);
for (int i = 0; i < 8; i++)
board[frontRow][i] = new Pawn(team);
}
private boolean isChecked(Team team) {
Position kingsPosition = getKingPosition(team);
Team otherTeam = Team.otherTeam(team);
for (Position position : getPositionsOfPiecesForTeam(otherTeam)) {
Move move = new Move(position, kingsPosition);
if (isValidMove(move, otherTeam))
return true;
}
return false;
}
// If pawn reached the end, replace with queen
private void checkForPawnReplacement(Position start, Position end) {
if (pieceAt(end) instanceof Pawn && (end.row() == 0 || end.row() == 7)) {
replacePawnWithQueen(end);
pawnToQueenConversionCache.push(start);
} else
pawnToQueenConversionCache.push(null);
}
private void replacePawnWithQueen(Position end) {
board[end.row()][end.column()] = new Queen(pieceAt(end).getTeam());
}
// Uses cache to reverse a move where a pawn has turned into a queen
private void checkForReversePawnReplacement() {
Position pos = pawnToQueenConversionCache.pop();
if (pos != null)
board[pos.row()][pos.column()] = new Pawn(pieceAt(pos).getTeam());
}
private List<Move> generatePossibleMovesForPiece(Position start) {
Piece piece = pieceAt(start);
if (piece instanceof Pawn)
updatePawnSurroundings(start);
return removeInvalidMoves(piece.generateMoveList(start));
}
// Tells a pawn object where its surrounding pieces are so it can make a move
private void updatePawnSurroundings(Position pawnPosition) {
boolean leftTake = false, rightTake = false;
boolean isPieceInFront = false, isPieceTwoInFront = false;
Pawn pawn = (Pawn) pieceAt(pawnPosition);
int directionModifier = getDirectionModifier(pawn.getTeam());
Position pos;
// True if an opposing teams piece is at top left of pawn
pos = new Position(pawnPosition.row() + directionModifier, pawnPosition.column() + 1);
if (pieceAt(pos) != null && pieceAt(pos).getTeam() != pawn.getTeam())
rightTake = true;
// True if an opposing teams piece is at top right of pawn
pos = new Position(pawnPosition.row() + directionModifier, pawnPosition.column() - 1);
if (pieceAt(pos) != null && pieceAt(pos).getTeam() != pawn.getTeam())
leftTake = true;
// True if a piece is in front of the pawn
pos = new Position(pawnPosition.row() + directionModifier, pawnPosition.column());
if (pieceAt(pos) != null)
isPieceInFront = true;
// True if no piece lies 2 spots ahead of pawn
pos = new Position(pawnPosition.row() + (directionModifier * 2), pawnPosition.column());
if (pieceAt(pos) != null)
isPieceTwoInFront = true;
pawn.setSurroundingPositions(leftTake, rightTake, isPieceInFront, isPieceTwoInFront);
}
// Returns the direction where a pawn should move given the team it's in
private int getDirectionModifier(Team team) {
if (team == Team.WHITE)
return 1;
else
return -1;
}
// Filters out any moves that don't follow the rules of the game
private List<Move> removeInvalidMoves(List<Move> moves) {
List<Move> ret = new ArrayList<>();
for (Move move : moves)
if (isClearPath(move) && isValidDestination(move))
ret.add(move);
return ret;
}
// Returns true if no other pieces lie in a pieces path when moving
private boolean isClearPath(Move move) {
List<Position> path = move.drawPath();
for (Position position : path)
if (pieceAt(position) != null)
return false;
return true;
}
private Position getKingPosition(Team team) {
for (int row = 0; row < 8; row++) {
for (int column = 0; column < 8; column++) {
Position pos = new Position(row, column);
if (pieceAt(pos) != null && (pieceAt(pos) instanceof King) && pieceAt(pos).getTeam() == team)
return pos;
}
}
throw new AssertionError("King not found");
}
// Returns List of all positions of a given teams pieces that can make a move
private List<Position> getPositionsOfPiecesForTeam(Team team) {
List<Position> ret = new ArrayList<>();
for (int i = 0; i < 8; i++)
for (int j = 0; j < 8; j++) {
Position pos = new Position(i, j);
if (pieceAt(pos) != null && pieceAt(pos).getTeam() == team)
if (generatePossibleMovesForPiece(pos).size() > 0)
ret.add(pos);
}
return ret;
}
// Returns true if the destination isn't occupied by a pieces own team
private boolean isValidDestination(Move move) {
Position start = move.start();
Position end = move.destination();
Team team = pieceAt(start).getTeam();
if (pieceAt(end) != null && pieceAt(end).getTeam() == team)
return false;
return true;
}
public Piece pieceAt(Position position) {
if (!position.isOnBoard())
return null;
return board[position.row()][position.column()];
}
@SuppressWarnings("unused")
private void printBoard() {
for (Piece[] row : board) {
System.out.println();
for (Piece piece : row)
if (piece == null)
System.out.print("-");
else
System.out.print(piece);
}
System.out.println("n");
}
public void clearCache() {
deletedPieceCache.clear();
moveCache.clear();
pawnToQueenConversionCache.clear();
}
private void buildHeuristicMapping() {
heuristicMap.put("k", 950);
heuristicMap.put("q", 100);
heuristicMap.put("r", 60);
heuristicMap.put("b", 40);
heuristicMap.put("n", 30);
heuristicMap.put("p", 10);
}
public int generateHeuristicValue(Team team) {
int value = 0;
for (Piece[] row : board)
for (Piece piece : row)
if (piece != null) {
if (team == piece.getTeam())
value += heuristicMap.get(piece.toString().toLowerCase());
else
value -= heuristicMap.get(piece.toString().toLowerCase());
}
return value;
}
}
Piece.java
package chess;
import java.io.Serializable;
import java.util.List;
abstract public class Piece implements Serializable {
private final Team team;
public Piece(Team t) {
team = t;
}
protected void addPositionToMoveList(List<Move> moves, Position start, Position pos) {
if (pos.isOnBoard())
moves.add(new Move(start, pos));
}
public Team getTeam() {
return team;
}
// Generates set of all possible positions a piece can move to
public abstract List<Move> generateMoveList(Position start);
}
View.java
package chess;
import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.GridLayout;
import java.awt.Image;
import java.awt.image.BufferedImage;
import java.io.File;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import java.util.Observable;
import javax.imageio.ImageIO;
import javax.swing.ImageIcon;
import javax.swing.JButton;
import javax.swing.JFileChooser;
import javax.swing.JFrame;
import javax.swing.JMenu;
import javax.swing.JMenuBar;
import javax.swing.JMenuItem;
import javax.swing.JOptionPane;
import javax.swing.JPanel;
import javax.swing.JTextField;
import javax.swing.filechooser.FileSystemView;
public class View extends Observable {
// Allows us to access a tile given a position on the board
private final JButton[][] tiles;
// Main frame that the GUI runs on
private final JFrame frame;
// Main panel that all tiles on the board are placed on
private final JPanel board;
// Panel that holds any buttons the player needs
private final JPanel playerOptions;
// Maps string representation of a piece to its image
private final Map<String, Image> pieceToImage;
// Displays any information on the game (i.e checks, illegal moves)
private final JTextField gameStatus;
// These components represent the filemenu dropdown menu for saving and loading
private final JMenuBar fileMenuBar;
private final JMenu fileMenu;
private final JMenuItem save;
private final JMenuItem load;
// Allows view to tell the controller any requests that come from the player
private UpdateType updateType;
public View() {
frame = new JFrame("Chess");
board = new JPanel(new GridLayout(0, 8));
fileMenuBar = new JMenuBar();
fileMenu = new JMenu("File");
save = new JMenuItem("Save");
load = new JMenuItem("Load");
setUpFileMenu();
playerOptions = new JPanel();
setupPlayerOptions();
gameStatus = new JTextField("");
gameStatus.setHorizontalAlignment(JTextField.CENTER);
tiles = new JButton[8][8];
setupBoardButtons();
addBoardBehaviour();
pieceToImage = new HashMap<>();
addPieceImagesToMap();
addComponentsToFrame();
configureFrame();
}
private void configureFrame() {
frame.setSize(1000, 1000);
frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
frame.setVisible(true);
}
private void setUpFileMenu() {
fileMenu.add(save);
fileMenu.add(load);
fileMenuBar.add(fileMenu);
addSaveBehaviour();
addLoadBehaviour();
}
// Tells program what to do when save button is pressed
private void addSaveBehaviour() {
save.addActionListener(actionEvent -> {
File file = getFileFromUser();
if (file != null) {
updateType = UpdateType.SAVE;
setChanged();
notifyObservers(file);
updateType = UpdateType.NONE;
}
});
}
// Tells program what to do when load button is pressed
private void addLoadBehaviour() {
load.addActionListener(actionEvent -> {
File file = getFileFromUser();
if (file != null) {
updateType = UpdateType.LOAD;
setChanged();
notifyObservers(file);
updateType = UpdateType.NONE;
}
});
}
public void fileIOError() {
JOptionPane.showMessageDialog(null, "Error when loading in file");
}
// Allows user to select a file from their computer's file menu
private File getFileFromUser() {
JFileChooser jfc = new JFileChooser(FileSystemView.getFileSystemView().getHomeDirectory());
if (jfc.showOpenDialog(null) == JFileChooser.APPROVE_OPTION)
return jfc.getSelectedFile();
return null;
}
public UpdateType getUpdateType() {
return updateType;
}
public void gameOverMessage(GameStatus status, Team team) {
if (status == GameStatus.STALEMATE)
JOptionPane.showMessageDialog(null, "Game has ended in a stalemate");
else
JOptionPane.showMessageDialog(null, "Checkmate, " + Team.toString(Team.otherTeam(team)) + " has won");
}
// Updates the images displayed on the board for a move
public void updateTile(Position position, String update) {
tiles[position.row()][position.column()].setIcon(new ImageIcon(pieceToImage.get(update)));
}
// Remove image from a tile
public void clearTile(Position position) {
tiles[position.row()][position.column()].setIcon(null);
}
public void invalidMoveMessage(Move move) {
gameStatus.setText("Attempted move " + move + " is invalid");
}
public void moveMessage(Move move) {
gameStatus.setText(move.toString());
}
public void checkMessage(Team team) {
gameStatus.setText(Team.toString(team) + " would be checked as the result of that move");
}
private void addComponentsToFrame() {
frame.getContentPane().add(BorderLayout.CENTER, board);
frame.getContentPane().add(BorderLayout.SOUTH, playerOptions);
frame.getContentPane().add(BorderLayout.NORTH, gameStatus);
}
private void setupPlayerOptions() {
playerOptions.add(fileMenuBar);
}
// Adds the actionlistener to every button in the board
private void addBoardBehaviour() {
for (int row = 0; row < 8; row++)
for (int column = 0; column < 8; column++)
addButtonBehaviour(row, column);
}
// Allows user to select pieces for a move
private void addButtonBehaviour(final int row, final int column) {
tiles[row][column].addActionListener(actionEvent -> {
updateType = UpdateType.MOVE;
setChanged();
notifyObservers(new Position(row, column));
updateType = UpdateType.NONE;
});
}
// Create buttons and add to panel
private void setupBoardButtons() {
for (int row = 0; row < 8; row++) {
for (int column = 0; column < 8; column++) {
JButton button = new JButton();
setBackgroundForTile(row, column, button);
tiles[row][column] = button;
board.add(button);
}
}
}
private void setBackgroundForTile(int row, int column, JButton button) {
if ((column % 2 == 0 && row % 2 == 0) || (column % 2 == 1 && row % 2 == 1))
button.setBackground(Color.WHITE);
else
button.setBackground(Color.BLACK);
}
private void addPieceImagesToMap() {
Image[][] pieceImages = new Image[2][6];
readPieceImages(pieceImages);
pieceToImage.put("q", pieceImages[0][0]);
pieceToImage.put("k", pieceImages[0][1]);
pieceToImage.put("r", pieceImages[0][2]);
pieceToImage.put("n", pieceImages[0][3]);
pieceToImage.put("b", pieceImages[0][4]);
pieceToImage.put("p", pieceImages[0][5]);
pieceToImage.put("Q", pieceImages[1][0]);
pieceToImage.put("K", pieceImages[1][1]);
pieceToImage.put("R", pieceImages[1][2]);
pieceToImage.put("N", pieceImages[1][3]);
pieceToImage.put("B", pieceImages[1][4]);
pieceToImage.put("P", pieceImages[1][5]);
}
// Get piece images from file
private void readPieceImages(Image[][] pieceImages) {
int imageSize = 64;
try {
BufferedImage imageBuffer = ImageIO.read(new File("piece_images.png"));
for (int i = 0; i < 2; i++)
for (int j = 0; j < 6; j++)
pieceImages[i][j] = imageBuffer.getSubimage(j * imageSize, i * imageSize, imageSize, imageSize);
} catch (IOException io) {
System.out.println("Error with handling images");
io.printStackTrace();
}
}
}
Move.java
package chess;
import java.io.Serializable;
import java.util.ArrayList;
import java.util.List;
public class Move implements Serializable {
private final Position start;
private final Position end;
public Move(Position s, Position e) {
start = s;
end = e;
}
// Example: drawPath((1, 1), (4, 4)) returns [(2, 2), (3, 3)]
public List<Position> drawPath() {
List<Position> path = new ArrayList<>();
MovementType movementType = getMovementType();
// Not necessary for horse, return empty list
if (movementType == MovementType.HORSE)
return path;
int rowIncrement = getIncrementValues(movementType)[0] * getRowDirection();
int columnIncrement = getIncrementValues(movementType)[1] * getColumnDirection();
int rowOffset = rowIncrement;
int columnOffset = columnIncrement;
// Draw path until we reach end position
while (start.row() + rowOffset != end.row() || start.column() + columnOffset != end.column()) {
path.add(new Position(start.row() + rowOffset, start.column() + columnOffset));
rowOffset += rowIncrement;
columnOffset += columnIncrement;
}
return path;
}
// Returns 1 if piece moved down, -1 if moved up, 0 if piece didn't change row
private int getRowDirection() {
if (end.row() - start.row() > 0)
return 1;
else if (end.row() - start.row() < 0)
return -1;
else
return 0;
}
// Returns 1 if piece moved right, -1 if moved left, 0 if piece didn't change column
private int getColumnDirection() {
if (end.column() - start.column() > 0)
return 1;
else if (end.column() - start.column() < 0)
return -1;
else
return 0;
}
public Position destination() {
return end;
}
public Position start() {
return start;
}
private MovementType getMovementType() {
if (Math.abs(start.row() - end.row()) == Math.abs(start.column() - end.column()))
return MovementType.DIAGONAL;
if (start.row() == end.row())
return MovementType.HORIZONTAL;
if (start.column() == end.column())
return MovementType.VERTICAL;
return MovementType.HORSE;
}
// Returns the change in co-ordinates that came from a movement
private int[] getIncrementValues(MovementType movement) {
int rowIncrement = 0;
int columnIncrement = 0;
switch (movement) {
case DIAGONAL:
rowIncrement = 1;
columnIncrement = 1;
break;
case HORIZONTAL:
columnIncrement = 1;
break;
case VERTICAL:
rowIncrement = 1;
break;
default:
throw new AssertionError("Enum doesn't seem to match with any supported types");
}
return new int[] { rowIncrement, columnIncrement };
}
@Override
public boolean equals(Object obj) {
if (!(obj instanceof Move))
return false;
Move move = (Move) obj;
return start.equals(move.start) && end.equals(move.end);
}
@Override
public int hashCode() {
return start.hashCode() * 27832 + end.hashCode();
}
@Override
public String toString() {
return start.toString() + " to " + end.toString();
}
enum MovementType {
DIAGONAL, HORIZONTAL, VERTICAL, HORSE
}
}
Minimax.java
package chess;
/*
* Uses the minimax algorithm with alpha beta pruning to make moves
*/
public class MinimaxAI {
private final int maxDepth;
private final Team team;
public MinimaxAI(int m, Team t) {
maxDepth = m;
team = t;
}
// Return move that minimax algorithm wants to make by
// running minimax on all possible moves
public Move pickMove(Board board) {
int max = Integer.MIN_VALUE;
int current;
Move optimalMove = null;
for (Move move : board.generatePossibleMovesForTeam(team)) {
if (board.makeMove(move)) {
current = min(board, 1, Integer.MIN_VALUE, Integer.MAX_VALUE);
if (current >= max) {
optimalMove = move;
max = current;
}
board.reverseLastMove();
}
}
board.clearCache();
return optimalMove;
}
// For all moves the opposing team could make, return least optimal for the AI
private int min(Board board, int depth, int alpha, int beta) {
if (depth == maxDepth)
return board.generateHeuristicValue(team);
for (Move move : board.generatePossibleMovesForTeam(Team.otherTeam(team))) {
if (board.makeMove(move)) {
beta = Math.min(max(board, depth + 1, alpha, beta), beta);
board.reverseLastMove();
}
if (alpha >= beta)
break;
}
return beta;
}
// For all moves the AI could make, return most optimal
private int max(Board board, int depth, int alpha, int beta) {
if (depth == maxDepth)
return board.generateHeuristicValue(team);
for (Move move : board.generatePossibleMovesForTeam(team)) {
if (board.makeMove(move)) {
alpha = Math.max(min(board, depth + 1, alpha, beta), alpha);
board.reverseLastMove();
}
if (alpha >= beta)
break;
}
return alpha;
}
}
On first glance, I wouldn't call things just View
, Controller
, and Observable
, even if they are indirectly qualified by their package. It's a bit surprising to see that in Java, the names suggest reusability and independence of Chess. It probably should be ChessBoardView
and ChessBoardController
. Having Observable
as an interface is a bit unusual in Java MVC, it's more common to have the counter-part Observer and name it Listener
, in this case ChessBoardListener
.
The fileStream
is overly qualified as FileInputStream
, InputStream
would suffice.
If readPieceImages()
fails, the program will continue with broken data and throw a NullPointerException
when calling updateTile()
at new ImageIcon(pieceToImage.get(update))
: pieceToImage.get(update)
will return null
, and new ImageIcon((java.awt.Image) null)
throws a NullPointerException
in the constructor.
Method fileIOError()
could use JOptionPane.ERROR_MESSAGE
to signal to the user that the message is about an error.
Nice: Mostly immutable fields. You could go one step further and use unmodifiable collections as well. Instead of, in the constructor,
pieceToImage = new HashMap<>();
addPieceImagesToMap();
you could have
private final Map<String, Image> pieceToImage = loadPieceImages();
and loadPieceImages()
would create a new HashMap<>()
plus return Collections.unmodifiableMap(pieceToImage)
.
Besides, pieceToImage
should probably cache the ImageIcon
, not the Image
. That would save repetitive constructor calls of new ImageIcon
in updateTile()
.
For equals()
and hashCode()
you might want to use Lombok, it saves a lot of boilerplate.
The switch (movement)
could be avoided altogether by giving the enum MovementType
fields rowIncrement
and columnIncrement
. This could also replace the int[]
return type, which is not necessarily intuitive (one has to remember whether row or column comes first). (In other words. the implementation of the enum is not OO.)
The MovementType
enum is also partially confusing, because in some contexts all four enum constants, including HORSE
are allowed, and in some contexts, only 3 excluding HORSE
are allowed.
Some of the methods and classes appear a bit long at first glance, and some responsibility misplaced. Loading and saving a board should probably not be in the same Controller
as other UI functions.
Update: The current class Board
conflates different responsibilities (it would change for more than one reason), and thus should be split:
Board
itself, merely representing the chess board with the positions of the pieces. It shouldn't even know that the pieces are chess. Hypothetically, this should be reusable for implementing Draughts instead of Chess.Rules
which merely connects the Board and the AI to what's allowed.ChessRules
which implements Rules
for the actual rules of Chess.ChessAI
or something like that for all parts currently in Board
which only serve the purpose of feeding the MinimaxAI
.MinimaxAI
is great, the behavior already doesn't know anything about Chess. The dependency could be decoupled so that even structurally it doesn't know about Chess. (Right now, Board
is still specific to Chess.)Update 2
Team
(not shown in the question), method otherTeam()
should not be a static
utility method but an instance method of the enum.Team
(not shown in the question), method toString()
also should not be a static
utility method but an instance method of that enum.Overall, I really enjoyed looking at the code. That's it for now, I might have a more detailed look later.
Answered by Christian Hujer on December 31, 2021
Get help from others!
Recent Questions
Recent Answers
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP