[블랙잭 1단계] 오이 미션 제출합니다. #109
Conversation
- Person 추상 클래스 사용해 중복 코드 제거
- 승패 여부는 Player가 관리
- card - person
- 플레이어 객체의 ResultState 제거
- 개행 & Depth 수정 필요
- GameRule Object 사용해 상수 관리 - Controller 도메인 로직 제거
- copy 메소드 추가
- isHit -> hitFlag
- 딜러 버스트 케이스
- UI Model 반영 - 상수 분리
- 상수 분리
- 딜러의 첫 카드만 공개합니다
- GameResult가 결과를 상태로 갖습니다
| class Deck { | ||
| private val _cards = mutableListOf<Card>() | ||
| val cards: List<Card> get() = _cards.toList() | ||
|
|
||
| init { | ||
| _cards.addAll(generateDeck()) | ||
| require(cards.size == DECK_SIZE) { INVALID_DECK_SIZE_ERROR_MESSAGE } | ||
| } |
There was a problem hiding this comment.
딜러와 플레이어의 손패 (List) 를 감싸기 위해 Hand라는 일급 컬렉션을 구현했습니다. 또한 Person이라는 추상 클래스를 만들어 딜러와 플레이어 클래스를 구현하고 Person 내부에서 Hand를 관리하는데, 테스트 코드에서 특정 점수에 대한 테스트 등 여러 테스트에서 원하는 대로 카드 목록을 구성하기 위해서 플레이어와 딜러 객체를 생성할 때 Hand 객체를 생성자로 받을 수 있게끔 코드를 작성했습니다. 실제 도메인 로직에서는 Deck에서 카드를 무작위로 생성하여 전달해주기 때문에 위와 같이 코드를 작성했는데, 이 방식과 Deck 자체를 인터페이스로 추상화하여 테스트 코드에서 고정된 카드 목록을 전달해주는 방식 중 어느 방식이 더 바람직한지 둘리의 의견이 궁금합니다.🤔
우선 저는 대체로 초기값을 전달해주는 편이에요. 이것만으로도 테스트가 쉬워진다고 생각하거든요. 카드를 전달해주거나, 카드 생성 방식을 전달해주거나요! 예시는 여러가지가 있을 것 같아요.
지금은 객체를 생성해서 어떤 함수를 실행한 다음에 assert가 돌아가도록 되어있지만, 초기값을 넣을 수 있다면 함수를 실행하지 않아도 되겠죠.
코드 전체적으로 초기값을 생성자에서 받기보다는, 빈 값을 넣고 액션을 하도록 되어있는데요. default parameter를 사용해볼 수도 있고, 빈 값으로 넣어주면 같은 동작을 하니, 각 도메인 모델들의 생성자를 한번 살펴보면 좋겠어요.
어떻게 건드리면 테스트코드가 쉬워질지 눈에 보일거에요!
| companion object { | ||
| fun create( | ||
| number: CardNumber, | ||
| pattern: CardPattern, | ||
| ): Card { | ||
| return Card(number, pattern) | ||
| } | ||
| } |
| import blackjack.domain.person.Person | ||
| import blackjack.domain.person.Player | ||
|
|
||
| data class PersonUiModel( |
There was a problem hiding this comment.
복잡한 출력 형식을 맞추기 위해서 도메인 모델의 데이터를 UI 모델을 거쳐서 View에 전달되도록 하였습니다. 현재 UI 모델 내부에서 출력해야하는 문자열의 일부를 직접 구성해주고 있는데(열거형 클래스 타입에 따른 분기 처리 등), 이런 식으로 사용하는 것은 View의 역할까지 담당하게 되는 것인가요?
사실 UiModel을 사용하는 경우는 실제 안드로이드 개발에서 자주 사용되긴 합니다. Data - Domain - Presentation 계층으로 나누게 되면서 각 계층의 모델들을 만들 수 있어요. (지금 당장 몰라도 되는 내용이니 그렇구나 하고 넘어가주세요.ㅎㅎ)
우선 저는 Ui Model이 도메인이 아니라고 생각해요. 지금 구조에서는 View의 역할에 가깝다고 생각합니다.
이 모델을 그대로 안드로이드로 옮겨야한다면, 안드로이드에서는 실제 이미지를 출력하기도 할거고, 색깔이 입혀지기도 할거고, 더 다양한 UI 요소들이 포함되겠죠?
하지만 그렇다고 해서 잘못된 구조라고 생각하지 않습니다. 지금 요구사항에서는 View가 Domain 모델을 알아도 되지만, 분리를 한다고 해서 잘못됐다고 생각하지는 않아요.
대신 앞으로 미션 구현하면서 이러한 Ui Model들이 View에서 사용될 수 있도록, View가 Domain 모델을 사용하지 않도록 일관적인 코드를 작성하는 것이 중요하겠어요!
| class Dealer(hand: Hand) : Person(hand) { | ||
| init { | ||
| gameState = DealerState.FIRST_TURN | ||
| } | ||
|
|
||
| constructor() : this(hand = Hand()) |
There was a problem hiding this comment.
이 부분이 위에서 언급했던 점입니다.
특정 카드들을 플레이어나 딜러가 가지고 있을 때의 상황을 테스트하기 위해서 Hand를 생성자로 전달하거나, 전달하지 않을 경우 내부에서 기본으로 생성하게 됩니다.
때문에 외부에서 부 생성자로 Hand를 받아올 경우 외부와의 참조를 끊기 위해 Hand에 copy를 선언하고 그대로 사용했습니다.
| companion object { | ||
| fun from(dealer: Dealer): DealerState = | ||
| when { | ||
| dealer.cards().size < GameRule.FIRST_TURN_DRAW_AMOUNT -> FIRST_TURN | ||
| dealer.score() > GameRule.DEALER_ADDITIONAL_DRAW_BASE_SCORE -> FINISH | ||
| else -> HIT | ||
| } | ||
| } |
There was a problem hiding this comment.
State들이 객체를 받기 보다는 실제 값들(카드 개수, 스코어)을 받아서 판단하면 어떨까요?
여기서 관심사는 사람보다는 사람이 가지고 있는 것들이라고 생각해요!
| protected fun getDrawAmount(state: PersonState): Int { | ||
| if (gameState == state) { | ||
| return GameRule.FIRST_TURN_DRAW_AMOUNT | ||
| } | ||
| return GameRule.HIT_DRAW_AMOUNT | ||
| } |
There was a problem hiding this comment.
state가 같으면 2장, 같지 않으면 1장 이런 로직을 갖고 있는데요, State에 따른 뽑을 수 있는 카드 장수를 선언하면 어떨까요?
There was a problem hiding this comment.
상태에 따른 분기 처리, 예외 처리까지 할 수 있을 것 같아요.
좋은 의견 감사합니다😊
| ) | ||
| } | ||
|
|
||
| private fun Card.toUiString(): String = "${number.toUiString()}${pattern.toUiString()}" |
|
|
||
| private fun CardNumber.toUiString(): String = | ||
| when (this) { | ||
| CardNumber.ACE -> "A" |
There was a problem hiding this comment.
페어와 이 부분에서 해당 상수들을 GameRule에서 관리하기 애매하다고 생각해서 바꾸지 않은 부분인데, 개선해보겠습니다!
| fun `고유한 52장의 카드를 가지고 있어야 한다`() { | ||
| val deck = Deck() | ||
|
|
||
| deck.cards.size shouldBe 52 |
| assertThat(card.number).isEqualTo(CardNumber.ACE) | ||
| assertThat(card.pattern).isEqualTo(CardPattern.HEART) |
There was a problem hiding this comment.
assert를 한 함수에 여러번 쓰면 어떤 불편함이 생길 수 있을까요?
There was a problem hiding this comment.
assertThat중 하나라도 검증에 실패하면 이후 assertThat이 실행되지 않는 문제가 있었네요.
kotest나, assertAll을 활용해보겠습니다!
셀프 체크리스트
README.md에 기능 목록을 정리하고 명확히 기술하였는가?어떤 부분에 집중하여 리뷰해야 할까요?
안녕하세요 둘리! 🫡
1차 리뷰 잘 부탁드립니다!!
딜러와 플레이어의 손패 (
List<Card>) 를 감싸기 위해Hand라는 일급 컬렉션을 구현했습니다. 또한Person이라는 추상 클래스를 만들어 딜러와 플레이어 클래스를 구현하고Person내부에서Hand를 관리하는데, 테스트 코드에서 특정 점수에 대한 테스트 등 여러 테스트에서 원하는 대로 카드 목록을 구성하기 위해서 플레이어와 딜러 객체를 생성할 때Hand객체를 생성자로 받을 수 있게끔 코드를 작성했습니다. 실제 도메인 로직에서는Deck에서 카드를 무작위로 생성하여 전달해주기 때문에 위와 같이 코드를 작성했는데, 이 방식과Deck자체를 인터페이스로 추상화하여 테스트 코드에서 고정된 카드 목록을 전달해주는 방식 중 어느 방식이 더 바람직한지 둘리의 의견이 궁금합니다.🤔복잡한 출력 형식을 맞추기 위해서 도메인 모델의 데이터를 UI 모델을 거쳐서 View에 전달되도록 하였습니다. 현재 UI 모델 내부에서 출력해야하는 문자열의 일부를 직접 구성해주고 있는데(열거형 클래스 타입에 따른 분기 처리 등), 이런 식으로 사용하는 것은 View의 역할까지 담당하게 되는 것인가요?
코드 리뷰 커뮤니케이션
📌 GitHub에서 PR에 댓글 남기는 방법
참고 자료
스크린샷