Design a chess game using object-oriented principles

Question

I would like to know if my approach is correct and how could it could be improved? Also, is there a way to get rid of the relation between the Piece and the Board? At the moment, I am storing the position of the piece both in the piece and on the board. Is there some way to change that?

I have considered a Game to contain an instance of the Board and the two Players (one black, one white). The pieces contain a connection to the Board, because in order to determine if they are valid, we need to know the relationship to other pieces.

Could I use design patterns for this? Should I use interfaces instead of the super class?

Game.java

public class Game { private Board board = new Board(); private Player white; private Player black; public Game() { super(); } public void setColorWhite(Player player) { this.white = player; } public void setColorBlack(Player player) { this.black = player; } public Board getBoard() { return board; } public void setBoard(Board board) { this.board = board; } public Player getWhite() { return white; } public void setWhite(Player white) { this.white = white; } public Player getBlack() { return black; } public void setBlack(Player black) { this.black = black; } public boolean initializeBoardGivenPlayers() { if(this.black == null || this.white == null) return false; this.board = new Board(); for(int i=0; i<black.getPieces().size(); i++){ board.getSpot(black.getPieces().get(i).getX(), black.getPieces().get(i).getY()).occupySpot(black.getPieces().get(i)); } return true; } } 

Player.java

public class Player { public final int PAWNS = 8; public final int BISHOPS = 2; public final int ROOKS = 2; public boolean white; private List<Piece> pieces = new ArrayList<>(); public Player(boolean white) { super(); this.white = white; } public List<Piece> getPieces() { return pieces; } public void initializePieces(){ if(this.white == true){ for(int i=0; i<PAWNS; i++){ // draw pawns pieces.add(new Pawn(true,i,2)); } pieces.add(new Rook(true, 0, 0)); pieces.add(new Rook(true, 7, 0)); pieces.add(new Bishop(true, 2, 0)); pieces.add(new Bishop(true, 5, 0)); pieces.add(new Knight(true, 1, 0)); pieces.add(new Knight(true, 6, 0)); pieces.add(new Queen(true, 3, 0)); pieces.add(new King(true, 4, 0)); } else{ for(int i=0; i<PAWNS; i++){ // draw pawns pieces.add(new Pawn(true,i,6)); } pieces.add(new Rook(true, 0, 7)); pieces.add(new Rook(true, 7, 7)); pieces.add(new Bishop(true, 2, 7)); pieces.add(new Bishop(true, 5, 7)); pieces.add(new Knight(true, 1, 7)); pieces.add(new Knight(true, 6, 7)); pieces.add(new Queen(true, 3, 7)); pieces.add(new King(true, 4, 7)); } } } 

Board.java

public class Board { private Spot[][] spots = new Spot[8][8]; public Board() { super(); for(int i=0; i<spots.length; i++){ for(int j=0; j<spots.length; j++){ this.spots[i][j] = new Spot(i, j); } } } public Spot getSpot(int x, int y) { return spots[x][y]; } } 

Spot.java

public class Spot { int x; int y; Piece piece; public Spot(int x, int y) { super(); this.x = x; this.y = y; piece = null; } public void occupySpot(Piece piece){ //if piece already here, delete it, i. e. set it dead if(this.piece != null) this.piece.setAvailable(false); //place piece here this.piece = piece; } public boolean isOccupied() { if(piece != null) return true; return false; } public Piece releaseSpot() { Piece releasedPiece = this.piece; this.piece = null; return releasedPiece; } } 

Piece.java

public class Piece { private boolean available; private int x; private int y; public Piece(boolean available, int x, int y) { super(); this.available = available; this.x = x; this.y = y; } public boolean isAvailable() { return available; } public void setAvailable(boolean available) { this.available = available; } public int getX() { return x; } public void setX(int x) { this.x = x; } public int getY() { return y; } public void setY(int y) { this.y = y; } public boolean isValid(Board board, int fromX, int fromY, int toX, int toY){ if(toX == fromX && toY == fromY) return false; //cannot move nothing if(toX < 0 || toX > 7 || fromX < 0 || fromX > 7 || toY < 0 || toY > 7 || fromY <0 || fromY > 7) return false; return true; } } 

King.java

public class King extends Piece{ public King(boolean available, int x, int y) { super(available, x, y); // TODO Auto-generated constructor stub } @Override public boolean isValid(Board board, int fromX, int fromY, int toX, int toY) { if(super.isValid(board, fromX, fromY, toX, toY) == false) return false; if(Math.sqrt(Math.pow(Math.abs((toX - fromX)),2)) + Math.pow(Math.abs((toY - fromY)), 2) != Math.sqrt(2)){ return false; } return false; } } 

Knight.java

public class Knight extends Piece{ public Knight(boolean available, int x, int y) { super(available, x, y); } @Override public boolean isValid(Board board, int fromX, int fromY, int toX, int toY) { if(super.isValid(board, fromX, fromY, toX, toY) == false) return false; if(toX != fromX - 1 && toX != fromX + 1 && toX != fromX + 2 && toX != fromX - 2) return false; if(toY != fromY - 2 && toY != fromY + 2 && toY != fromY - 1 && toY != fromY + 1) return false; return true; } } 

Bishop.java

public class Bishop extends Piece{ public Bishop(boolean available, int x, int y) { super(available, x, y); // TODO Auto-generated constructor stub } @Override public boolean isValid(Board board, int fromX, int fromY, int toX, int toY) { if(super.isValid(board, fromX, fromY, toX, toY) == false) return false; if(toX - fromX == toY - fromY) return true; return false; } } 

Rook.java

public class Rook extends Piece{ public Rook(boolean available, int x, int y) { super(available, x, y); // TODO Auto-generated constructor stub } @Override public boolean isValid(Board board, int fromX, int fromY, int toX, int toY) { if(super.isValid(board, fromX, fromY, toX, toY) == false) return false; if(toX == fromX) return true; if(toY == fromY) return true; return false; } } 

Queen.java

public class Queen extends Piece{ public Queen(boolean available, int x, int y) { super(available, x, y); } @Override public boolean isValid(Board board, int fromX, int fromY, int toX, int toY) { if(super.isValid(board, fromX, fromY, toX, toY) == false) return false; //diagonal if(toX - fromX == toY - fromY) return true; if(toX == fromX) return true; if(toY == fromY) return true; return false; } } 

Answer 1

Without offering a deep code review (as I don't have a lot of specific Java knowledge), let's look at what a full "move" entails in chess:

• Player chooses piece to move.
• Piece makes legal move according to its own move rules.
• In addition to purely move-based rules, there's also capture logic, so a bishop cannot move from a1-h8 if there's a piece sitting on c3.
• If the player was previous under check and the move does not remove the check, it must be undone.
• If the move exposes check, it must be undone / disallowed.
• If player captures a piece, remove the piece (including en passant!)
• If the piece is a pawn reaching the back rank, promote it.
• If the move is a castling, set the new position of the rook accordingly. But a king and rook can only castle if they haven't moved, so you need to keep track of that. And if the king moves through a check to castle, that's disallowed, too.
• If the move results in a stalemate or checkmate, the game is over.

There may be more even (?). This is a complicated step, more than just counting and subsequently occupying spaces.

So my general intuition would be to just call:

Game.move(currentSpot, NewSpot); 

And the move method would contain all the code to validate the steps above:

• Check Piece.isValidMove(currentSpot, newSpot); - probably need castling logic here since king moves more than 1 space and rook jumps the king)
• Check Player.isChecked() (which is just sugar for Player.Pieces["King"].CanBeCaptured() - more fun logic here!)
• Check if newSpot contains a piece and if so, newSpot.Piece.Remove();
• Build some logic to call Piece.CheckEnPassant() (Piece is pawn, first move, 2 steps, past an enemy pawn who moved into capturing position on previous move - have fun with that!)
• Piece.CheckPromote() (Piece is pawn, move ends on opposing player's back rank)
• Check if Game.isOver(), which checks Game.isStaleMate() and Game.isCheckMate().

Your Board class is highly anemic, you're only using it in your code as a proxy object for the array of spots. You might as well just create Board as an array of Spots in Game. In either case, you can already remove it from all your piece logic since all your logic is entirely predicated on the Xs and Ys you're passing in.

UPDATE

I would remove all your position properties from the piece. You're only using it as a proxy to figure out what spot the piece occupies during initializiation. Instead, remove Player.initializePieces() and just initialize the Board with the pieces in the right spot (Board.Spot.Piece = King, etc.) and then let players choose a color.

Answer 2

My comments are with respect to design of the game. I see responsibilities of entities are mixed up in many places

• Player should not initialize pieces. We can move the responsibility to board. Board and pieces doesn't need to know about players.
• Pieces should not handle move. Pieces can provide list of possible moves to reach the destination path but board should choose a valid path.
• Board should have check for "Check Mate" condition.
• Game should track the players move history and piece color selection.
• Player class should have only player details.

A rough class diagram is attached below

Answer 3

Some quick shots

• a if condition written like if (booleanVariable==true) can be simplified to if (booleanVariable)
• you shouldn't have public variables like public boolean white;
• no constructor of Game, Board, Player and Piece should call super() because they are obviously not inheriting / extending any class.

Some design quickshots

• a chessgame needs a Board, 2 Players and 32 pieces.
• the pieces are part of the Board
• the Player moves the piece by rules
• the rules are bound to the type of piece and the pieces position on the board
• these rules needs to be evaluated by some object, either the Game or a RuleEvaluator.

Answer 4

Instead of using a boolean to flag if something is something (and thus implying that it isn't the other thing), consider an enum; in this case that doesn't buy you anything in terms of flexibility (since it's not as if there will ever be, say, red or purple or whatnot) but it will make your code much more clear.

On the game, instead of setting black and white separately, you should have a single setPlayers(Player black, Player white), or better yet, have the board be a factory that provides the black and white Players itself - if there's even a reason to have a Player object in the first place.

Answer 5

Couple of additions to everyone else:

• both Player and Board need to know where pieces are, both theirs and the opponents. Can't think offhand the best way to decompose this, but consider how they'll talk to each other with minimal duplication. I guess it makes more sense to put it under Board, then have Player store a reference to Board and either inherit or composite its methods:

• *.isValid() only check from- and to-coords, they currently don't check if there are intervening friendly or enemy pieces, or indeed if the to-spot is occupied. I think you should do that in one function at the Board level, it will get very ugly if you do that for each piece. You might need a helper function (iterator?) for each piece to generate interveningSpots(fromX,fromY,toX,toY,pieceType) so Board.validMove() can test if they're occupied.

• the name Piece.isValid() is confusing, it could mean either validMove() or validPosition(). Personally I'd rename it validMove(). (We'll also need a validPosition() if you ever implement promotion, but again, that would be implemented at Board level not Piece or Player)

• King.isValid() seems to currently always return false - bug?

• Queen/Bishop/Rook.isValid() currently allow null zero moves (toX==fromX && toY==fromY). This might sound like nitpicking, but a) it would wrongly allow you/AI player to evade mate by "doing nothing" b) it is likely to mess up recursion in any AI you or someone might want to add.

• a tip for more performant and more compact code for King.isValid(): you don't need to take the sqrt; just directly test that dist2 = (dx^2 + dy^2) is either 1 or 2. And you don't need abs(dx) since the square of a negative number is positive. So:

   @Override public boolean isValid(...) { if(!super.isValid(...)) { return false; int dist2 = Math.pow((toX - fromX), 2) + Math.pow((toY - fromY), 2); return (dist2 == 1 || dist2 == 2); } 

Answer 6

Here are a few different ways you can simplify your logic:

• As already mentioned, instead of writing if (condition == true), write if (condition).

• Likewise, instead of if (condition == false), write if (!condition).

• If you find yourself tempted to write:

   if (condition) return true; return false; 

...instead write return condition;.

• Likewise, instead of:

   if (condition) return false; return true; 

...write return (!condition);.

• If you're tempted to write:

   if (condition1) return true; if (condition2) return true; return false; 

...consider instead return condition1 || condition2;.

In general, simplify logic if it increases clarity.