[수달] 2단계 블랙잭 제출합니다. #58
Conversation
- 블랙잭 상태 연습
- Deck의 생성자에서 불필요한 Mutable이 아닌 Immutable로 수정 - Deck과 Cards에서 리스트로 받아 생성하는 것이 아닌 가변인자로 받아 생성할 수 있도록 함 - Card에서 카드를 받지 않고 숫자와 모양의 쌍을 받아 생성하도록 변경 - Card에서 기본 생성자를 숨기고 from을 통해 동일한 카드 내용물에 대해 여러 주소를 가진 인스턴스가 생성되지 않도록 함
- getOrElse를 이용해 null을 받을 시 재입력을 받는 방식에서 getOrElse를 통해 불필요한 과정을 줄임
- 인스턴스화를 할 때 생성자에 카드들이 있어 혼동을 줄 수 있기에 private으로 변경 - 하나의 덱이 아닌 블랙잭에서 여러 덱을 가르키는 용어인 MultiDeck이라는 이름을 사용 - 백킹 프로퍼티인 _cards를 val로 변경
- 첫 턴에만 A를 11로 보는 일부 카지노에서만 적용되는 soft 규칙 대신에 A를 11로 계산하였을 때 21이 넘지 않는 선에서 A를 11로 계산하는 대중적인 soft 규칙 적용
- 게임 참여자와 딜러의 공통 부분을 묶기 위해 생성 - decideGameResult를 추상 클래스로 옮김
- 게임 결과를 뒤집는 함수가 호출되지 않고 있었음
There was a problem hiding this comment.
2단계 미션 고생 많으셨습니다.
여러가지 시도와 노력을 하신 모습이 보기 좋았습니다.
자동차 경주, 로또 미션에 이어서 블랙잭 미션까지 오면서 객체지향 관점의 여러가지 피드백들을 다시 되돌아보는 것도 좋을 것 같습니다.
(메시지를 보내라, 스스로 하도록 만들어라, 함수를 만들어라 등)
또한 객체지향에서 사이드 이펙트라고 부르는 개념에 대해 시간되실 때 한번 학습 해보시면 성장에 여러가지 도움이 되실 것 같아요.
남겨드린 피드백은 충분히 고민 해보시고 도전해보세요.
| interface State { | ||
|
|
||
| fun profit(bet: Double): Double | ||
|
|
||
| fun stay(): State | ||
| } |
| private val countBy: MutableMap<GameResult, Int> = | ||
| GameResult.values().associateWith { 0 }.toMutableMap() |
There was a problem hiding this comment.
countBy는 외부에서 값을 주입하는 일이 없으므로 생성자에서 받지 않고 내부 변수로 바꾸었습니다!!
| fun count(gameResult: GameResult) { | ||
| countBy[gameResult] = | ||
| countBy[gameResult]?.plus(1) ?: throw IllegalArgumentException() | ||
| } |
There was a problem hiding this comment.
키값이 없는 경우는 논리적인 오류로 보이지 않습니다.
예외를 발생시키지 않도록 만들어주세요
| fun getResult(): GameResult = | ||
| countBy.filter { it.value == 1 }.keys.first() |
There was a problem hiding this comment.
getResult()가 무엇을 반환하는 것인지 사용하는 측이 알 수 있을까요?
| enum class GameResult(val ratio: Float) { | ||
| WIN(1.0f), | ||
| BLACKJACK(0.5f), | ||
| DRAW(0.0f), | ||
| LOSE(-1.0f), |
There was a problem hiding this comment.
수익금에 관한 배율은, 단순히 자신의 결과로 결정이 되지 않습니다.
가령, 플레이어가 블랙잭일때
상대의 상태에 따라 이길수도있고 비길수도있고 블랙잭일수도있습니다.
즉, 게임의 결과 != 배율이라고 정의할 수 없다는 의미라고 생각됩니다.
There was a problem hiding this comment.
자신이 블랙잭임에도 상대 또한 블랙잭일 경우는 결국 무승부라고 생각하여 이 경우에는 무승부라는 GameResult를 가지도록 하였습니다!
이와 관련된 기능목록과 테스트케이스를 추가해두었습니다!!
| fun decideGameResult(otherPlayer: Player) { | ||
| val otherPlayerCardsSum: Int = otherPlayer.cards.sum() | ||
| val cardsSum = cards.sum() | ||
|
|
||
| val gameResult = when { | ||
| cardsSum > MAX_SUM_NUMBER -> GameResult.LOSE | ||
| otherPlayerCardsSum > MAX_SUM_NUMBER -> GameResult.WIN | ||
| otherPlayerCardsSum > cardsSum -> GameResult.LOSE | ||
| otherPlayerCardsSum == cardsSum -> GameResult.DRAW | ||
| isBlackjack() -> GameResult.BLACKJACK | ||
| else -> GameResult.WIN | ||
| } | ||
|
|
||
| matchResult.count(gameResult) |
There was a problem hiding this comment.
같은 의견을 반복해서 드리는 것 같습니다.
위 함수에서 MatchResult를 반환하지 않고, 참가자가 계속해서 가지고 있는 상태로 만든 이유는 무엇인가요?
카드를 3장 가지고있는상태로 결과를 만들고
또다시 카드를 받으면 이전 결과는 데이터가 현재와 일치하지 않는 데이터 오염이 되지는 않을까요?
| private fun isBlackjack() = | ||
| (cards.size == CARD_SETTING_COUNT) && (cards.sum() == MAX_SUM_NUMBER) |
| printFirstTurnSettingCards: (Dealer, Participants) -> Unit, | ||
| ) { | ||
| dealer.setFirstTurnCards(deck) | ||
| participants.values.forEach { it.setFirstTurnCards(deck) } |
There was a problem hiding this comment.
마찬가지로 participants.values를 꺼내서 사용하지 않고 participants가 스스로 하도록 만들어주세요. 다른 코드도 모두 동일한 의견입니다
| fun printGameResult( | ||
| printSumResult: (Dealer, Participants) -> Unit, | ||
| printPlayersResults: (Dealer, Participants) -> Unit | ||
| ) { | ||
| printSumResult(dealer, participants) | ||
| printPlayersResults(dealer, participants) | ||
| } |
There was a problem hiding this comment.
도메인 모델에서는 read, print와 같은 UI와 관련된 이름이 포함되면 안됩니다.
파라미터, 함수이름 모두 동일합니다
| val blackjackGame = BlackjackGame(participants = inputView.readParticipants()) | ||
| blackjackGame.bettingParticipants(inputView::readParticipantBattingAmount) | ||
| blackjackGame.setFirstTurnPlayersCards(outputView::printFirstTurnSettingCards) | ||
| blackjackGame.hitPlayersCards( | ||
| inputView::readHitOrNot, | ||
| outputView::printPlayerCards, | ||
| outputView::printDealerHitOrNotMessage | ||
| ) | ||
| blackjackGame.decidePlayersResult() | ||
| blackjackGame.printGameResult(outputView::printSumResult, outputView::printPlayersResults) |
There was a problem hiding this comment.
아래 내용은 참고만 해주세요.
변경은 하지 않으셔도 됩니다.
블랙잭 게임 도메인 모델을 만들어 게임의 로직을 만들어 보신 시도가 좋았습니다.
다만, 지금 구조로 보면 단순히 컨트롤러의 이름이 BlackGame으로 바뀐 것 정도로만 느껴집니다.
예를 들어 블랙잭 게임을 한다면 "게임 시작", "게임 결과" 라는 두가지 관점의 개념이 있습니다.
하지만 구현한 블랙잭 게임에서는 아래 모든 코드를 호출해야 한다는 것을 외부에서 반드시 알아야만 합니다.
blackjackGame.bettingParticipants()
blackjackGame.setFirstTurnPlayersCards()
blackjackGame.hitPlayersCards()
...블랙잭 게임을 제어하는 하나의 도메인이라면 이런 구조를 생각해볼 수 있겠습니다
class BlackjackGame(...) {
fun start(): BlackjackGameResult { ... }
}
코드리뷰 규칙에 따라서, 이후로 요청주신 내용은 추가적인 확인이 어렵습니다. 이 점 참고 부탁드립니다. |
- 뷰의 메서드를 이동시켜 controller와 역할이 다를 것이 없다고 판단.
- 외부에서 변경이 불가능하도록 생성시 값을 받도록 함. - 읽기전용 값으로 변경
- 외부에서 생성해 사용하도록 변경 - MatchResult에서는 값을 count하기 때문에 여러번 결과를 계산할 경우 원하지 않는 결과가 나올 수 있음
페로로님! 1단계에서 코드 자세하게 봐주시고 자세한 피드백 남겨주셔서 코드를 더 많이 들여다볼 수 있어 좋았습니다!! 감사합니다!!
배려해주시고 좀 더 자세하게 설명해 주셔서 이번에는 이전 보다 최대한 피드백을 반영할 수 있던 것 같습니다!!
이번에도 조금 급하게 하느라 코드가 엉망입니다...
고칠 부분이 많아서 걱정이기도 하고 ,바쁘신 와중에 일거리를 만들어드리는 것 같아 죄송하지만 수업에서 배운 내용인 상태를 클래스로 나타내는 패턴을 적용해보고 싶습니다...
번거로우시겠지만 이후 피드백 반영과 함께 코드를 조금 많이 바꾸어도 괜찮을까요...?