-
Notifications
You must be signed in to change notification settings - Fork 313
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
Step3 블랙잭(딜러) #785
base: duhanmo
Are you sure you want to change the base?
Step3 블랙잭(딜러) #785
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 두한님!
리뷰가 조금 늦었네요 ㅠㅠ
몇가지 고민할법한 코멘트 남겼으니, 확인해주세요!
fun compareScore(other: Cards): MatchResult { | ||
return when { | ||
score > other.score -> WIN | ||
score < other.score -> LOSS | ||
else -> DRAW | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
점수가 같더라도, 블랙잭 여부에 따라서 승패가 나눠질수도 있어요!
처음 받은 2장 합쳐 21이 나오는 경우 블랙잭이 되며
https://namu.wiki/w/%EB%B8%94%EB%9E%99%EC%9E%AD(%ED%94%8C%EB%A0%88%EC%9E%89%20%EC%B9%B4%EB%93%9C)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네! 해당부분도 고려하도록 할게요🙂
import blackjack.view.InputView | ||
import blackjack.view.ResultView | ||
|
||
data class BlackjackGame( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BlackjackGame을 data class 로 정의한 이유가있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
일관성을 맞추기 위해 특별한 경우가 아니면 data class 로 선언을 했는데요,
좀더 의미를 가지고 정의를 하도록 할게요🙂
--
추가 반영하며 data class와 일반 class를 나눈 근거는
객체가 직접 자신의 상태를 변경하며 관리하는 클래스는 일반 class,
내부 로직이 없으며 데이터로서의 역할을 하는 클래스는 data class로 선언하였어요!
private fun printGameResult(participants: List<Participant>) { | ||
resultView.linebreak() | ||
resultView.printGameResult(GameResult.from(participants)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BlackjackGame은 Controller역할을 하고 있어요,
하지만 BlackjackGame이 너무 비대하진 않을까요?
너무 많은 로직이 있는거 같아요
책임을 분리해보아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네! 말씀대로 GameTable을 service로직처럼 상위패키지(controller)로 추출하여 로직을 분담하도록했어요🙂
private fun printCard(participants: List<Participant>) { | ||
resultView.linebreak() | ||
resultView.printParticipantsCard(participants = participants, printScore = true) | ||
} | ||
|
||
private fun printGameResult(participants: List<Participant>) { | ||
resultView.linebreak() | ||
resultView.printGameResult(GameResult.from(participants)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 로직들은 Controller보다는 View의 책임은 아닐까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네! View에 위임하도록 할게요👍
resultView.linebreak() | ||
resultView.printInitCardReceive(participants) | ||
resultView.printParticipantsCard(participants = participants, printScore = false) | ||
resultView.linebreak() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
resultView에게 책임을 전달해주면 어떨까요?
resultView를 사용하여 print하는게 아니라
resultView에게 책임을 주어 메세지를 던져서 일을 시키는 구조를 고민해보면 좋을거같아요 :)
View가 콘솔뷰가 아니라, 웹이나 모바일 화면이라면
View와 의존성이 크지 않은 Controller의 로직들도 많이 바뀌어야하진 않을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네! 반영하도록 할게요😄
|
||
private fun playGame(gameTable: GameTable): List<Participant> { | ||
val participants = setUpInitCard(gameTable) | ||
val (players, dealer) = Participant.separate(participants) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
딜러와 플레이어를 분리해서 사용한다면,
애초에 participants로 하나로 합칠 필요가 있을까요?
일부분에서는 participants를 통해 함께 관리하고,
일부분이서는 separate를 통해 따로 관리하는 구조는 일관성이 조금 없는거 같단 생각이 들어요 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네! 피드백 반영하며 수정하였어요😀 플레이어와 딜러의 상위클래스를 따로 두지 않고 플레이어의 기능만을 상속받는 딜러를 구현하였어요😊
fun compareScore(dealer: Dealer): MatchResult { | ||
return when { | ||
dealer.isBust -> WIN | ||
this.isBust -> LOSS | ||
else -> cards.compareScore(dealer.cards) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
승패를 결정하는 로직이 Cards, Player 객체에 혼재되어있는건 아닐까요?
책임을 하나의 객체로 위임해보면 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네! 해당 결정로직이 카드까지 흘러들어가지 않고 (반영후에는 Hand 클래스) 플레이어가 판단하도록 수정하였어요😊
import blackjack.domain.MatchResult.LOSS | ||
import blackjack.domain.MatchResult.WIN | ||
|
||
sealed class Participant(val name: String, val cards: Cards) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
일반적으로 코틀린에서는 클래스별로 하나의 파일에서 관리되도록 가이드하고 있긴합니다 :)
이부분은 스타일마다 다를수 있으니, 참고만해주세요 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네! 조언 감사합니다🙇♂️
private val deck: Deck, | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
게임테이블에서 참가자목록을 갖고있도록 했었는데,
참가자가 hit할때마다 새로운 참가자로 카피되어서 나오다보니, 게임테이블에서 갖고있게 되는(최초생성할 때 인자로넣어주는) 참가자목록은 변하지않고 계속 hit 하지 않은 최초 참가자가 되더라구요.
그래서 참가자를 따로 메서드에서 인자로받아 처리하도록 수정했어요.
먼저 게임테이블에서 참가자목록을 가진 불변 컬렉션으로 만들었다면,
게임 진행될때마다, 새로운 상태의 게임테이블을 반환하게 할수도 있고,
게임테이블에서 유저의 상태를 관리하는 책임을 가질수도 있을거같네요 :)
이렇게 하다보니 불변성을 지키려할수록 객체지향스러운가? 싶은 고민들이 생겨났어요🙄 (특히, Cards 입장에서 hit 할 때마다 새로운 Cards 객체를 생성해서 반환하는 부분) 이부분에 대한 남재님의 의견이 특히 궁금해요😄
불변성과 객체지향은 조금 다른 이야기라고 볼수 있어요 :)
객체지향이 항상 불변성을 가진것도 아니고, 불변성이 항상 객체지향을 의미하지 않습니다!
불변성과 객체지향 서로 다르지만, 서로 대립되는 개념 또한 아니기도합니다.
어떻게 보면 불변성은 함수형프로그래밍에서 강조되는 부분이기도하고,
객체지향에서 객체내 상태가 변경되는것도 자연스럽다고 볼수도 있어요 :)
각각의 장단점을 고려하면서, 요구사항에 따라서 적절히 조화를 하는게 가장 이상적인 방법은 아닐까요?
코드에는 정답이 없으니, 두한님만의 규칙과 철학을 고민해보면 어떨까요?
객체가 상태를 관리해야하는 비즈니스로직이 있다면, 객체가 내부적으로 상태관리를 해주는게 맞지는않을까요?
개인적으로는 결국 BlackJackGame 내에서는 Controller역할 이외에도 블랙잭 게임의 책임을 가지고있고,
참가자들에 대한 상태 관리 책임까지도 가지고 있는건 아닐까요?
안녕하세요 남재님! 그럼 이번에도 잘부탁드립니다🙇♂️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세여 두한님!
피드백 잘적용하셨네요 :)
몇가지 추가적으로 코멘트 남겼으니, 확인해주세요!!
import blackjack.view.InputView | ||
import blackjack.view.ResultView |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BlackjackGame , GameTable은 어떤 책임이 있을까요?
BlackjackGame은 Controller역할을 하고 있다면, GameTable은 전반적인 블랙잭 게임을 관리하고 있는거같아요
GameTable은 비즈니스로직 영역은 아닐까요?
GameTable에서 View의 의존도를 제거해보면 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GameTable에서 게임을 진행하며 (turn) 카드상태를 출력해줘야 하는 요구사항을 만족시키느라 View에 대한 의존성이 들어가게되었는데요,
고민을 해보아도 출력에 대한 의존을 어떻게 끊을지 마땅한 방법이 떠오르질 않아요 🥲
혹시 생각나시는 방법이 있으시다면 조언 부탁드려도 될까요 남재님!?🙇♂️
dealerTurn() | ||
} | ||
|
||
fun getGameResult(): GameResult = GameResult.from(dealer, players) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get으로 접근하기보다 함수의 결과값으로 전달해보면 어떨까요?
val gameResult = gameTable.playGame( ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵! 해당방법으로 수정하도록 할게요 🙂
fun getGameResult(): GameResult = GameResult.from(dealer, players) | ||
|
||
private fun playerTurn(player: Player) { | ||
if (!player.canHit() || !InputView.inputHit(player)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
InputView에 대한 의존성을 어떻게 끊을수 있을까요?
여러 방법있겠지만,
간단하게 람다를 전달하여 분리하는 방식도 있을거같아요 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 제일 어렵게 느껴지는 부분인데🥲
람다를 이용해서 InputView에 대한 의존성을 끊도록 수정해보겠습니다!
fun from( | ||
dealer: Dealer, | ||
players: List<Player>, | ||
): GameResult { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dealer, players 를 파라미터로 받아서 연산하기보단,
결과를 다루는 동작은 dealer에게 있으면 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네! 그게 좀 더 의미있을 것 같아요 dealer에게 결과를 다루는 동작을 위임하도록 해볼게요🙇♂️
data class GameResult( | ||
val dealerGameResult: DealerGameResult, | ||
val playerGameResults: List<PlayerGameResult>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dealerGameResult와 playerGameResults 는 서로 의존적인 상태를 가진거 같아요,
playerGameResults만 가지고 있다면, dealerGameResult를 계산할수 있을거같아요 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네! 말씀하신대로 의존상태 분리하여 계산해보도록 할게요😊
|
||
open fun hit(card: Card) = hand.add(card) | ||
|
||
fun matchHand(other: Player): MatchResult = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
other, this 중 누가 딜러냐에 따라서 승패가 결정되곤해요
플레이어가 버스트라면, 딜러의 점수와 상관없이 딜러 승리입니다 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아 ! 그렇군요 😃
승패로직을 좀 더 꼼꼼하게 작성하도록 하겠습니다🙇♂️
when { | ||
other.isBust() -> WIN | ||
this.isBust() -> LOSE | ||
other.isBlackjack() && this.isBlackjack() -> DRAW | ||
other.isBlackjack() -> LOSE | ||
this.isBlackjack() -> WIN | ||
else -> compareScore(other) | ||
} | ||
|
||
private fun compareScore(other: Player): MatchResult = | ||
when { | ||
this.hand.score > other.hand.score -> WIN | ||
this.hand.score < other.hand.score -> LOSE | ||
else -> DRAW | ||
} | ||
|
||
private fun isBust(): Boolean = hand.score > PLAYER_SCORE_LIMIT | ||
|
||
private fun isBlackjack(): Boolean = hand.score == PLAYER_SCORE_LIMIT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Player 책임에 Score관련된 책임이 많은거같아요,
hand.score 를 통해 연산을 하고있어요, hand의 책임은 아닐까요?
관련된 책임을 Score 객체로 별도로 분리해보면 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네! Score 관련된 책임을 추출해보도록 하겠습니다🙇♂️
|
||
private fun isBust(): Boolean = hand.score > PLAYER_SCORE_LIMIT | ||
|
||
private fun isBlackjack(): Boolean = hand.score == PLAYER_SCORE_LIMIT |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blackjack은 2장으로 이루어진 21점만 해당해요!
3장이상의 21점은 Blackjack이 아닙니다! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네! 블랙잭 여부 로직 수정하도록 할게요! 😀
Then("카드를 반환한다") { | ||
deck.draw().rank shouldBe ACE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then("카드를 반환한다") { | |
deck.draw().rank shouldBe ACE | |
val card = deck.draw() | |
Then("카드를 반환한다") { | |
card.rank shouldBe ACE |
실행부와 검증부와 명확하게 나누면,
가독성 측면이나, 무엇을 테스트해야하는지 좀더 명확히할수 있을거 같아요 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 테스트에 명확성을 부여하도록 하겠습니다 🙇♂️
Given("플레이어가 이긴 경우") { | ||
val dealer = Dealer() | ||
val player1 = Player("유저1") | ||
val player2 = Player("유저2") | ||
|
||
dealer.hit(createCard("3", SPADE)) | ||
player1.hit(createCard("4", HEART)) | ||
player2.hit(createCard("5", DIAMOND)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
게임 결과에 대한 테스트지만,
테스트 세팅과정에 hit 함수도 실행이 되고 있네요,
hit함수에 문제가 있다면, 해당 테스트 코드도 정상적으로 동작하지 않을거 같아요!
테스트 의존성이 있다고 볼수도 있을거 같아요 :)
독립적인 테스트를 하기 위해 생성시점에 cards를 주입받아도 좋을거같단 생각이 들어요 :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네! 좋은 의견감사합니다 수정하도록 할게요🙂
안녕하세요 남재님!
이전 스텝에서 받았던 리뷰를 반영해보았어요🙂
추가로
게임테이블에서 참가자목록을 갖고있도록 했었는데,
참가자가 hit할때마다 새로운 참가자로 카피되어서 나오다보니, 게임테이블에서 갖고있게 되는(최초생성할 때 인자로넣어주는) 참가자목록은 변하지않고 계속 hit 하지 않은 최초 참가자가 되더라구요.
그래서 참가자를 따로 메서드에서 인자로받아 처리하도록 수정했어요.
이렇게 하다보니 불변성을 지키려할수록 객체지향스러운가? 싶은 고민들이 생겨났어요🙄 (특히, Cards 입장에서 hit 할 때마다 새로운 Cards 객체를 생성해서 반환하는 부분) 이부분에 대한 남재님의 의견이 특히 궁금해요😄
그럼, 이번 스텝도 리뷰 잘부탁드립니다! 🙇♂️