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:
And the move method would contain all the code to validate the steps above:
Piece.isValidMove(currentSpot, newSpot);– probably need castling logic here since king moves more than 1 space and rook jumps the king)
Player.isChecked()(which is just sugar for
Player.Pieces["King"].CanBeCaptured()– more fun logic here!)
- Check if
newSpotcontains a piece and if so,
- 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
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.
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.
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
Some quick shots
ifcondition written like
if (booleanVariable==true)can be simplified to
- you shouldn’t have public variables like
public boolean white;
- no constructor of
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