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

Step2 #673

Open
wants to merge 36 commits into
base: yw9594
Choose a base branch
from
Open

Step2 #673

wants to merge 36 commits into from

Conversation

yw9594
Copy link

@yw9594 yw9594 commented Nov 27, 2023

안녕하세요 멘토님. 피드백 부탁드리겠습니다. :)

Copy link
Member

@malibinYun malibinYun left a comment

Choose a reason for hiding this comment

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

2단계 미션 고생 많으셨어요 :)
생각해볼만한 곳에 코멘트 남겨두었습니다.
피드백 반영 후 다시 리뷰 요청 부탁드려요!

Comment on lines +31 to +48
playerNames.forEach {
it.play(blackjackController)
}

blackjackController.printGameResultStatus()
}

private fun PlayerName.play(blackjackController: BlackjackController) {
while (blackjackController.getHitPossible(this)) {
if (blackjackController.getHit(this).isNo()) {
return
}

val player = blackjackController.hit(this)

blackjackController.printPlayerInfo(player)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

플레이어가 카드를 받는 것은 도메인 객체로 표현해보는 건 어떨까요?

Comment on lines +6 to +8

class BlackjackGameProxy(
private var blackjackGame: BlackjackGame? = null
Copy link
Member

Choose a reason for hiding this comment

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

이 객체가 필요한 이유가 궁금하네요 ?_? 어떤 역할을 가지는 객체일까요?

Comment on lines +2 to +3

class Card private constructor(
Copy link
Member

Choose a reason for hiding this comment

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

카드 인스턴스를 재활용해보는 건 어떨까요?
똑같은 카드를 동일한 인스턴스를 활용할 수 있게끔 52장의 카드를 캐싱해볼 수 있겠어요 :)

Comment on lines +9 to +10
private val CARD_ACE_SCORES = listOf(Score(1), Score(11))
private val CARD_KQJ_SCORES = listOf(Score(10))
Copy link
Member

Choose a reason for hiding this comment

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

CardNumber enum class가 카드의 점수를 갖게 해보는 건 어떨까요?

Copy link
Member

Choose a reason for hiding this comment

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

또, Ace의 점수는 기본적으로 1점입니다.
10점이 되는 경우는 로직으로 녹여내보는 건 어떨까요?

Comment on lines +2 to +3

class CardPool private constructor(private var cards: MutableList<Card>) {
Copy link
Member

Choose a reason for hiding this comment

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

생성자에 Mutable Collection을 넣는 것은 지양해보는 건 어떨까요?

생성자에 넘겨준 가변 컬렉션의 참조를 외부에서 실수로 다루게 되면 이 객체에도 문제가 발생할 수 있어요.

val mutableList = mutableListOf()
val pool = CardPool(mutableList)

mutableList.clear()

방어적 복사를 활용해보는 건 어떨까요?

Comment on lines +4 to +6
fun pickAndRemove(): Card {
check(cards.size > 0) { "카드 풀의 카드가 존재하지 않습니다." }

Copy link
Member

Choose a reason for hiding this comment

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

52장의 카드를 모두 사용하면 Exception이 발생하고 게임이 비정상 종료될 것으로 보여요.
아직 플레이어들이 카드게임을 진행하고 있을 수도 있는데 말이죠!

카드를 다 사용하더라도 게임을 계속 진행할 수 있게 구성해보는 건 어떨까요?
또, 블랙잭 게임의 다른 룰에서는 한 번에 여러 벌의 카드를 가지고 시작하는 경우도 있어요. 이런 요구사항 등에 유연하게 대응할 수 있게 만들어보셔도 좋겠어요!

Comment on lines +14 to +15
return listOf()
}
Copy link
Member

Choose a reason for hiding this comment

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

빈 list는 emptyList() 함수를 활용해볼 수 있어요 :)

Comment on lines +4 to +12
fun scores(): List<Score> {
check(cards.isNotEmpty()) { "카드의 개수가 0장으로 점수를 계산할 수 없습니다." }

return cards
.drop(1)
.fold(cards.getFirstCardScores()) { scores, card -> calculatePossibleScores(card, scores) }
.toList()
.sortedBy { it.score }
}
Copy link
Member

Choose a reason for hiding this comment

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

Score의 List가 필요한 이유는, Ace의 경우때문인걸까요?
다른 개발자들은 이 List<Score>가 어떤 의미를 가지는 지 한 번에 이해할 수 있을지 고민해보시면 좋겠어요 :)

ACE의 점수 힌트를 드리면, ACE는 유리한 경우에만 10점을 더하면 돼요.
1점으로 더한 뒤, 10점을 더했을 때 Bust면, 1점으로 활용하면 됩니다.

또, ACE가 n장이어도 추가점수로 활용하는 건 1개 뿐이에요. 물론 Ace2장을 22점으로 칠 수 있게 룰을 만들어서 죽을 수 있게 만들수도 있겠지만... 대부분의 경우 12점으로 활용할 거예요.
이 원리를 이용한다면 점수 계산을 하는 것은 쉽게 하실 수 있을거예요!

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.

None yet

2 participants