[부나] 2단계 블랙잭 제출합니다. #51
Conversation
- STAY_SCORE -> DEALER_HIT_MAX_SCORE
src/test/kotlin/blackjack/domain/participant/ParticipantsTest.kt
Outdated
Show resolved
Hide resolved
| override fun nextStateCondition(): Boolean = cards.isBust | ||
|
|
||
| override fun nextState(): CardState = BustState(cards) |
There was a problem hiding this comment.
히트 상태에서는 스테이 혹은 버스트로 넘어갈 수 있지 않을까요?
다음 상태가 무조건 버스트가 되는 것이 맞을까요?
다음 상태라는 추상 함수로 정의하는 것이 맞을까? 를 생각 해보세요
There was a problem hiding this comment.
제가 잘 이해한지 모르겠습니다만..🥲
히트 상태에서는 스테이 혹은 버스트로 넘어갈 수 있지 않을까요?
우선 히트 상태에서는 스테이와 버스트가 될 수 있습니다.
플레이어라면 n을 입력했을 때, 딜러라면 17점 이상일 때 스테이가 될 수 있습니다.
따라서 각 Participant의 draw()를 호출했을 때 canDraw()가 거짓이라면 Stay되도록 하였습니다.
다음 상태가 무조건 버스트가 되는 것이 맞을까요?
따라서 카드의 점수가 21점을 초과했다면 BustState를 반환하고, 그것이 아니라면 HitState를 반환하도록 수정하였습니다.
다음 상태라는 추상 함수로 정의하는 것이 맞을까? 를 생각 해보세요
다음 상태를 추상 함수로 정의하기보다 draw()에서 카드를 뽑았을 때의 상태를 반환하도록 하는 것이 적합해보입니다.
- 하나의 블랙잭 객체에 대해 여러번 시작 함수 호출시 소모된 카드덱을 재사용하는 문제 발생하던 오류 수정
|
저의 만행으로 인해 코드가 많이 변경되었을텐데.. 다시금 리뷰해주셔서 정말 감사드립니다..!! 🥹 |
| Blackjack(deck = CardDeck(), players = InputView.inputPlayers()).start(object : BlackjackEventListener { | ||
| override fun onStartDrawn(participants: Participants) { | ||
| OutputView.printFirstDrawnMessage(participants) | ||
| } | ||
|
|
||
| override fun onFirstDrawn(participant: Participant) { | ||
| OutputView.printFirstOpenCards(participant) | ||
| } | ||
|
|
||
| override fun onDrawnMore(participant: Participant) { | ||
| OutputView.printAllCards(participant) | ||
| } | ||
|
|
||
| override fun onEndGame(gameResults: List<GameResult>) { | ||
| OutputView.printBlackjackResult(gameResults) | ||
| } | ||
| }) |
There was a problem hiding this comment.
리스너로 잘 묶어 주셨네요.
OutputView에서 Listener 자체를 반환하도록 하면 조금 더 마법이 생겨납니다
Blackjack(deck, InputView::inputPlayers, OutputView::eventListener)There was a problem hiding this comment.
오오.. 이런 마법이..!
이 코드를 참고해서 OutputView에서 BlackjackEventListener를 상속하도록하여 OutputView 자체를 넘겨주는 방식으로 해보았습니다!
object OutputView : BlackjackEventListener {
.
.
.
}
Blackjack(CardDeck(), InputView.inputPlayers(), OutputView).start()
| class Cards(private val _items: MutableList<Card> = mutableListOf()) { | ||
| constructor(vararg cards: Card) : this(cards.toMutableList()) |
There was a problem hiding this comment.
가변 컬렉션 타입을 바로 받는 대신 방어적 복사를 활용하면 좋을 것 같습니다.
여전히 외부에서 items를 변경시킬 수 있습니다.
그런데 add()함수가 불변 객체를 지향하기 때문에, 다른 상태를 변경하는것도 모두 필요없지 않을까? 라는 생각도 듭니다
There was a problem hiding this comment.
val cardItems = mutableListOf<Card>(Card(), Card() ...)
val cards = Cards(cardItems)
cardItems.add(Card()) // 불변 X
위와 같은 상황이 발생할 수 있겠군요!
제가 올바르지 않은 일급 컬렉션의 예시를 하나 추가해버렸군요..😅
라코데브님 말씀처럼 add() 함수가 불변 객체를 지향하므로 아래와 같이 변경하였습니다!
class Cards(val items: List<Card> = listOf())
| @Test | ||
| fun `배팅 금액을 곱했을 때 값이 음수일 수 없다`() { | ||
| assertThrows<IllegalArgumentException> { | ||
| BetMoney(1000) * -2.0 | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
결국 BetMoney 타입이기 때문에 곱한 것과 상관 없이 이렇게 작성하는게 맞겠네요.
fun `배팅 금액은 음수일 수 없다`() { ... }There was a problem hiding this comment.
배팅 금액은 결국 음수일 수 없기 때문에 곱하는 것과 무관하게 제안해주신 테스트 코드명이 올바른 것 같습니다!
바로잡아주셔서 감사합니다!
|
|
||
| /** | ||
| 플레이어 손익의 반대이다. | ||
| */ | ||
| override fun getProfit(other: Participant): Money = -other.getProfit(this) | ||
| override fun getProfit(others: List<Participant>): Money { | ||
| val players = others.filterIsInstance<Player>() |
There was a problem hiding this comment.
하나의 마이너한 팁이라면 딜러가 플레이어 타입을 직접 아는 것 보다는, 반대로
"딜러가 아닌"으로 조건을 바꿔보는 것도 방법입니다.
그럼 다른 객체와의 의존 관계가 사라집니다
others.filterNot { it is Dealer }There was a problem hiding this comment.
정말 감동적인 코드입니다.. 🫶
기존 코드는 클래스간의 의존성이 생기는군요
Player 또한 아래와 같이 변경하였습니다!
val dealer = others.first { it !is Player }
| override fun getProfit(other: Participant): Money = -other.getProfit(this) | ||
| override fun getProfit(others: List<Participant>): Money { | ||
| val players = others.filterIsInstance<Player>() | ||
| return -Money(players.sumOf { it.getProfit(listOf(this)).getAmount() }) |
There was a problem hiding this comment.
players를 만드는 메소드에 체이닝을 이용해서 만들어 볼 수 있지 않을까요?
val playerTotalProfit = others.xxx
.sumOf { ... }
reteurn Money(...)There was a problem hiding this comment.
감사합니다!
아래와 같이 메소드를 변경해보았습니다!
override fun getProfit(others: List<Participant>): Money = Money(others
.filterNot { it is Dealer }
.sumOf { player -> player.getProfit(listOf(this)).getAmount() }
)
Player도 함께 변경해볼까 했지만
dealer를 변수에 할당해야 가독성이 높아 보여서 Dealer.class만 적용하였습니다.
| abstract fun getFirstOpenCards(): List<Card> | ||
|
|
||
| abstract fun draw(card: Card): Participant | ||
| abstract fun draw(card: Card, justDraw: Boolean = false): Participant |
There was a problem hiding this comment.
Player의 경우 draw() 메서드 호출시 canDraw()를 호출합니다.
이 때 Player의 canDraw()에는 사용자에게 카드를 더 뽑을 것인지 묻는 needToDraw() 함수를 호출합니다.
따라서 최초에 카드를 2장 뽑을 때에도 의도치 않게 needToDraw() 함수를 호출하는 문제가 발생합니다.
그래서 처음 카드는 무조건 뽑을 수 있는 조건을 추가해주기 위해 justDraw를 추가하였습니다.
- Dealer는 Dealer가 아닌 참여자들을 탐색하도록 변경 - Player는 Player가 아닌 참여자를 한명 탐색하도록 변경
라코데브님, 안녕하세요!
너무나도 면밀하게 작성해주신 1단계 피드백 덕분에 정말 많이 성장할 수 있었습니다.
찾아뵙고 인사드리고 싶을 정도로 감동받았습니다..! 🥹
이번 2단계 미션을 진행하면서
카드의 상태에 집중하라는 리뷰어님의 피드백을 생각해보았습니다.그리고 수업시간에 다룬 State 패턴과 Template method 패턴을 적용해보았습니다.
올바르지 않은 구현일 수 있지만, 위 패턴을 적용하면서 수익을 계산하는 코드가 상태별로 분리되어 있어서 코드를 수정할 때 수월했습니다!
혹시나..... 라코데브님께서 괜찮으시다면 더 채찍질해주셔도 좋을 것 같습니다..!
부족한 부분에 대해 많이 배울 수 있어서 리뷰 하나하나가 소중하게 느껴집니다 😃