[블랙잭 1단계] 미플 미션 제출합니다 #119
Conversation
| EIGHT("8", 8), | ||
| NINE("9", 9), | ||
| TEN("10", 10), | ||
| JACK("J", 10), |
There was a problem hiding this comment.
View에 표현하기 위해서 value가 생긴 것 같은데요.
그러면 만약 J를 이미지로 교체한다고 했을 때 해당 모델의 변경이 생길텐데 이런 변경사항이 생기는게 맞을지 MVC 관점에서 고민해봅시다.
| fun addCard(card: Card) = _cards.add(card) | ||
|
|
||
| fun addCards(cards: List<Card>) = _cards.addAll(cards) | ||
|
|
There was a problem hiding this comment.
abstract 으로 만들었는데 addCard addCards 이 2개 정말 공존해야 할까요? 사람 입장에서는 한 장씩 받는 것이지 않을까요?
|
|
||
| abstract class Person { | ||
| private val _cards: MutableList<Card> = mutableListOf() | ||
| open val cards get() = _cards.toList() |
There was a problem hiding this comment.
이 값이 open 되어야 할 필요가 있을까요?
그리고 외부로 공개 되어 있는 이유는 지금 이 객체에게 메세지를 던지지 않고 있는 상황이 아닌지 생각해 봅시다.
| enum class ResultType(val value: Char) { | ||
| WIN('승'), | ||
| TIE('무'), | ||
| LOSS('패'), ; |
There was a problem hiding this comment.
해당 value도 view에만 표시하기 위한 선언부인데 만약 승이 라는 것 말고 디자인에서 승리 라고 띄어주고 싶다고 하면 domain model 까지 변경이 되어야 하는 상황이 와버립니다.
| companion object { | ||
| fun judgeScore( | ||
| dealer: Dealer, | ||
| player: Player, | ||
| ): ResultType { | ||
| val dealerFinalScore = if (dealer.isBust()) 0 else dealer.calculateTotalScore() | ||
| val playerFinalScore = if (player.isBust()) 0 else player.calculateTotalScore() | ||
| if (dealerFinalScore < playerFinalScore) return WIN | ||
| if (dealerFinalScore == playerFinalScore) return TIE | ||
| return LOSS | ||
| } | ||
|
|
||
| const val BUST_NUMBER = 21 | ||
| } |
There was a problem hiding this comment.
ResultType 이라는 객체를 봤을 때 결과 에 집중을 하고 있는데 여기서 BUST_NUMBER 도 갖고 있고 승부 결과까지 만들고 있네요.
블랙잭이라는 도메인으로 생각했을 때, 블랙잭은 플레이어와 딜러간의 대결일텐데요. 이 부분에 대해서 천천히 고민해봅시다.
| import org.junit.jupiter.api.assertDoesNotThrow | ||
| import org.junit.jupiter.api.assertThrows | ||
|
|
||
| class DeckTest { |
There was a problem hiding this comment.
Deck을 Object로 만들어 싱글턴으로 만들었는데, 테스트에서 Deck의 카드를 모두 뽑았을 때 예외처리를 테스트하면서 카드를 52장 드로우하니 다른 테스트에서 Deck의 카드를 뽑을 때 에러가 발생했습니다. (coverage 테스트로 실행했을 때)
실제로 프로그램을 실행했을 경우는 이런 경우가 발생하지 않는데, 이런 테스트 때문에 object를 다시 class로 변경해야 하는지, 카드를 모두 뽑았을 때 예외처리하는 테스트를 제거해야 하는지 궁금합니다!
위와 같은 질문이 해당 테스트 같은데요. 우선
실제로 프로그램을 실행했을 경우는 이런 경우가 발생하지 않는데, 이런 테스트 때문에 object를 다시 class로 변경해야 하는지,
테스트 때문에 object -> class로 변경을 한다. 이 관점에서 봤을 때 2가지 측면이 있습니다.
- 잘못된 테스트이다.
- object로 구현하면 안 된다.
이 부분을 기억하고 다음 질문을 보면
카드를 모두 뽑았을 때 예외처리하는 테스트를 제거해야 하는지 궁금합니다!
테스트를 제거한다.라는 관점으로 봤을 때 아까 1번의 상황이라고 판단이 어느정도 되신 것 같은데요.
근데 지금 만약 게임이 1판이 아니고 2판 3판 이뤄진다
위 경우를 생각 해보면 이 테스트가 필요했을지 아닐지 직접 판단하실 수 있을 것이라고 생각합니다.
| private lateinit var dealer: Dealer | ||
|
|
||
| @BeforeEach | ||
| fun setUp() { | ||
| dealer = Dealer() | ||
| } |
There was a problem hiding this comment.
테스트가 독립적으로 수행될 수 있게 잘해주셨습니다. 단 구조적인 리뷰가 진행됨에 따라 테스트가 변경이 많이 될 수도 있기에 테스트는 구조적인 리뷰 반영 혹은 의견을 서로 나누고서 다시 한번 보겠습니다.
| private fun playerDrawOrStay(player: Player) { | ||
| val condition: DrawChoice = inputView.readMoreCardCondition(player) | ||
| if (condition.isStay()) { | ||
| outputView.printPlayerHands(player) | ||
| return | ||
| } | ||
| gameManager.drawCard(player) | ||
| outputView.printPlayerHands(player) | ||
| if (player.isBust()) return | ||
| playerDrawOrStay(player) | ||
| } |
There was a problem hiding this comment.
이 함수가 단순 플레이어가 draw 하거나 stay를 하겠구나 하고 봤는데요 지금 �너무 어렵게 구현이 되어있는 것 같아요.
return을 2개나 하고 있고 재귀로 호출이 되게끔 만들었는데요.
while과 when문을 적절하게 섞으면
지금 어떠한 상태 일 때 player의 턴을 종료한다.
라는 것을 조금 더 명시적으로 나타낼 수 있을 것 같습니다.
|
|
||
| fun isMoreCard() = calculateTotalScore() < DEALER_MORE_CARD_MINIMUM | ||
|
|
||
| companion object { |
There was a problem hiding this comment.
companion object 는 어디에 위치를 해야 하는가. 를 확인해보시면 좋을 것 같네요.
https://kotlinlang.org/docs/coding-conventions.html#class-layout
| @@ -0,0 +1,14 @@ | |||
| package blackjack.model | |||
|
|
|||
| abstract class Person { | |||
안녕하세요 코니. 잘 부탁드립니다!
셀프 체크리스트
README.md에 기능 목록을 정리하고 명확히 기술하였는가?어떤 부분에 집중하여 리뷰해야 할까요?
실제로 프로그램을 실행했을 경우는 이런 경우가 발생하지 않는데, 이런 테스트 때문에 object를 다시 class로 변경해야 하는지, 카드를 모두 뽑았을때 예외처리하는 테스트를 제거해야 하는지 궁금합니다!