Skip to content

Conversation

@krrong
Copy link

@krrong krrong commented May 6, 2025

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 😆

Copy link

@byjo byjo left a comment

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

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!

if (!response || player.hand.isBust()) return
player.drawCard(deck.drawCard(1).first())
outputView.printPlayerCards(player)
when (participant) {
Copy link

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?


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

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
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!


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

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)

import state.Bust
import state.Stay

class WinningResult {
Copy link

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