Skip to content

Step 3 - Blackjack (Dealer)#835

Open
krrong wants to merge 18 commits into
next-step:krrongfrom
krrong:step3
Open

Step 3 - Blackjack (Dealer)#835
krrong wants to merge 18 commits into
next-step:krrongfrom
krrong:step3

Conversation

@krrong

@krrong krrong commented May 6, 2025

Copy link
Copy Markdown

Hello Brie, I hope you are doing well.
Came back with third step of blackjack and feel free to add some comments!
Actually, I didn't have much time, so I couldn't give it a lot of thought, and it might be a bit disorganized.
I'm looking forward your feedback.

Thanks and have a nice day 😆

@byjo byjo left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Seokjin, Great work on this step as well!

I've added a few comments for improvement.
Try refactoring your code using those suggestions and the two programming requirements below:

  • Implement all features using Test-Driven Development (TDD), with unit tests for all logic except UI
  • Maintain indentation depth of 1 or less—do not nest more than two levels.

Hope you wrap up this step smoothly and also enjoy your holiday!

import Hand
import card.PlayingCard

interface State {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've finally applied the state pattern to this mission! 👍👍
Great to see you reach your goal!

Comment thread src/main/kotlin/Casino.kt Outdated
if (!response || player.hand.isBust()) return
player.drawCard(deck.drawCard(1).first())
outputView.printPlayerCards(player)
when (participant) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to eliminate this conditional by using polymorphism?

Comment thread src/main/kotlin/Casino.kt Outdated

val winningResult = WinningResult()
participants.forEach {
when (it) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you think this conditional could be refactored?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's also possible by using polymorphism like above!

Comment thread src/main/kotlin/participant/Dealer.kt Outdated

class Dealer(name: String = "Dealer", override var state: State) : Participant(name) {
override fun showCardFirst(): List<PlayingCard> {
return listOf(state.hand.cards.first())

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line reaches through multiple objects, which slightly violates the Law of Demeter.
You might consider exposing a function to encapsulate this access and make the intention clearer!

This is similar to what I mentioned in a previous comment: #833 (comment)

Comment thread src/main/kotlin/WinningResult.kt Outdated
import state.Bust
import state.Stay

class WinningResult {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job extracting the game result logic into WinningResult! 👍

However, I noticed there's no test coverage for this logic yet.
It would be even better to add tests to make sure this logic is well covered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants