💡 Intro
세 번째 미션 블랙잭. 다른 미션들과 다르게 블랙잭이라는 게임에 대해 알고 있는 것이 굉장히 부족했고, 그래서인지 더 어렵게 느껴졌다. 나름 블랙잭 규칙에 대해 공부했고, 알고있다고 생각했는데 리뷰가 올 때마다 부족한 부분이 많았다는 것을 알게 되었다. 리뷰어님께 굉장히 죄송한 마음이다..
블랙잭 게임
로또미션에서 배운 TDD, 일급컬렉션 등의 내용은 디폴트로 가져가고, 이외에 추가된 요구사항은 다음과 같다.
1. 모든 엔티티를 작게 유지한다.
2. 3개 이상의 인스턴스 변수를 가진 크래스를 사용하지 않는다.
3. 딜러와 플레이어에서 발생하는 중복코드를 제거해야 한다.
도메인
나무위키를 통해 블랙잭 도메인에 대해 간단하게 알아보자.
딜러에게 카드를 한 장씩 받아 21에 가까운 수를 만드는 사람이 이기며 21을 초과하면 지는 게임이다.
더 자세한 규칙은 다음과 같다.
- 숫자 카드 2~9는 그 숫자대로 점수를 세고, J,Q,K는 10점으로 계산한다. 특이하게 A는 1점 또는 11점 둘 중의 하나로 계산할 수 있다.
- 처음 2장의 카드가 에이스와 10(J,Q,K를 포함)으로 21점이 된 것을 블랙잭이라고 한다. 딜러가 블랙잭이라면 같은 블랙잭을 가진 사람 이외의 플레이어는 모두 패배한다.
- 딜러와 참가자가 동시에 블랙잭인 경우 푸시(Push)라 하며 무승부가 된다.
- 참가자들은 블랙잭이 아닌 경우, 합계가 21점에 가까워지도록 하기 위해 딜러로부터 카드를 추가로 받을 수 있다.
- 추가 카드는 합계가 21이 되거나, 초과하지 않는 한 제한없이 몇 장이라도 요구할 수 있다.
- 딜러는 참가자들의 추가카드 받기가 모두 끝난 뒤에 정해진 규칙에 따라 카드를 더 받을 것인지를 결정한다.
- 딜러가 가진카드의 합계가 16점 이하면 반드시 1장을 더 받아야 하고, 17점 이상이면 카드를 더 받지 않고 멈춘다.
Hit : 처음 2장의 상태에서 카드를 더 뽑는 것을 말한다.
Stand, Stay : 카드를 더 뽑지 않고 차례를 마치는 것을 말한다.
Bust : 카드 총합이 21점을 넘는 경우를 말한다.
Blackjack : A 한장과 10에 해당하는 패(10,J,Q,K)로 21을 이루는 경우를 말한다.
❗ 페어 프로그래밍 (Step1)
새로운 페어를 만나 3일 동안 열심히 미션을 진행했다. 이번에도 둘 다 변수, 함수의 이름을 짓는데 영.. 이어서 꽤나 힘들었다.
로또에서 배웠던 클래스를 객체의 능동적인 관리자로 적용하기 위해 52장의 카드를 인스턴스화 해놓은 뒤 가져다 쓰는 것이 어떤가?에 대해 이야기를 나누었다. 여기서 페어가 트럼프 카드는 4종류의 슈트와 13개의 끗수로 이루어져 있으니 4*13으로 하면 더 효율적이지 않을까? 라는 의견을 제시했고, 나도 동의했다. 그래서 이 부분을 적용하려고 많은 시간을 쏟았다.(리뷰를 받고 리팩토링하면서 이 방법을 고수했던 것이 굉장히 비효율적인 노력이었다.라고 느꼈다...😢)
카드의 슈트와 끗수를 Enum class로 만들었는데 여기서도 페어의 의견이 많이 반영되었다. ACE는 1과 11로 계산할 수 있는 특수한 카드이기 때문에 value값에 null을 넣어주어 계산하는 로직에서 처리하기로 했다.
🦾 Step1 Refactoring
함수명
addCardToPlayer() 함수의 반환 값이 Boolean이면 이 값이 어떤 것을 의미하는지 다른 개발자들은 한 번에 알아볼 수 없을 것 같아요.
private fun addCardToPlayer(player: Player): Boolean {
if (InputView.getDecision(player)) {
player.cardBunch.addCard(cardDeck.drawCard())
OutputView.printPlayerCards(player)
return true
}
OutputView.printPlayerCards(player)
return false
}
플레이어는 카드를 더 받지 않겠다고 말하기 전까지 계속 카드를 받을 수 있다. 컨트롤러에 플레이어에게 카드를 추가하는 함수를 작성했는데 while문에서 조건으로 사용하기 위해 성공한경우 true를, 실패한 경우 false를 반환하도록 작성했다. 그래서 Boolean값이 어떤 의미를 가지는지 더 쉽게 알아볼 수 있도록 함수명을 isSuccessAddToCardToPlayer()로 변경했다. (와 다시쓰고보니 정말 별론데?)
도메인1
class CardBunch private constructor(cards: MutableSet<Card>) {
private val _cards: MutableSet<Card> = cards
val cards: Set<Card> get() = _cards.toSet()
...
방어적 복사는 잘 진행했다고 말씀해주셨다.🙃 하지만 블랙잭 게임은 여러 벌의 카드를 가지고 게임을 진행할 수 있기 때문에 Set으로 관리하면 문제가 발생할 수 있어 리스트로 수정했다.
도메인 지식이 부족해서 발생한 문제다.
CardBunch 생성 조건
CardBunch이라는 단어만 보고, 다른 개발자들이 한 번에 알 수 있는 생성 조건일까요?
class CardBunch private constructor(cards: MutableSet<Card>) {
private val _cards: MutableSet<Card> = cards
val cards: Set<Card> get() = _cards.toSet()
constructor(vararg cards: Card) : this(cards.toMutableSet()) {
validateSize()
}
CardBunch(카드 묶음) 클래스는 카드를 관리하는 일급 컬렉션으로 설계했다. 게임을 시작하면 모든 사람이 2장의 카드를 가지고 시작하기 때문에 CardBunch를 생성하기 위해서는 최소한 2장의 카드가 필요하다 라는 생각을 해서 위처럼 코드를 작성했다. 곰곰이 고민해본 결과 굳이 필요한 생성 조건인가?라는 생각이 들었고, 2장임을 검사하지 않도록 수정했다.
중복 카드 검사
중복 카드를 사이즈로 비교하는 게 매우 어색합니다. contains를 활용해보세요.
fun addCard(card: Card) {
val originSize = _cards.size
_cards.add(card)
require(originSize != _cards.size) { DUPLICATE_ERROR }
}
페어랑 같이 짠 코드인데 어떻게 둘 다 이 부분을 오케이하고 넘어갔는지 의문이다.
fun addCard(card: Card) {
_cards.add(card)
require(_cards.contains(card)) { ADD_ERROR }
}
contains를 활용하여 중복카드를 검사하도록 수정했다.(게다가 사실은 여러 벌의 카드를 사용할 수 있기 때문에 중복 검사를 할 필요도 없다.)
ACE 값
ACE의 값은 왜 null인가요? 일반적으로 ACE카드를 봤을 때 우리는 어떤 숫자라고 생각하고 있을까요?
enum class CardNumber(val value: Int?) {
ACE(null),
TWO(2),
...
이 부분에 대해 페어와 이야기를 많이 나누었는데, 다른 카드와는 다르게 ACE는 1과 11로 사용이 가능하고, 특정 값으로 고정시키기보다 null로 처리한 뒤 점수를 계산하는 로직에서 상황에 맞게 처리하자는 의견으로 좁혀져 이와같이 작성했었다. 사실 나도 null로 처리해서 얻을 수 있는 이점이 무엇이 있는가는 이해하지 못했지만 페어의 의견을 적극 반영해보았다.
일반적으로 ACE는 1로 사용하기 때문에 value에 1을 넣어주고 점수를 계산하는 로직에서 상황에 맞게 처리하도록 수정했다.
"점수를 계산하는 부분에서 상황에 맞게 처리하자"는 의견에는 동의하지만 CardNumber 내부 값이 null인지 아닌지 비교하기보다는, CardNumber자체가 ACE인지 비교하는 것이 어떻냐는 리뷰어님의 추가 의견이 있어 좋은 방법인 것 같아 적극 반영하여 수정했다.
Domain과 View 분리
해당 메서드는 View에서만 사용하네요. 도메인과 뷰의 로직을 분리해주세요.
fun valueOf(cardNumber: CardNumber): String {
return when (cardNumber) {
ACE -> "A"
TWO, THREE, FOUR, FIVE, SIX, SEVEN, EIGHT, NINE, TEN -> cardNumber.value.toString()
JACK -> "J"
QUEEN -> "Q"
KING -> "K"
}
}
Enum class인 CardNumber안에 위 메서드가 있었다. 우리는 CardNumber가 값을 가지고 있기 때문에 메시지를 던져 내부적으로 처리한 뒤 결과를 돌려받는 방식으로 작성했다고 생각했다. 하지만 실제로 View에서만 사용하고 있었고, 이 로직은 결국 도메인에 있을 필요가 없는 로직이 되어 OutputView로 이동하였다.
승 패 무 라는 문자열은 도메인 영역일지 고민해볼까요?
enum class CardNumber(val value: Int) {
ACE(1),
TWO(2),
THREE(3)
}
enum class Shape(val korean: String) {
HEART("하트"),
SPADE("스페이드"),
CLOVER("클로버"),
DIAMOND("다이아몬드")
}
enum class Consequence(val value: String) {
WIN("승"), LOSE("패"), DRAW("무")
}
현재 코드에서 enum class는 총 3개다. 가장 먼저 설계한 CardNumber는 "총 합계 계산"이라는 도메인 로직에서 값들을 사용하기 때문에 value를 가지고 있는 것이 당연하다!라고 생각했다. 뒤이어 설계한 Shape, Consequence도 당연히! value를 가지고 있으면 출력할 때 사용하면 되겠지? 라고 생각하며 코드를 작성했다.
이 리뷰를 받고나서야 Shape, Consequence이 가지고 있는 value는 도메인 로직이 아닌 뷰로직에서만 사용한다는 것을 알 수 있었고, OutputView에서 각 값에 맞도록 변경하여 사용하도록 수정했다.
사이드 이펙트
이 역할은 정말 이 객체의 역할일 지 고민해보세요. 또, 사이드이펙트를 발생시키는 메서드의 작성은 최대한 지양해보세요.
class Player(val name: String, override val cardBunch: CardBunch) : Participant {
lateinit var consequence: Consequence
fun chooseWinner(score: Int) {
val playerScore = cardBunch.getTotalScore()
consequence = if (playerScore > MAX_SCORE_CONDITION) {
Consequence.LOSE
} else if (playerScore > score) {
Consequence.WIN
} else if (playerScore == score) {
Consequence.DRAW
} else if (score > MAX_SCORE_CONDITION) {
Consequence.WIN
} else {
Consequence.LOSE
}
}
Dealer의 compareScore() 함수에서 Player를 인자로 받는다. 내부적으로는 Player의 chooseWinner()를 호출하면서 Dealer 카드의 score를 넘겨주고, Player가 승패의 결과를 가지고 있는 상태다.
이렇게 되면 Player가 갖고 있는 승패에 대한 정보를 chooseWinner() 함수가 실행될 때마다 바뀌는 사이드이펙트가 발생한다.
class Referee {
private val _gameResult = mutableMapOf<String, Consequence>()
val gameResult: Map<String, Consequence>
get() = _gameResult.toMap()
fun chooseWinner(dealer: Dealer, player: Player) {
val dealerScore = dealer.cardBunch.getTotalScore()
val playerScore = player.cardBunch.getTotalScore()
val gameConsequence = getPlayerConsequence(dealerScore, playerScore)
_gameResult[player.name] = gameConsequence
}
...
게임의 승패를 계산하는 코드가 Player에 있는 것이 어색하다고 느껴졌다. 그래서 게임의 승패를 관리하는 Referee클래스를 하나 더 만들고, 승패를 계산하여 Map<String, Consequence>를 가지고 있는 방식으로 수정했다.
Referee클래스가 갖고있는 상태 값을 chooseWinner() 함수에서 변경해주기 때문에 현재 코드에서는 여전히 사이드이펙트가 발생하고 있다. 사이드 이펙트를 제거하기 위해 함수 안에서 Map<String, Consequence>를 생성하고 값을 추가한 뒤 이 결과를 반환하는 방식으로 수정했다. (처음에는 이 방식도 사이드 이펙트가 발생하는 것이 아닌가 했었는데, 다른 크루들의 도움으로 아니라는 것을 깨달았다.)
class Dealer(override val cardBunch: CardBunch) : Participant {
override fun canGetCard(): Boolean = cardBunch.getSumOfCards() < ADD_CARD_CONDITION
override fun getScore(): Int = cardBunch.getSumOfCards()
private fun versusPlayer(player: Player): Consequence {
val dealerScore = cardBunch.getSumOfCards()
val playerScore = player.getScore()
return getPlayerConsequence(dealerScore, playerScore)
}
fun versusPlayers(players: List<Player>): Map<String, Consequence> {
val gameResult = mutableMapOf<String, Consequence>()
players.forEach { player ->
gameResult[player.name] = versusPlayer(player)
}
return gameResult
}
private fun getPlayerConsequence(dealerScore: Int, playerScore: Int): Consequence {
if (dealerScore > MAX_SCORE_CONDITION) return Consequence.WIN
if (playerScore > MAX_SCORE_CONDITION) return Consequence.LOSE
if (dealerScore == MAX_SCORE_CONDITION && playerScore != MAX_SCORE_CONDITION) return Consequence.LOSE
if (playerScore > dealerScore) return Consequence.WIN
if (playerScore == dealerScore) return Consequence.DRAW
return Consequence.LOSE
}
}
굳이 상태값을 갖고있는 Referee클래스가 필요하지 않기 때문에, 승리 계산 로직을 Dealer로 옮겼다. versusPlayers() 함수의 결과로 Map<String, Consequence>를 반환해주도록 했다.
승리 결과를 어떻게 저장할지에 대해 고민을 많이했다. 플레이어에게 직접 저장도 했었고, Referee 클래스를 만들어 저장하는 과정을 거치고 다시 딜러로 옮겨와 결과를 반환하는 함수로 만들었다. 이게 그렇게 어려운 내용이야?라고 보는 사람도 있을 수 있지만 당시의 나는 승패 로직을 어디에 두어야 하는지 갈피를 못잡고 어려워했었다. 그리고 리뷰를 받고 리팩토링을 진행하면서 현재 코드에서 어떤 문제가 발생할 수 있는지, 왜 고쳐야하는지에 대해 꽤나 많은 시간투자를 했고, 그만큼 성장했다고 생각한다.
카드 캐싱
Shape와 Card를 랜덤으로 생성할 필요가 있을까요?
해당 객체를 랜덤으로 생성했기에 필요하지 않는 로직들이 생겨나진 않았을까요?
정말로 랜덤이 필요한 곳은 어디일까요?
class CardDeck(
private val shapeGenerator: ShapeGenerator,
private val cardNumberGenerator: CardNumberGenerator
){
private lateinit var shape: Shape
fun drawCard(): Card {
val cardNumber: CardNumber = getCardNumber()
return Card(shape, cardNumber)
}
private fun changeShape() {
shape = shapeGenerator.pickShape()
}
private fun getCardNumber(): CardNumber {
changeShape()
return cardNumberGenerator.getCardNumber(shape) ?: getCardNumber()
}
}
PR을 보낼때도 작성했던 질문이기도 했다. "CardDeck에서 shape을 상태 값으로 갖지 않으면 CardNumber값이 null이 나와 다시 뽑고 싶을 때 인자로 들어오는 shape을 바꿔줄 수 있는 좋은 방법이 생각나지 않는다. 이를 개선할 방법이 있을까요?"
Step1에서는 이 리뷰 반영이 가장 어려웠던 것 같다. 랜덤한 Shape을 뽑고, Shape에 대한 랜덤한 숫자를 뽑아 카드를 만들어야 한다는 생각을 바꾸지 못해서 그랬던 것 같다.
data class Card(val shape: Shape, val cardNumber: CardNumber) {
init {
require(shape in Shape.values()) { SHAPE_ERROR }
require(cardNumber in CardNumber.values()) { NUMBER_ERROR }
}
companion object {
private val CARDS: List<Card> = Shape.values().flatMap { shape ->
CardNumber.all().map { number -> Card(shape, number) }
}
fun getAllCards(): MutableList<Card> = CARDS.toMutableList()
const val SHAPE_ERROR = "모양은 하트, 다이아, 클로버, 스페이드 중 하나여야 합니다."
const val NUMBER_ERROR = "숫자는 2부터 10까지, ACE, JACK, QUEEN, KING만 가능합니다."
}
}
Card 클래스에 미리 52개의 인스턴스를 만들어두고 재사용할 수 있도록 수정했다. 이렇게 만들면 기존에 있던 모양과 숫자를 랜덤으로 생성하는 클래스들은 필요하지 않게된다. Card 클래스의 getAllCards()함수를 이용하여 모든 카드를 가져올 수 있도록 수정했다.
class CardDeck {
private val cards = Card.getAllCards().shuffled().toMutableList()
fun drawCard(): Card {
return cards.removeFirstOrNull() ?: throw IllegalArgumentException()
}
}
CardDeck이 카드를 랜덤으로 갖고 있고, 카드를 뽑는 함수 drawCard()을 통해 한 장의 카드를 반환 받을 수 있도록 수정했다.
현재 코드에서는 CardDeck이 카드를 랜덤으로 가지고 있기 때문에 랜덤에 대한 테스트가 불가능한 구조다.
class CardDeck(private val cards: MutableList<Card>) {
fun drawCard(): Card {
return cards.removeFirstOrNull() ?: throw IllegalArgumentException()
}
...
CardDeck을 생성할 때 생성자를 통해 어떤 카드를 가지고 덱을 구성할 것인지 결정할 수 있도록 수정했고, 테스트에서 한 장의 카드만 넣고 drawCard()를 실행했을 때 넣어준 카드가 나왔는지 확인하였다.
리뷰어님이 남겨주신 "개발자가 제어할 수 있는 환경으로 만들어라."라는 말이 인상깊었다.
ACE 체크
ACE가 4장 있더라도, ACE의 점수 체크는 1번만 하면 됩니다.(카드 목록에 ACE의 유무 검사)
애초에 ACE가 2장이면, 22점이 아닌, 12점이기 때문이에요. 물론 2점으로도 사용할 수 있겠지요.
그 점을 고려해서 로직을 작성해볼까요?
class CardBunch private constructor(cards: MutableList<Card>) {
private val _cards: MutableList<Card> = cards
val cards: List<Card> get() = _cards.toList()
constructor(vararg cards: Card) : this(cards.toMutableList())
fun addCard(card: Card) {
_cards.add(card)
}
fun getTotalScore(): Int {
var result = 0
val sortedCards = cards.sortedBy { it.cardNumber.value }.reversed()
sortedCards.forEach { card ->
result += if (card.cardNumber == CardNumber.ACE) checkAceValue(result) else card.cardNumber.value
}
return result
}
private fun checkAceValue(result: Int): Int {
return when (result + BIG_ACE_SCORE > MAX_SCORE_CONDITION) {
true -> SMALL_ACE_SCORE
false -> BIG_ACE_SCORE
}
}
fun isBurst(): Boolean = getTotalScore() > MAX_SCORE_CONDITION
companion object {
private const val ADD_ERROR = "카드가 추가되지 않았습니다!"
private const val BIG_ACE_SCORE = 11
private const val SMALL_ACE_SCORE = 1
}
}
ACE가 여러 장 있어도 한 장만 11로 계산할 수 있다는 것을 이 리뷰를 받고 깨달았다. 그러면 복잡하게 작성했던 로직이 깔끔해질 수 있겠구나는 생각도 들었다.
class CardBunch private constructor(cards: MutableList<Card>) {
private val _cards: MutableList<Card> = cards
val cards: List<Card> get() = _cards.toList()
constructor(vararg cards: Card) : this(cards.toMutableList())
fun addCard(card: Card) {
_cards.add(card)
}
fun getTotalScore(): Int {
val sum = cards.sumOf { it.cardNumber.value }
return if (containAce()) decideAddTen(sum) else sum
}
private fun containAce(): Boolean {
return _cards.any { card -> card.cardNumber == CardNumber.ACE }
}
private fun decideAddTen(sum: Int): Int {
return when (sum + TEN > MAX_SCORE_CONDITION) {
true -> sum
false -> sum + TEN
}
}
fun isBurst(): Boolean = getTotalScore() > MAX_SCORE_CONDITION
companion object {
private const val TEN = 10
}
}
ACE의 값은 1로 고정하여 계산하도록 수정했다. 만약 ACE를 가지고 있는 경우 총점에 10을 더해줘도 21을 넘지 않는지 확인하고 넘지 않는 경우에만 10을 더해주도록 수정했다.
ACE를 가지고 있는지 체크를 위해 .any{ }를, 합계를 구하기 위해 .sum{ }을 활용했고, return if ~, return when~ 을 사용하여 로직을 깔끔하게 수정했다.
Step2
Step2에서는 "베팅" 요구사항이 추가되었다.
- 플레이어는 게임을 시작할 때 베팅 금액을 정해야 한다.
- 카드를 추가로 뽑아 21을 초과할 경우 베팅 금액을 모두 잃게 된다.
- 처음 두 장의 카드 합이 21일 경우 블랙잭이 되며 베팅 금액의 1.5배를 딜러에게 받는다. 딜러와 플레이어가 모두 동시에 블랙잭인 경우 플레이어는 베팅한 금액을 돌려받는다.
- 딜러는 처음에 받은 2장의 합계가 16이하이면 반드시 1장의 카드를 추가로 받아야 하고, 17점 이상이면 추가로 받을 수 없다. 딜러가 21을 초과하면 그 시점까지 남아 있던 플레이어들은 가지고 있는 패에 상관없이 승리해 베팅 금액을 받는다.
베팅머니를 불변 객체로 만들었다.
🦿 Step2 Refactoring
multiply operator
1~10000개의 곱셈이 생긴다면 1만 개의 메서드가 생겨야할까요?
BettingMoney를 불변 객체로 만들었으니, BettingMoney자체를 곱셈할 수 있게, multiply operator를 오버라이딩 해보는 것을 도전해보세요.
class BettingMoney(private val money: Int) {
init {
require(money > MIN_MONEY) { ERROR_MONEY }
}
fun multipleTwoPointFive(): Int = (money * 2.5).toInt()
fun multipleOne(): Int = money
fun multipleTwo(): Int = money * 2
}
class BettingMoney(private val money: Int) {
init {
require(money > MIN_MONEY) { ERROR_MONEY }
}
operator fun times(d: Double): Int = (money * d).toInt()
}
times를 통해 곱하기 연산자를 오버라이딩 할 수 있다.
BettingMoney(1000) * 2.0
그래서 이렇게 코드를 작성하면 BettingMoney에는 2000이라는 값이 들어가 있을 것이다. 이외에도 여러 연산자를 오버라이딩 할 수 있고 어떤 함수를 쓰는지 몇 개만 보자.
- a + b : a.plus(b)
- a - b : a.minus(b)
- a * b : a.times(b)
- a / b : a.div(b)
확장함수 체이닝
calculateDividend() 함수를 확장 함수 체이닝으로 변경을 도전해보세요!
class Calculator {
private fun getDividend(player: Player, consequence: Consequence): Int {
return when (consequence) {
Consequence.WIN -> if (player.isBlackjack()) player.bettingMoney.times(MULTI_BLACKJACK)
else player.bettingMoney.times(MULTI_WIN)
Consequence.LOSE -> player.bettingMoney.times(MULTI_LOSE)
Consequence.DRAW -> player.bettingMoney.times(MULTI_DRAW)
fun calculateDividend(playerResultsBy: Map<Player, Consequence>): Map<Player, Int> {
val result = mutableMapOf<Player, Int>()
playerResultsBy.forEach {
val dividend = getDividend(it.key, it.value)
result[it.key] = it.key.getProfit(dividend)
}
return result
}
companion object {
private const val MULTI_BLACKJACK = 2.5
private const val MULTI_WIN = 2.0
private const val MULTI_LOSE = 0.0
private const val MULTI_DRAW = 1.0
}
}
fun calculateDividend(playerResultsBy: Map<Player, Consequence>): Map<Player, Int> {
return playerResultsBy.asSequence().associate {
it.key to it.key.getProfit(getDividend(it.key, it.value))
}
}
이 함수의 반환 값은 Map 형태이다. Map을 만들기 위해 associate{ }를 사용해야 하고, associate{ }를 사용하기 위해서는 Map을 Sequence로 바꿔줘야 했다. 그리고 {} 안에서는 중위 to를 통해 Pair로 만들어줌으로써 로직을 더 간단하게 표현하도록 수정했다.
코틀린은 신기하고 대단한 언어다.
CardDeck
class CardDeck(private val cards: MutableList<Card>) {
...
아래 코드를 실행시키면 어떤 일이 발생할 수 있을까요?
val mutableList = mutableListOf()
val cardDeck = CardDeck(mutableList)
mutableList.clear()
테스트 코드에 작성해서 확인해본 결과 신기하게도 카드덱이 가지고 있는 카드도 삭제되는 것을 확인했다.
class CardDeck(private val _cards: List<Card>) {
private val cards = _cards.toMutableList()
...
CardDeck이 생성될 때 인자로 받은 리스트를 기반으로 backing property를 다시 생성해서 갖도록 수정했다. 그리고 내부 로직에서는 backing property를 사용하도록 수정했다.
이 부분은 혼자서는 체크할 수 없었던 부분인 것 같다.
Object
calculator가 내부에 상태를 지니지 않으니, object로 만들어도 좋겠어요
class Calculator {
private fun getDividend(player: Player, consequence: Consequence): Int {
return when (consequence) {
Consequence.WIN -> if (player.isBlackjack()) player.bettingMoney * MULTI_BLACKJACK else player.bettingMoney * MULTI_WIN
Consequence.LOSE -> player.bettingMoney * MULTI_LOSE
Consequence.DRAW -> player.bettingMoney * MULTI_DRAW
}
}
fun calculateDividend(playerResultsBy: Map<Player, Consequence>): Map<Player, Int> {
return playerResultsBy.asSequence().associate {
it.key to it.key.getProfit(getDividend(it.key, it.value))
}
}
companion object {
private const val MULTI_BLACKJACK = 2.5
private const val MULTI_WIN = 2.0
private const val MULTI_LOSE = 0.0
private const val MULTI_DRAW = 1.0
}
}
실제로 Calculator는 계산하는 함수만 가지고 있고, 상태값을 가지고 있지 않다. 지금까지 미션을 진행하면서 무엇을 class로, 무엇을 object로 만들어야 하는지 감이 오지 않았었는데, 이번 리뷰를 통해 class와 object를 구분할 수 있는 하나의 기준이 생긴 것 같다.
에러 메시지 검증
어떤 에러 메시지가 나타났는지도 검증해보세요
@Test
fun `카드덱은 외부에서 변경이 불가능하다`() {
// given
val mutableList = mutableListOf(
Card.get(Shape.HEART, CardNumber.ACE),
Card.get(Shape.HEART, CardNumber.TWO)
)
val cardDeck = CardDeck(mutableList)
mutableList.clear()
// when
repeat(2) { cardDeck.drawCard() }
// then
assertThrows<IllegalArgumentException> { cardDeck.drawCard() }
}
지금까지 init{ } 안에 있는 require에 걸리는 조건을 주었을 때 assertThrows를 통해 에러가 발생하는지만 확인했다. 여기서 생긴 예외가 내가 예상한 에러가 아닐수도 있고, 내가 예상하지 못한 에러 메시지가 나올 수도 있구나 하는 생각이 들었다. 에러 메시지를 검증하는 것도 좋은 테스트 방법인 것 같다.
@Test
fun `카드덱은 외부에서 변경이 불가능하다`() {
// given
val mutableList = mutableListOf(
Card.get(Shape.HEART, CardNumber.ACE),
Card.get(Shape.HEART, CardNumber.TWO)
)
val cardDeck = CardDeck(mutableList)
mutableList.clear()
// when
repeat(2) { cardDeck.drawCard() }
val exception = assertThrows<IllegalArgumentException> { cardDeck.drawCard() }
// then
assertThat(exception.message).isEqualTo("더이상 뽑을 카드가 없습니다.")
}
assertThrows<IllegalArgumentException>의 결과는 IllegalArgumentException이고, assertThrows<IllegalStateException>의 결과는 IllegalStateException이다. 에러의 메시지와 init{ } 안에 작성한 메시지가 같은지 확인하도록 수정했다.
회고
많이 배울 수 있어서 재밌었지만 머지될 때 약간의 아쉬움이 있었다. 코드가 많이 좋아져서 머지가 되었다기보다는 시간이 지나서 머지된 느낌?
블랙잭 Step1 PR
[크롱] 블랙잭 1단계 미션 제출합니다. by krrong · Pull Request #32 · woowacourse/kotlin-blackjack
안녕하세요 리뷰어님! 시간이 충분하지 않아서 코드를 잘 작성하지 못한 것 같습니다.. 제가 봐도 리팩토링이 필요한 코드가 많아보여요..😢 미션 진행하면서 몇 가지 궁금한 점이 생겼습니다.
github.com
블랙잭 Step2 PR
[크롱] 2단계 블랙잭 제출합니다. by krrong · Pull Request #46 · woowacourse/kotlin-blackjack
step1을 머지해주시면서 남겨주셨던 코멘트들도 최대한 반영하려고 노력했습니다! 2단계도 잘 부탁드릴게요 🙃
github.com