Step2 #645

moses-lee96 · 2023-11-21T12:31:33Z

필수 리뷰어

@laco-dev

개요

step1 코멘트 반영
MVC 패턴에 맞춰서 로직 개발
불변 객체 사용 연습

- dto -> domain 패키지 변경 - 빌더 패턴 fix

# step2

- 가독성 증진 - unused file 정리 # step2

moses-lee96 · 2023-11-21T13:10:27Z

src/main/kotlin/blackJack/view/OutputView.kt

+    fun printPlayersState(players: List<Player>) {
+        players.forEach { printPlayerState(it) }
+    }
+
+    fun printFinalState(players: List<Player>) {
+        players.forEach { printPlayerFinalState(it) }
+    }
+
+    fun printPlayerState(player: Player) {
+        player.hand
+            .joinToString { it.rank.symbol + it.suit.symbol }
+            .let { println(PLAYER_STATE_FORMAT.format(player.name, it)) }
+    }
+
+    private fun printPlayerFinalState(player: Player) {
+        player.hand
+            .joinToString { it.rank.symbol + it.suit.symbol }
+            .let { println(PLAYER__FINAL_STATE_FORMAT.format(player.name, it, player.calculateScore())) }
+    }


MVC 패턴의 정의에 따라서 View 를 작성했습니다.

view 는 model 에만 의존해야하고 (= view 내부에 model 의 코드만 있을 수 있고)

Q. view 가 model 을 알게됨으로 발생하는 비용은 없나요?
= view 가 아예 model 을 모르도록 dto 를 사용하면 이점이 있을까요?

view 가 model 을 알게됨으로 발생하는 비용은 없나요?

질문에서 이미 답을 어느정도 가지고 계신 것 같습니다.
Model이 가변 객체라면 View에서 의도치 않은 상태 변경을 야기할 수는 있겠습니다.
이는 흔히 DTO 를 이용한 MVC 패턴에서의 문제점과 같구요.

DTO를 이용해서 출력을 하는 것 자체는 관심사 분리라는 이점을 가지지만 충분히 이해하고 있다면 미션에서 이를 적용하시지는 않아도 괜찮습니다.

moses-lee96 · 2023-11-21T13:11:40Z

src/main/kotlin/blackJack/model/Player.kt

+infix fun Player.askMoreCard(dealer: Dealer) {
+    if (dealer checkDrawCardIsAllowedFor this) {
+        this requestCardToDealer dealer.drawCard()
+    }
+}
+
+infix fun Player.requestCardToDealer(card: Card) {
+    hand += card
+}


가독성을 위해 중위 함수를 선언해서 사용했습니다.

프로젝트 규모가 커졌을 경우, 이러한 함수들 관리가 어려울 수도 있을거 같은데요.

유지보수 측면에서 확장함수를 활용한 중위 함수 사용이 적절한지 조언 부탁드립니다!

중위함수도 결국 함수이기 때문에 프로젝트의 규모와 관계없다고 생각을 합니다.
다만 작성하신 코드를 통해 드는 의문은 아래와 같습니다.

유지보수르 고려한다면 왜 비즈니스 로직이 객체 내부에 있지 않고 외부에 작성되었을까?

중위 함수일 필요가 있는 형태인가?

코틀린에서 사용되는 대표적인 중위 함수로는 in operator 가 있는데요.
누가봐도 명확하게 이해가 가는 형태가 되어야 합니다.

val contains = player in players

moses-lee96 · 2023-11-21T13:14:30Z

src/main/kotlin/blackJack/model/Dealer.kt

+class Dealer(val name: String) {
+    private var cardDeck = CardDeck.of()


카지노에 Dealer 마다 CarDeck 이 여러개 인것처럼,

Dealer 에게 할당되는 CarDeck 이 있다고 생각했습니다.

moses-lee96 · 2023-11-21T13:15:44Z

src/main/kotlin/blackJack/model/Dealer.kt

+    fun drawCard(): Card {
+        val currentCard = cardDeck.cards
+            .shuffled()
+            .first()
+
+        cardDeck = cardDeck.cards
+            .filter { it != currentCard }
+            .let(::CardDeck)
+
+        return currentCard
+    }


CarDeck 의 cards 를 매번 새로 생성해서, 불변 객체로 관리하고자 했습니다.

laco-dev

블랙잭 미션 고생 많으셨습니다.

우선 PR 제목에 어떠한 미션을 구현했는지 같이 명시 해 주시면 좋을 것 같습니다.
미션을 진행하면서 구현해야 하는 결과에 집중하다보니 작은 단위의 객체들에 대한 기능과 테스트가 다소 아쉽게 작성되었습니다.

로또 미션에서 경험하신 것을 토대로 작은 단위부터 만들어 보면 조금 더 설계에 도움이 되실 것 같은데요.

잘 작성해주신 만큼 고민 해볼만한 내용을 많이(!?) 남기게 되었어요.
충분히 고민하고 도전 해 보시면 좋겠습니다.

laco-dev · 2023-11-22T10:52:27Z

src/main/kotlin/blackJack/view/InputView.kt

+
+        return input
+    }
+}


gradlew ktlintCheck 를 확인 해 주세요!

blackJack\view\InputView.kt:1:1 File must end with a newline (\n) (final-newline)

laco-dev · 2023-11-22T11:01:19Z

src/main/kotlin/blackJack/controller/BlackJackController.kt

+
+class BlackJackController {
+    fun play() {
+        val req = InputView.getNames()


req 라는 이름 대신 명확하게 변수 이름을 정해주면 좋을 것 같아요!
이하 동일합니다.

laco-dev · 2023-11-22T11:05:51Z

src/main/kotlin/blackJack/model/enums/Suit.kt

+enum class Suit(val symbol: String) {
+    CLUBS("클로버"),
+    DIAMONDS("다이아몬드"),
+    HEARTS("하트"),
+    SPADES("스페이드")
+}


symbol의 경우 어떻게 출력할지는 UI의 관심사로 볼 수 있지 않을까요?
가령 스페이드를 "♤" 이렇게 출력하게 된다면 도메인 모델의 수정이 불가피합니다.

laco-dev · 2023-11-22T11:07:27Z

src/main/kotlin/blackJack/model/enums/Rank.kt

@@ -0,0 +1,21 @@
+package blackJack.model.enums


enums 를 별도의 패키지로 관리하는 것은 어떤 이점이 있나요?
sealed class, interface, abstract class도 모두 패키지가 나뉘어야 한다고 봐야 할까요?

만약 더이상 enum class가 아니게 된다면 패키지도 바꿔야 하는 문제가 되지는 않을까요?

laco-dev · 2023-11-22T11:08:40Z

src/main/kotlin/blackJack/view/InputView.kt

+        return input.replace(" ", "")
+            .split(",")


순서의 차이같지만, trim() 을 사용하도록 만들 수도 있을 것 같네요.

input.split(",") .map { it.trim() }

laco-dev · 2023-11-22T11:36:17Z

src/main/kotlin/blackJack/model/Player.kt

+
+class Player(
+    val name: String,
+    var hand: List<Card> = listOf()


중위함수를 객체 외부에 작성하면서 외부에서 값을 수정할 수 있도록 var 가 되었어요.
이는 어디서든 이 객체의 hand 값을 바꿀 수 있다는 위험성을 가지게 됩니다.

불변 객체를 유지하는 것을 목표로 한다면, 어떠한 동작이 새로운 Player를 반환하도록 만들 수 있지 않을까요?
그렇지 않다면 backing property 등을 이용해 내부에서 가변 객체를 가지는 전략을 사용해 보면 좋을 것 같습니다.

laco-dev · 2023-11-22T11:38:41Z

src/test/kotlin/blackjack/domain/DealerSpec.kt

+        `when`("딜러에게 카드를 한장 나눠줬을때") {
+            val prevCount = dealer.countCard()
+            dealer.drawCard()
+            val currentCount = dealer.countCard()
+
+            then("카드덱에는 한장의 카드가 사라졌다.") {
+                currentCount shouldBe prevCount - 1
+            }
+        }


이 부분에 있어서도 카드덱을 유연하게 구성한다면 조금 더 간단하게 만들 수 있어 보이네요!

2장의 카드를 가지고 있을 때 1장을 나눠주면 보유한 카드는 총 1장이다.

laco-dev · 2023-11-22T11:40:11Z

src/test/kotlin/blackjack/domain/PlayerSpec.kt

+        val player = Player("플레이어")
+        val dealer = Dealer("딜러")


플레이어 입장에서는 딜러에게 카드를 받던 누구에게 카드를 받던 관심이 없다고 볼 수 있습니다.
*옆사람에게 카드를 받을 수도 있겠군요!
그렇다면 딜러와의 의존성을 낮춰 플레이어에 대한 테스트에 집중 해 볼 수 있지 않을까요?

laco-dev · 2023-11-22T11:43:27Z

src/test/kotlin/blackjack/domain/PlayerSpec.kt

+            }
+        }
+
+        `when`("bust 상태가 아닌 플레이어가 딜러에게 카드를 더 달라고 요구하면") {


테스트 코드는 기능 명세서라고 부릅니다.
즉 프로그램에 대한 기능을 최대한 구체적으로 나타내야 한다는 의미인데요.

이 테스트를 봤을 때 bust 상태가 무엇이지? 부터 시작할 수 있습니다.
TDD 로 구현한다고 가정했을 때 이를 고려한다면 아마도 이러한 순서로 기능이 나올 수 있겠습니다.

> 숫자 카드의 점수는 각 숫자로 계산된다. > A, K, J 카드는 각각 10점으로 계산된다. > ... > 플레이어가 가진 카드들의 점수 합계가 21점을 초과하면 버스트이다. // 아 이래서 버스트구나! > 버스트 상태가 아닌 플레이어가 ...

laco-dev · 2023-11-22T11:45:03Z

src/test/kotlin/blackjack/domain/PlayerSpec.kt

+        player requestCardToDealer Card(Suit.SPADES, Rank.QUEEN)
+        player requestCardToDealer Card(Suit.CLUBS, Rank.EIGHT)
+
+        `when`("딜러에게 카드를 더 달라고 요구하면") {


플레이어가 자율적으로 행동하는 객체라면, 카드를 더 뽑을 수 있는지 여부에 대해서도 플레이어가 결정할 수 있도록 만들어 볼 수 있을 것 같습니다.

val player: Player // 플레이어가 가지고 있는 카드들을 스스로 판단해서, 더 뽑을 수 있는지 여부를 반환한다. val shouldDraw: Boolean = player.shouldDraw()

moses-lee96 added 11 commits November 15, 2023 22:18

fix: step1 리뷰 반영

4d8a92c

- dto -> domain 패키지 변경 - 빌더 패턴 fix

feat: 1차 요구사항 정리

8cc8728

# step2

chore: 의존성 정리

cc9a256

# step2

chore: 요구사항 및 용어정리

77f999a

# step2

feat: Player 동작 정의

3efda6a

# step2

feat: Dealer 동작 정의

562450a

# step2

feat: enum 객체 정의

6f49311

# step2

feat: 카드객체 정의

e241a39

# step2

feat: 뷰 객체 정의

118996e

# step2

feat: 컨트롤러 정의

0a5687d

# step2

feat: 파일 정리

5f843a9

- 가독성 증진 - unused file 정리 # step2

moses-lee96 commented Nov 21, 2023

View reviewed changes

laco-dev requested changes Nov 22, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Step2 #645

Step2 #645

moses-lee96 commented Nov 21, 2023 •

edited

moses-lee96 Nov 21, 2023

laco-dev Nov 22, 2023

moses-lee96 Nov 21, 2023

laco-dev Nov 22, 2023

moses-lee96 Nov 21, 2023

moses-lee96 Nov 21, 2023

laco-dev left a comment

laco-dev Nov 22, 2023

laco-dev Nov 22, 2023

laco-dev Nov 22, 2023

laco-dev Nov 22, 2023

laco-dev Nov 22, 2023

laco-dev Nov 22, 2023

laco-dev Nov 22, 2023

laco-dev Nov 22, 2023

laco-dev Nov 22, 2023

laco-dev Nov 22, 2023

		class Dealer(val name: String) {
		private var cardDeck = CardDeck.of()

		val player = Player("플레이어")
		val dealer = Dealer("딜러")

Step2 #645

Are you sure you want to change the base?

Step2 #645

Conversation

moses-lee96 commented Nov 21, 2023 • edited

필수 리뷰어

개요

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

laco-dev 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

Choose a reason for hiding this comment

moses-lee96 commented Nov 21, 2023 •

edited