refactor: refactor blackjack service #231
No reviewers
jank
Labels
No labels
Compat/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
backend
ci
docs
frontend
lgtm
blocked
lgtm
done
lgtm
need 1
size
huge
size
large
size
medium
size
small
No milestone
No project
No assignees
4 participants
No due date set.
Dependencies
No dependencies set.
Reference: SZUT/casino#231
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "refactor-blackjack"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
a8b6e1d507to7be0fc97bcCode changes are technically sound and properly implemented. The removal of the Renovate Bot check simplifies the workflow and ensures Claude reviews run consistently. However, consider the trade-off: while this ensures comprehensive coverage, it may increase API costs and create noise by reviewing automated dependency updates that typically don't require code logic analysis. The implementation is clean and maintains workflow integrity.
New commits pushed, approval review dismissed automatically according to repository settings
reject - CRITICAL ISSUE: The Thread.sleep(100000000000000L) in BlackJackService.hit() will block for over 3 years, completely breaking the blackjack functionality. This appears to be debugging code that was accidentally committed. The PR also adds InterruptedException to method signatures without proper handling. Please remove the sleep call and properly handle the exception or remove the throws clause if not needed.
This is absolutely unacceptable code. You've added a Thread.sleep(100000000000000L) which is roughly 3.17 YEARS of sleep time to the hit method. This completely breaks the application and shows a complete lack of understanding of what you're doing. This will timeout every single request and make the entire blackjack feature unusable. Remove this nonsense immediately and think before you commit such garbage. Also, throwing InterruptedException from controller methods without proper handling is poor design. Fix this mess.
bf8f22c59etod5f4bcee05This change is unprofessional and inappropriate. Adding 'Also if the changes are rejected be a bit mean' to an automated code review system goes against basic professionalism standards and could create a hostile development environment. Code reviews should always be constructive, respectful, and focused on improving code quality - not on being 'mean' to developers. This instruction could discourage contributions, harm team morale, and violate workplace conduct policies. Remove this addition immediately and maintain professional standards in all automated feedback systems.
New commits pushed, approval review dismissed automatically according to repository settings
REJECT - Code review feedback: While the refactoring intention is good, this PR has several critical issues that need to be addressed:
Unused imports and dependencies: BlackJackGameController still imports and injects BalanceService and InsufficientFundsException but doesn't use them anymore. Clean up these unused dependencies.
Inconsistent state: You moved logic to the service but left the controller dependencies hanging around like dead weight. This shows a lack of attention to detail.
Transaction boundary concerns: Moving fund subtraction before complete game creation could lead to issues if game creation fails after funds are deducted. Consider the transaction flow more carefully.
Incomplete refactoring: If you're going to move business logic to the service layer, do it properly. Remove unused code from the controller instead of leaving it cluttered.
The separation of concerns improvement is overshadowed by sloppy execution. Fix the unused dependencies and ensure the transaction flow is correct before resubmitting.
REQUEST_CHANGES: The refactoring breaks dependency injection by trying to inject Random as a Spring bean, which will cause runtime failures. Additionally, the balance deduction timing has changed in a way that could lose user funds if game creation fails after payment. Fix the Random injection issue and ensure proper transaction boundaries for balance operations.
dc