[Step2] 🚀 2단계 - 블랙잭 #656

dlwnsgus777 · 2023-11-24T10:28:19Z

안녕하세요 리뷰어님!! 잘부탁드립니다! 🙏

참고로 이번 코틀린 클린코드 With TDD에서의 목표는 다음과 같습니다! 👍

코틀린 문법 익숙해지기(코틀린 답게)
유지보수 용이하고(객제지향적인) 테스트하기 좋은 코드에 대해 감잡기

고민한 부분

나름대로 고민을 하긴 했지만 객체들의 협력에 대해서 의존관계를 설정하는데 감을 잡지 못해 많이 미흡했던 것 같습니다!!

wisemuji

안녕하세요 준현님!

구현을 잘 해주셨습니다 💯 깔끔한 구현이 좋았습니다
고민해보시면 좋을만한 코멘트를 달았으니 확인해주시면 감사하겠습니다.

궁금한 점이나 논의하고 싶은 내용이 있으면 언제든 DM 주세요! 🙏 💪

wisemuji · 2023-11-25T07:30:12Z

src/main/kotlin/blackjack/GameManager.kt

+        var score = scoreCalculator.calcScore(it.cards)
+        var drawAmount = -1
+
+        while (score <= 21 && drawAmount != 0) {


GameManager는 controller 역할을 하는 객체라고 이해했습니다

이 로직이 controller에 있는게 적절할까요?
controller 로직이 프로그램이 커지면 커질수록 계속 늘어나는게 적절할까요?
적절한 객체의 책임으로 위임할 수 있는 controller 로직이 있나요?

이 코멘트를 반영하며 Controller의 책임을 분산시키는걸 이반 단계 핵심 목표로 삼아보시죠 ㅎㅎ

wisemuji · 2023-11-25T07:31:54Z

src/main/kotlin/blackjack/card/CardPattern.kt

+package blackjack.card
+
+enum class CardPattern(val patternName: String) {
+    DIAMOND("다이아"),


도메인 객체에 뷰 요구사항이 포함되는게 적절할까요? 자동차 경주 5단계 요구사항을 참고하여 도메인 로직과 뷰 로직을 명확하게 분리하고, 의존 방향을 정리하면 어떨까요?

wisemuji · 2023-11-25T07:34:00Z

src/main/kotlin/blackjack/GameManager.kt

+
+    private fun joinPlayers() {
+        val playerNames: List<String> = inputManager.inputPlayerNames()
+        this.players = playerNames.map(::Player)


players가 다시 초기화될 필요가 없다면 객체 생성 시 초기화할 수 있도록 바꿔보면 어떨까요?

lateinit var을 사용할 때에는 꼭 variable로 선언될 필요가 있을지 고민해보면 좋겠습니다.

불변함은 코틀린 언어의 가장 큰 장점 중 하나라고도 하는데요, 이러한 장점을 온전히 누리기 위해 전 코틀린에서는 가능하면 var이나 Mutable 타입 사용을 지양하려고 하고 있습니다. 🙂

wisemuji · 2023-11-25T07:35:57Z

src/main/kotlin/blackjack/ScoreCalculator.kt

+
+        card.filter { it is AceCard }
+            .forEach { _ ->
+                score = calcScore(score)


Kotlin Collection의 fold 등을 활용해보면 어떨까요?

wisemuji · 2023-11-25T07:37:34Z

src/main/kotlin/blackjack/ScoreCalculator.kt

+        return score
+    }
+
+    private fun calcScore(score: Int): Int {


어떤 네이밍이 다른 개발자들도 이해하기 쉬운 네이밍일까요?

wisemuji · 2023-11-25T07:38:06Z

src/main/kotlin/blackjack/ScoreCalculator.kt

+        totalScore += when {
+            (abs(plusMin - BEST_SCORE) < abs(plusMax - BEST_SCORE)) -> MIN_ACE_SCORE
+            (abs(plusMin - BEST_SCORE) > abs(plusMax - BEST_SCORE)) -> MAX_ACE_SCORE
+            else -> MIN_ACE_SCORE


다른 개발자들도 이 로직을 보고 한눈에 이해할 수 있을까요?

wisemuji · 2023-11-25T07:38:43Z

src/main/kotlin/blackjack/card/AceCard.kt

+    val pattern: CardPattern
+) : BlackJackCard {
+    override fun toString(): String {
+        return "ace${pattern.patternName}"


도메인 객체에 뷰 요구사항이 포함되는게 적절할까요?

wisemuji · 2023-11-25T07:43:19Z

src/main/kotlin/blackjack/card/CardDeck.kt

+
+    companion object {
+        private const val EMPTY_DECK_ERROR_MESSAGE: String = "카드 덱이 모두 소진되었습니다."
+        private fun initNormalCard(list: MutableList<BlackJackCard>) {


이 함수가 Companion Object로 관리될 필요가 있을까요?

블랙잭의 카드는 매번 인스턴스화될 필요가 있을까요?

dlwnsgus777 · 2023-11-27T02:04:20Z

안녕하세요 리뷰어님!
질문드렸던 내용에 맞는 목표를 잡아주신 점이 큰 도움되었습니다!!ㅎ

확실히 controller로직과 도메인 로직을 분리하려다보니 약간은(?) 감이 잡혀가는 것 같았습니다!

이번 리뷰도 잘부탁드립니다! 🙏

고민한 부분

점수를 요청하는 부분에 대해

컨트롤러가 -> 플레이어에게 점수가 얼마인지 요청 -> 플레이어는 점수 계산 (ScoreCalculator사용)

이렇게 해서 Player가 ScoreCalculator 의존성을 가지도록 했는데 이런식의 흐름이 리뷰어님이 보셨을 때 자연스러운지 궁금합니다!!

wisemuji

안녕하세요 준현님!
리뷰 반영을 잘 해주셨습니다 💯

조금 고민해보시면 좋을 내용과 질문에 대한 답변을 코멘트로 달았으니 다음 단계 미션 진행해주시면서 같이 반영해주시면 됩니다!

다음 미션도 화이팅입니다 🙏

wisemuji · 2023-11-27T10:21:53Z

src/main/kotlin/blackjack/card/CardDeck.kt

다음 단계부터는 다음 자료를 참고하여 블랙잭 도메인에 대해 이해도를 높여보면 어떨까요?

https://www.high1.com/casino/contents.do?key=1764

https://namu.wiki/w/%EB%B8%94%EB%9E%99%EC%9E%AD(%EC%B9%B4%EB%93%9C%EA%B2%8C%EC%9E%84)

+) 특히 다음 내용은 네이밍에 활용해보셔도 좋을 것 같아요~

wisemuji · 2023-11-27T10:24:41Z

src/main/kotlin/blackjack/card/BlackJackCard.kt

@@ -0,0 +1,3 @@
+package blackjack.card
+
+sealed interface BlackJackCard


딱히 컨벤션에서 강제하지는 않지만 개인적으로 sealed interface/class 등은 한 파일에 모아두는 것이 가독성이 높다고 생각하는 편입니다! 참고만 해주세요 ㅎㅎ

sealed interface BlackJackCard { class AceCard(val pattern: CardPattern) : BlackJackCard ... }

wisemuji · 2023-11-27T11:36:05Z

src/main/kotlin/blackjack/ScoreCalculator.kt

+import blackjack.card.PictureCard
+import kotlin.math.abs
+
+class ScoreCalculator {


점수를 요청하는 부분에 대해
컨트롤러가 -> 플레이어에게 점수가 얼마인지 요청 -> 플레이어는 점수 계산 (ScoreCalculator사용)
이렇게 해서 Player가 ScoreCalculator 의존성을 가지도록 했는데 이런식의 흐름이 리뷰어님이 보셨을 때 자연스러운지 궁금합니다!!

지금 단계에서는 어색해보이진 않습니다! 블랙잭 미션의 꽃은 바로 다음 단계에서 딜러와 플레이어의 책임을 어떻게 분할하는지인데요, 다음 단계 미션을 진행하시면서 좀 더 고민하시게 되겠네요!

wisemuji · 2023-11-27T11:36:14Z

src/main/kotlin/blackjack/ScoreCalculator.kt

+class ScoreCalculator {
+    fun calcScore(card: List<BlackJackCard>): Int {
+        return card.filterIsInstance<AceCard>().fold(calcScoreToNormalAndPictureCard(card)) { acc, _ ->
+            calcAceScore(acc)


fold 활용 👍

wisemuji · 2023-11-27T11:36:58Z

src/main/kotlin/blackjack/ScoreCalculator.kt

+import kotlin.math.abs
+
+class ScoreCalculator {
+    fun calcScore(card: List<BlackJackCard>): Int {


함수 이름을 지을 때에는 약어 (calc, sub 등)를 최대한 활용하지 않는 것이 좋습니다. 통상적으로 활용되는 약어(sum, println)가 아니라면 혼란을 줄 수 있어요.

wisemuji · 2023-11-27T11:39:33Z

src/main/kotlin/blackjack/card/CardDeck.kt

+        val list = mutableListOf<BlackJackCard>()
+        initNormalCard(list)
+        initPictureCard(list)
+        initAceCard(list)
+
+        list.shuffle()


가변 빈 리스트를 만들어 이를 일일히 초기화하는 절차 지향 스타일을 활용하기보다, 불변 리스트 타입만 활용하면 어떨까요? 예를 들어 다음과 같이 함수 반환값을 만들어보는거예요.

val cards: List<BlackJackCard> = normalCard() + pictureCard() + aceCard()

tip: List에서는 shuffle()대신 shuffled()를 활용하면 됩니다.

wisemuji · 2023-11-27T11:40:07Z

src/main/kotlin/blackjack/ui/OutputManager.kt

+    private fun parsingCardPatternToString(cardPattern: CardPattern): String {
+        return when (cardPattern) {
+            CardPattern.DIAMOND -> "다이아"
+            CardPattern.CLOVER -> "클로버"
+            CardPattern.SPADE -> "스페이드"
+            CardPattern.HEART -> "하트"
+        }
+    }
+
+    private fun parsingCardPictureToString(cardPicture: CardPicture): String {
+        return when (cardPicture) {
+            CardPicture.KING -> "K"
+            CardPicture.JACK -> "J"
+            CardPicture.QUEEN -> "Q"
+        }
+    }


UI 로직 분리 👍

dlwnsgus777 and others added 24 commits November 17, 2023 22:15

doc: Dsl 과제 패키지 분리

e3bd640

doc: 요구사항 정리

d160388

feat: player는 이름을 가질 수 있다.

8265b2e

feat: card는 숫자와 무늬를 가진다

aff45de

feat: card, player main package로 이동

a7a45c9

feat: King, Queen, Jack과 같이 그림만 가지고 있는 카드 클래스 생성

c214909

feat: Ace 카드 추가

25c40ba

feat: Ace 카드 추가

ba47608

Merge remote-tracking branch 'origin/step2' into step2

70cc8ab

feat: card pattern enum 분리

04e1775

doc: README update

99fbfb6

feat: 카드 그림 enum 추가 및 카드 관련 클래스 패키지 이동

29eb4b9

feat: 카드 덱 클래스 개발

c1868ea

feat: 플레이어는 카드를 가질 수 있다.

3fba58d

feat: 플레이어 입력 기능 개발

45143ec

feat: 카드 덱 드로우 기능 개발

675cb9e

feat: 플레이어는 카드를 뽑을 수 있다

4362886

feat: 블랙잭 첫번째 Turn 기능 개발

8e4f0c3

feat: 카드 출력 기능 개발

19dc1ee

doc: 기능 요구 사항 체크

631831f

doc: 카드 점수 계산 클래스 개발

ca8ad0a

doc: READ.ME 업데이트

8d81a55

refactor: 코드 리팩터링

e8d5714

feat: 블랙잭 게임 기능 개발

7bea84b

wisemuji requested changes Nov 25, 2023

View reviewed changes

Ted1_Lee added 5 commits November 27, 2023 09:34

feat: fold를 사용하도록 리팩터링

313b89a

refactor: calcScore메서드 명 변경

cbb058a

refactor: 도메인 객체에서 뷰 요구사항 분리

2152518

refactor: var 변수를 val 변수로 변경

4ac16c4

refactor: 드로우 가능 여부 계산 책임을 player에게 위임

e32719e

Ted1_Lee added 6 commits November 27, 2023 10:18

test: test코드 추가

a3316a8

refactor: CardDeck을 object로 변경

72dff31

refactor: Player가 ScoreCalculator를 가지고 있도록 변경

fec96e3

refactor: private const val 변수 추가

a831029

refactor: ScoreCalculator 함수 리팩터링

2b419ed

refactor: UI 로직 도메인 로직과 분리

e418997

wisemuji approved these changes Nov 27, 2023

View reviewed changes

wisemuji merged commit e963d2c into next-step:dlwnsgus777 Nov 27, 2023

wisemuji mentioned this pull request Dec 1, 2023

[Step3] : 🚀 3단계 - 블랙잭(딜러) #690

Merged

wisemuji mentioned this pull request Dec 9, 2023

[Step4] : 🚀 4단계 - 블랙잭(베팅) #703

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Step2] 🚀 2단계 - 블랙잭 #656

[Step2] 🚀 2단계 - 블랙잭 #656

dlwnsgus777 commented Nov 24, 2023 •

edited

wisemuji left a comment

wisemuji Nov 25, 2023

wisemuji Nov 25, 2023

wisemuji Nov 25, 2023

wisemuji Nov 25, 2023

wisemuji Nov 25, 2023

wisemuji Nov 25, 2023

wisemuji Nov 25, 2023

wisemuji Nov 25, 2023

wisemuji Nov 25, 2023

dlwnsgus777 commented Nov 27, 2023 •

edited

wisemuji left a comment

wisemuji Nov 27, 2023

wisemuji Nov 27, 2023

wisemuji Nov 27, 2023

wisemuji Nov 27, 2023

wisemuji Nov 27, 2023

wisemuji Nov 27, 2023

wisemuji Nov 27, 2023

		@@ -0,0 +1,3 @@
		package blackjack.card

		sealed interface BlackJackCard

[Step2] 🚀 2단계 - 블랙잭 #656

[Step2] 🚀 2단계 - 블랙잭 #656

Conversation

dlwnsgus777 commented Nov 24, 2023 • edited

고민한 부분

wisemuji left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlwnsgus777 commented Nov 27, 2023 • edited

고민한 부분

wisemuji left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dlwnsgus777 commented Nov 24, 2023 •

edited

dlwnsgus777 commented Nov 27, 2023 •

edited