Skip to main content
Added formatting for code
Source Link
Phrancis
  • 20.5k
  • 6
  • 70
  • 155

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);

Game.move(currentSpot, NewSpot);

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

  • Check Piece.isValidMove(currentSpot, newSpot); Piece.isValidMove(currentSpot, newSpot); - probably need castling logic here since king moves more than 1 space and rook jumps the king)
  • Check Player.isChecked()Player.isChecked() (which is just sugar for Player.Pieces["King"].CanBeCaptured()Player.Pieces["King"].CanBeCaptured() - more fun logic here!)
  • Check if newSpotnewSpot contains a piece and if so, newSpot.Piece.Remove()newSpot.Piece.Remove();
  • Build some logic to call Piece.CheckEnPassant()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.CheckPromote() (Piece is pawn, move ends on opposing player's back rank)
  • Check if Game.isOver()Game.isOver(), which checks Game.isStaleMate()Game.isStaleMate() and Game.isCheckMate()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()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.

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.

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.

added 141 characters in body
Source Link
Kyle Hale
  • 840
  • 6
  • 13

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.

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.

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.

added 141 characters in body
Source Link
Kyle Hale
  • 840
  • 6
  • 13

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.

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.
  • 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.

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.

Source Link
Kyle Hale
  • 840
  • 6
  • 13
Loading