trjhtr

I was trying to design a chess game in Java. I have just started learning design patterns and am not sure if my approach here is right. Could you please suggest ways to better this design ? Below is how the code looks like :

public enum SquareState 
 OCCUPIED, EMPTY

public enum Color 
 BLACK, WHITE 

public class ChessBoard 
 public static final int MAX_ROWS = 8;
 public static final int MAX_COLS = 8;
 public static final int NUM_SQUARES = 64;
 private Square squares;
 private List<Piece> whitePieces;
 private List<Piece> blackPieces;
 private Map<Square, Piece> occupiedSquares;

 public Piece pieceAt(Square square) 
 return occupiedSquares.get(square);
 

//Square class
public class Square 
 private int row, col;
 private Color color;
 private SquareState state;

//Piece class -not an abstract class(do I need one, I already have PieceType)
public class Piece 
 private PieceType type;
 private Color color;
 private Square position;

 public void move(ChessBoard chessBoard, Square dest) 
 //place the piece from its position to dest on the board
 

public class Move 
 private MoveType moveType; //undo operation will require move type
 private Square source;
 private Square destination;
 private Piece sourcePiece; //at source 
 private Piece killedPiece; //at dest
 private Move previous, next;

public class Player 
 private boolean isWhite;
 private ChessBoard chessBoard;
 private MovementGenerator movementGenerator;
 private List<Piece> alivePieces;

 public void makeMove() 
 Piece chosenPiece = alivePieces.get(0);
 List<Square> squares = movementGenerator.getPossibleMoves(chessBoard, chosenPiece);
 chosenPiece.move(chessBoard, squares.get(0));
 

public class MovementGenerator 
 private MovementStrategy movementStrategy;
 private MovementStrategyResolver movementStrategyResolver;

 public MovementGenerator(MovementStrategy movementStrategy) 
 this.movementStrategy = movementStrategy;
 

 public List<Square> getPossibleMoves(ChessBoard chessBoard, Piece piece) 
 return movementStrategyResolver.resolveStrategy(piece).getPossibleMoves(chessBoard, piece);
 

public interface MovementStrategy 
 public List<Square> getPossibleMoves(ChessBoard chessBoard, Piece piece);

public class BishopMovementStrategy implements MovementStrategy
 @Override
 public List<Square> getPossibleMoves(ChessBoard chessBoard, Piece piece) 
 // TODO Auto-generated method stub
 return null;
 

public class KnightMovementStrategy implements MovementStrategy 

 @Override
 public List<Square> getPossibleMoves(ChessBoard chessBoard, Piece piece) 
 // TODO Auto-generated method stub
 return null;
 

//A class to validate if move made by the player is a legal one

public class MovementValidator 

 private MovementGenerator movementGenerator;

 public boolean isValidMove(ChessBoard board, Move move) 

 private boolean outsideBoard(Square square) 

Also, isValidMove() method of MovementValidator class should get called as soon as a player makes a move and should flag an error if the move made is illegal. How can I enforce this in the above code(given that validating the legality of a move shouldn't be the responsibility of the Player class)?

Alternatively, should I make Piece as a base class and extend it for King, Queen, Bishop, Rook, Pawn and let all these classes have reference to concrete MovementStartegy objects? Would that be a better design?

Your Piece class should be abstract and you should extend all pieces to it. You can get rid of your conventional MovementValidator and MovementGenerator classes this way. Shorter code is better as long as it's also done conventionally.

public abstract class Piece 
 private PieceType type;
 private Color color;
 private Square position;

 /**
 * @param move to be checked
 * @return if move is valid
 */
 public abstract boolean isValidMove(Move move);

 /**
 * @return List of moves
 */
 public abstract List<Move> getValidMoves();

 public void move(ChessBoard chessBoard, Square dest) 
 //place the piece from its position to dest on the board
 Move move;

 /*Instantiate move*/

 if (validMove(move))
 //Make the move
 
 

As a silly suggestion, since you are using a Color object for certain checks see if you can implement more colors than just black and white. No point in programming if you can't have a bit of fun.