Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ChessPieceTests to Explicitly Test equals and hashCode #11

Open
frozenfrank opened this issue Jan 23, 2025 · 4 comments · May be fixed by #16
Open

Add ChessPieceTests to Explicitly Test equals and hashCode #11

frozenfrank opened this issue Jan 23, 2025 · 4 comments · May be fixed by #16
Assignees
Labels
enhancement New feature or request

Comments

@frozenfrank
Copy link
Member

No description provided.

@frozenfrank frozenfrank added the enhancement New feature or request label Jan 23, 2025
@frozenfrank frozenfrank self-assigned this Jan 23, 2025
@19mdavenport
Copy link
Contributor

Were you thinking of making a new file for this? If so, what do you think about moving the "Piece moves on all pieces" test into that file? It feels a little awkward in ChessBoardTests but i didn't have a better place to put it at the time

@frozenfrank
Copy link
Member Author

Were you thinking of making a new file for this? If so, what do you think about moving the "Piece moves on all pieces" test into that file? It feels a little awkward in ChessBoardTests but i didn't have a better place to put it at the time

@19mdavenport That's a great idea! I'll do that too.

@frozenfrank frozenfrank changed the title Add ChessPieceTests to Explicitly test equals and hashCode Add ChessPieceTests to Explicitly Test equals and hashCode Jan 23, 2025
@frozenfrank
Copy link
Member Author

Were you thinking of making a new file for this? If so, what do you think about moving the "Piece moves on all pieces" test into that file? It feels a little awkward in ChessBoardTests but i didn't have a better place to put it at the time

@19mdavenport After looking at it more carefully, there is a technical snag to completing this refactor: the test currently relies on a working ChessBoard.resetBoard() method.

@Test
@DisplayName("Piece Move on All Pieces")
public void pieceMoveAllPieces() {
var board = new ChessBoard();
board.resetBoard();
for(int i = 1; i <= 8; i++) {
for(int j = 1; j <= 8; j++) {
ChessPosition position = new ChessPosition(i, j);
ChessPiece piece = board.getPiece(position);
if(piece != null) {
Assertions.assertDoesNotThrow(() -> piece.pieceMoves(board, position));
}
}
}
}

We could leave the method there, I don't think that would be a problem.

What do you think about calling pieceMoves() a board with every single piece placed in every single position? 886 = 384 total invocations... I think that's reasonable.

@frozenfrank
Copy link
Member Author

@19mdavenport Update: We doubled the total tests and completed this request in commit 41db8c8.

@frozenfrank frozenfrank linked a pull request Jan 28, 2025 that will close this issue
@frozenfrank frozenfrank linked a pull request Feb 4, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants