refactor: outsource deck related actions to service #236

Merged

ptran merged 1 commit from refactor-blackjack into main

2025-05-28 10:38:15 +00:00

ptran commented

2025-05-28 10:32:55 +00:00

Member

No description provided.

ptran added 1 commit 2025-05-28 10:32:55 +00:00

refactor: outsource deck related actions to service

CI / Get Changed Files (pull_request) Successful in 10s

Details

CI / eslint (pull_request) Has been skipped

Details

CI / oxlint (pull_request) Has been skipped

Details

CI / prettier (pull_request) Has been skipped

Details

CI / test-build (pull_request) Has been skipped

Details

CI / Docker frontend validation (pull_request) Has been skipped

Details

CI / Checkstyle Main (pull_request) Successful in 57s

Details

CI / Docker backend validation (pull_request) Successful in 49s

Details

Setup Gitea Tea CLI / setup-and-login (pull_request) Successful in 1m32s

Details

2a51dce565

Claude commented

2025-05-28 10:34:38 +00:00

Collaborator

This PR has several critical issues that prevent acceptance:

Compilation Errors:

DeckService uses CardType enum without importing it
Code won't compile as submitted

Code Quality Issues:

Unused validation imports (@NotNull, @Positive) in BlackJackGameEntity
Inconsistent separation of concerns - some card dealing methods moved to DeckService while dealCardsToDealerUntilMinimumScore stays in BlackJackService
Public methods in DeckService should be package-private for better encapsulation
Trailing whitespace and poor formatting

Breaking Changes Without Justification:

Removed AuthProvider.LOCAL assignment from UserEntity constructor - this could break registration logic
Added @Version to UserEntity without any optimistic locking strategy

Missing Tests:

No tests for the new DeckService
No verification that existing functionality still works

Documentation:

No JavaDoc for new service
No explanation of why this refactoring was necessary

Fix the compilation errors, add proper tests, justify the breaking changes, and clean up the code formatting before resubmitting.

Status: REJECTED

This PR has several critical issues that prevent acceptance: **Compilation Errors:** - DeckService uses CardType enum without importing it - Code won't compile as submitted **Code Quality Issues:** - Unused validation imports (@NotNull, @Positive) in BlackJackGameEntity - Inconsistent separation of concerns - some card dealing methods moved to DeckService while dealCardsToDealerUntilMinimumScore stays in BlackJackService - Public methods in DeckService should be package-private for better encapsulation - Trailing whitespace and poor formatting **Breaking Changes Without Justification:** - Removed AuthProvider.LOCAL assignment from UserEntity constructor - this could break registration logic - Added @Version to UserEntity without any optimistic locking strategy **Missing Tests:** - No tests for the new DeckService - No verification that existing functionality still works **Documentation:** - No JavaDoc for new service - No explanation of why this refactoring was necessary Fix the compilation errors, add proper tests, justify the breaking changes, and clean up the code formatting before resubmitting. **Status: REJECTED**