[1단계 - 블랙잭 게임 실행] 커찬(이충안) 미션 제출합니다. #8

leegwichan · 2024-03-08T12:12:02Z

설계에서 고려했던 부분

딜러와 플레이어의 중복 코드 제거

추가된 요구 사항에 있는 '딜러와 플레이어에서 발생하는 중복 코드를 제거'를 위해 추상 클래스(Participant)를 도입했습니다. "시작 카드 추가", "중간 카드 추가", "현재 카드의 점수 계산" 등의 중복 로직은 Participant에 위치 했습니다. 그리고 "승패 판단", "카드를 추가적으로 뽑을 수 있는 기준" 등은 Dealer와 Player가 다르므로 각각 클래스에 구현했습니다.

`Card`의 최대 및 최소 점수 제공

처음에는 Ace카드는 1점을 제공하고 외부에서 경우에 따라 10점을 추가해주려고 했습니다. 이 방법은 "Ace 카드는 1점 또는 11점으로 사용된다"는 책임을 외부로 넘긴다고 생각하여 다른 방법을 고민했습니다.
그래서 저희는 Card에서 getMinScore(), getMaxScore()을 제공하는 방법으로, 카드 한 장의 점수를 알 수 있도록 했습니다. 대부분의 카드의 getMinScore(), getMaxScore()의 값이 같아 중복된다고 생각할 수 있지만, "Ace는 1점 또는 11점으로 사용된다"는 책임을 Card에 부여할 수 있었습니다.

이외 고려했던 부분

요구사항 최소화

주어진 시간 내에 동작하는 코드를 작성하기 위해 요구사항을 최소화 했습니다. 블랙잭의 승패 판단의 경우 요구사항에 따로 명시 되지 않아, "딜러와 플레이어가 21점을 넘지 않고, 같은 점수인 경우 딜러가 승리"하도록 했습니다. 최대한 현실에 가까운 기능보다는 동작하는 코드를 빠르게 완성하는 것을 우선시 했습니다. 이를 통해 시간 내에 주어진 요구사항을 만족하면서도 도메인의 역할과 책임을 분리하는데 집중할 수 있었습니다.

단위 테스트의 픽스쳐 분리

PlayerTest, DealerTest 공통적으로 사용하는 List<Card>가 많아 이를 Fixture로 분리했습니다. 이를 통해 Fixture를 사용하는 곳에서 더 가독성 있는 코드를 사용할 수 있었습니다. 그리고 추후 변경되는 요구 사항이 변경된다면 Fixture가 있는 부분만을 수정하여 쉽게 대응할 수 있도록 했습니다.

Co-authored-by : zangsu <[email protected]>

Co-authored-by: zangsu <[email protected]>

- Player 가 Name 을 가지도록 생성자 변경 - 추상 클래스의 생성자 접근제어자 변경 - Dealer 기본 생성자 추가 Co-authored-by: zangsu <[email protected]>

Co-authored-by: zangsu <[email protected]>

Libienz

안녕하세요 커찬 🙌
6기 크루 리비입니다 😀

아이디어가 번뜩이는 코드 잘 보았습니다 🙇🏻‍♂️
커찬과 얘기해보고 싶은 내용 코멘트로 남겨보았어요 👊

이어지는 블랙잭 미션도 화이팅입니다 💪🏻

Libienz · 2024-03-09T14:29:12Z

src/main/java/blackjack/BlackJackGame.java

+    private void play(Players players, Dealer dealer, Deck deck) {
+        players.play(this::playTurn, deck);
+        while (dealer.isDrawable()) {
+            outputView.printDealerDraw();
+            dealer.add(deck.draw());
+        }


아이디어가 번뜩이시는군요 👍

뷰로직과 엉켜있는 플레이어 덱 완성 로직을 컨트롤러의 메서드 레퍼런스로 해결하셨군요
배우고 갑니다 💪🏻

저는 정말 이 방법이 좋은 방법인지 고민되긴 해요... 제가 받았던 리뷰에는 이런 내용도 있었어요!

코드상으로는 의존성이 분리된 것 처럼 보이지만, 런타임에 도메인 객체 내에서 BiConsumer로 View의 영향을 받는 로직이 주입되어 수행될 수도 있겠어요.
BlackJackGame.playTurn 메서드에 핵심적인 도메인 로직이 노출되어 있는 것 같기도 하네요.
view에서 값을 받아온 후에 넘겨주어 이곳에 핵심 도메인 규칙들이 위치하도록 변경해보는 것은 어떤가요?

Libienz · 2024-03-09T14:30:59Z

src/main/java/blackjack/domain/Deck.java

+
+    public static Deck createShuffledDeck() {
+        List<Card> cards = Arrays.stream(Shape.values())
+                .map(Deck::makeCards)
+                .flatMap(List::stream)
+                .collect(toList());
+        Collections.shuffle(cards);
+        return new Deck(cards);
+    }
+


복잡한 생성 로직을 정적 팩토리 메서드로 추상화 해주셨네요 👍

Libienz · 2024-03-09T14:32:10Z

src/main/java/blackjack/domain/Deck.java

+
+    private final Queue<Card> cards;
+


cards의 자료구조로 큐를 운용하신 이유가 있을까요?
커찬의 의도 궁금합니다 🤔

Deck에서는 기본적으로 card들을 만들어 놓고, 제일 위에 있는 카드를 뽑아쓰는 구조이다 보니, Queue구조를 사용했습니다. 만약 중간에 있는 카드가 필요했다면 LinkedList 등과 같은 다른 구조를 사용하지 않았을까 하네요.

Libienz · 2024-03-09T14:33:01Z

src/main/java/blackjack/domain/card/Card.java

+        this.value = Objects.requireNonNull(value);
+        this.shape = Objects.requireNonNull(shape);


null 검증 👍

Libienz · 2024-03-09T14:42:30Z

src/main/java/blackjack/domain/participant/Dealer.java

+
+public class Dealer extends Participant {
+


상속에 대한 커찬의 생각 궁금합니다. 개인적으로는 상속에 여러 단점을 높게 평가하는 편인데요

한번 읽어보셔도 좋을 만한 참고글 첨부합니다 👍
https://tecoble.techcourse.co.kr/post/2020-05-18-inheritance-vs-composition/

상속보다는 합성을 사용하라는 말, 저도 많이 들었던 것 같아요. 저는 "지금 코드에서 Participant에 따라 Dealer와 Player가 크리티컬하게 문제가 생길까?에 대해 고민해보고, 문제가 없어 사용한 것 같아요.

상속의 단점들을 보완하기 위해. 자체적인 규칙들을 도입해봤어요~

상속을 이용할 때는 '왠만하면' 추상 클래스를 이용한다.

직접 객체가 되면서 다른 클래스의 상위 클래스가 된다고 하면, 너무 많은 역할을 가지게 되어 유지보수가 힘들 것 같았어요.

추상 클래스의 모든 메서드는 final 또는 abstract로 선언한다.

외부로 제공하는 public method와 서로 다른 부분을 나타내는 abstract method를 구분해서 공용 메서드를 오버라이딩을 막아 최대한 예측되지 않는 일이 일어나는 것을 방지했어요.

Libienz · 2024-03-09T15:21:43Z

src/main/java/blackjack/domain/participant/Dealer.java

+
+    public MatchResultDto match(Players players) {
+        int winCount = (int) players.getPlayers().stream()
+                .filter(this::isWin)
+                .count();
+        int loseCount = (int) players.getPlayers().stream()
+                .filter(this::isLose)
+                .count();
+        return new MatchResultDto(winCount, loseCount);
+    }
+


만약 비긴 횟수도 관리해달라는 요구사항이 생기면 현재의 코드에서 Dealer를 찾아와야 합니다.

승패를 말아주는 새로운 도메인에 책임을 위임하는 것이 개인적으로는 좋아보이네요 😃

지금은 Dealer와 Players를 감싸는 도메인이 필요하지 않아서, 승부를 판단하는 메서드를 Dealer에 사용한 것 같아요. 조금 더 요구사항이 복잡해지고 Dealer의 역할이 무거워지는 것이 느껴진다면 다른 도메인 객체를 도입해야 겠네요 (정확하게는 2단계에 도입하게 되겠네요...)

Libienz · 2024-03-09T15:26:19Z

src/main/java/blackjack/view/OutputView.java

+    private static final Map<Shape, String> SHAPE_NAME = Map.of(
+            Shape.HEART, "하트",
+            Shape.SPADE, "스페이드",
+            Shape.DIAMOND, "다이아몬드",
+            Shape.CLOVER, "클로버"
+    );
+
+    private static final Map<Value, String> VALUE_NAME = Map.ofEntries(
+            Map.entry(Value.ACE, "A"), Map.entry(Value.TWO, "2"),
+            Map.entry(Value.THREE, "3"), Map.entry(Value.FOUR, "4"),
+            Map.entry(Value.FIVE, "5"), Map.entry(Value.SIX, "6"),
+            Map.entry(Value.SEVEN, "7"), Map.entry(Value.EIGHT, "8"),
+            Map.entry(Value.NINE, "9"), Map.entry(Value.TEN, "10"),
+            Map.entry(Value.JACK, "J"), Map.entry(Value.QUEEN, "Q"),
+            Map.entry(Value.KING, "K")
+    );


뷰로직을 enum으로부터 분리해주셨군요 👍

Libienz · 2024-03-09T15:28:27Z

src/main/java/blackjack/view/OutputView.java

+        System.out.println();
+        System.out.println("딜러와 " + toPrintedFormat(players) + "에게 2장을 나누었습니다.");


줄바꿈을 위해 시스템 콜을 이용하시는 것보다 System.lineSeparator() 사용하는 것이 더 좋아보입니다 🤔

제가 시스템 콜에 대해서 잘 몰라서 그러는데, println()과 System.lineSeparator()를 사용하는 것이 어떤 차이가 있을까요? 저는 둘 다 OS에 맞춘 개행 문자를 사용하는 걸로 알고 있어서 가독성이 좀 더 좋은 println()을 사용했어요~

Libienz · 2024-03-09T15:30:58Z

src/test/java/blackjack/domain/CardTest.java

+    public static final List<Card> CARDS_SCORE_4 = List.of(
+            new Card(Value.TWO, Shape.HEART),
+            new Card(Value.TWO, Shape.SPADE)
+    );


테스트를 위한 데이터들 미리 생성해주셨군요

다만 public 접근 제어자와 static 키워드가 개인적으로는 우려되네요 🤔

여러 테스트에서 공용으로 사용되는 데이터들이어서 접근 제어자를 public으로 해주었습니다.
만약에 Card의 요구사항이 변경될 때는 단순히 CardTest의 픽스처만을 수정하도록 하기 위해서요.

Libienz · 2024-03-09T15:33:01Z

src/test/java/blackjack/domain/participant/DealerTest.java

+    @DisplayName("플레이어와의 승패를 판단할 수 있다.")
+    @Nested
+    class IsWinTest {
+
+        @DisplayName("플레이어가 21일 넘을 경우, 딜러가 이긴다.")
+        @ParameterizedTest
+        @MethodSource("dealerCards")
+        void whenPlayerBusted_dealerWin(List<Card> cards) {
+            Dealer dealer = new Dealer(cards);
+            Player player = new Player(BUSTED, DEFAULT_NAME);
+
+            assertThat(dealer.isWin(player)).isTrue();
+        }
+


앞서 말씀드린 것처럼 승패를 판정하는 새로운 도메인이 등장하면 관련 로직을 응집할 수 있고 테스트도 분리 시킬 수 있어보여요 👍

Co-authored-by: zangsu <[email protected]>

3Juhwan

커찬, 안녕하세요 좋은 코드 잘 봤어요!
코드를 보면서 제가 고민했던 부분에 대한 정답도 찾을 수 있었어요. 😊
몇가지 코멘트 남겼으니까 확인 부탁드려요 😀

3Juhwan · 2024-03-10T04:10:26Z

src/main/java/blackjack/BlackJackGame.java

+    public void run() {
+        Deck deck = Deck.createShuffledDeck();
+        Dealer dealer = new Dealer();
+        Players players = createPlayers();
+        drawStartCards(dealer, players, deck);
+        play(players, dealer, deck);
+        printResult(dealer, players);
+    }


도메인 로직과 뷰 로직이 엉켜있는 미션이었는데 깔끔하게 처리하셨네요 👍

3Juhwan · 2024-03-10T04:14:20Z

src/main/java/blackjack/domain/Deck.java

+        if (cards.isEmpty()) {
+            throw new IllegalStateException("카드를 더 이상 뽑을 수 없습니다.");


깔끔하네요 👍

3Juhwan · 2024-03-10T04:19:05Z

src/test/java/blackjack/domain/DeckTest.java

+    @DisplayName("카드는 최대 52장만 뽑을 수 있다.")
+    @Test
+    void drawTest_drawTooManyTimes_throwException() {
+        Deck deck = Deck.createShuffledDeck();
+        drawRepeat(deck, 52);
+
+        assertThatThrownBy(() -> deck.draw())
+                .isInstanceOf(IllegalStateException.class)
+                .hasMessage("카드를 더 이상 뽑을 수 없습니다.");
+    }


테스트가 깔끔하네요 👍

3Juhwan · 2024-03-10T04:19:59Z

src/main/java/blackjack/domain/Deck.java

+    public static Deck createShuffledDeck() {
+        List<Card> cards = Arrays.stream(Shape.values())
+                .map(Deck::makeCards)
+                .flatMap(List::stream)
+                .collect(toList());
+        Collections.shuffle(cards);
+        return new Deck(cards);
+    }


정적 팩토리 메서드를 사용한 커찬의 이유가 궁금해요 💭

"52장을 랜덤으로 구성하는 부분"과 "카드를 생성하는 부분"을 분리하고 싶었습니다.
처음에는 DeckFactory라는 클래스를 만들어 생성을 담당하게 하려 했으나, 로직이 짧고 한 가지 종류밖에 없어 정적 팩토리 메서드를 이용했습니다.

3Juhwan · 2024-03-10T06:08:58Z

src/main/java/blackjack/view/OutputView.java

+
+    private static final Map<Shape, String> SHAPE_NAME = Map.of(
+            Shape.HEART, "하트",
+            Shape.SPADE, "스페이드",
+            Shape.DIAMOND, "다이아몬드",
+            Shape.CLOVER, "클로버"
+    );
+
+    private static final Map<Value, String> VALUE_NAME = Map.ofEntries(
+            Map.entry(Value.ACE, "A"), Map.entry(Value.TWO, "2"),
+            Map.entry(Value.THREE, "3"), Map.entry(Value.FOUR, "4"),
+            Map.entry(Value.FIVE, "5"), Map.entry(Value.SIX, "6"),
+            Map.entry(Value.SEVEN, "7"), Map.entry(Value.EIGHT, "8"),
+            Map.entry(Value.NINE, "9"), Map.entry(Value.TEN, "10"),
+            Map.entry(Value.JACK, "J"), Map.entry(Value.QUEEN, "Q"),
+            Map.entry(Value.KING, "K")
+    );


적절히 잘 분리되어 있네요 👍

3Juhwan · 2024-03-10T06:12:29Z

src/main/java/blackjack/view/OutputView.java

+    private static final int PLAYER_START_CARDS_SIZE = 2;
+
+    public void printStartCards(Dealer dealer, Players players) {
+        System.out.println();


개행 출력을 위해 사용할 수 있는 방법이 다음 2가지 정도 알고 있는데요

System.lineSeparator()

System.out.println()

\n 대신에 System.lineSeparator()을 사용하는 것이 좋다고 생각해요. 이유는 아실 거라 생각합니다!
하지만 한 줄을 띄운다라는 의미를 명확하게 표현하는 데에는 println()도 괜찮다고 생각해요 😀
커찬은 어떻게 생각하시나요?

어느 방법이든 가독성이 좋다면 다 좋지 않을까 생각해요. (내부 메서드가 어떻게 구현했는지는 상관없는 느낌)

public void printNextLine() { System.out.print(System.lineSeparator()); } public void printNextLine() { System.out.println(); } public void printNextLine() { System.out.printf("%s"); }

그런데, 따로 메서드로 분리하지 않고 직접 사용한다면, System.out.println()이 가장 가독성이 좋지 않나 싶네요.

3Juhwan · 2024-03-10T06:43:44Z

src/main/java/blackjack/domain/participant/Player.java

+
+    Player(List<Card> cards, Name name) {
+        super(cards);
+        this.name = Objects.requireNonNull(name);


널 체크 좋은 것 같습니다 👍 빠른 실패가 가능하겠네요!

3Juhwan · 2024-03-10T06:49:51Z

src/main/java/blackjack/domain/Deck.java

+
+    public static Deck createShuffledDeck() {
+        List<Card> cards = Arrays.stream(Shape.values())
+                .map(Deck::makeCards)
+                .flatMap(List::stream)
+                .collect(toList());
+        Collections.shuffle(cards);
+        return new Deck(cards);
+    }
+
+    private static List<Card> makeCards(Shape shape) {
+        return Arrays.stream(Value.values())
+                .map(value -> new Card(value, shape))
+                .toList();
+    }
+


해당 부분은 모든 모양과 모든 숫자에 대해 카드를 생성하는 로직 같은데요!
중첩 반복문을 사용해서 생성하는 것에 대해 어떻게 생각하시나요? 🧐

예쁜 코드를 작성하는 것이 모두의 공통 목표라고 생각해요. 코드를 읽는 것은 같은 개발자이고, 중첩 반복문을 모르는 개발자는 없겠죠 👀
중첩 반복문으로 작성하는 것이 훨씬 가독성이 좋다고 생각하는데, 커찬의 생각이 궁금해요! 💭

저는 java8에 익숙한 개발자가 읽는다면, flatmap() 정도는 사용해도 되지 않을까 합니다. java에서 제공해주는 라이브러리를 잘 사용하는 것, 잘 아는 것도 개발자가 알아야 할 의무이니까요~

위와 같은 시선에서 본다면, flatmap()도 이중 반복문처럼 보이지 않을까요?

3Juhwan · 2024-03-10T06:56:58Z

src/main/java/blackjack/domain/participant/Player.java

+
+    public boolean isWin(Dealer dealer) {
+        return !dealer.isWin(this);
+    }
+


플레이어에게 승패를 물어보는 동작인데, 플레이어는 딜러에게 되묻는 것 같네요 😅
많은 고민을 하셨을 것 같은데, 이렇게 작성하신 이유가 궁금해요!
한쪽으로 책임을 몰아주는 건 어떤가요?

최대한 한쪽(Dealer)으로 책임을 몰아주다보니 Player에서는 역으로 Dealer에게 물어보는 코드가 작성된 것 같아요.
Controller에서 Player의 승패 여부를 출력할 때, Player에게 승리 여부를 물어보는 코드가 있는 것이 자연스럽다고 생각한 것 같아요.

3Juhwan · 2024-03-10T07:02:26Z

src/test/java/blackjack/domain/participant/DealerTest.java

+    private static final List<Card> CARDS_SCORE_4 = CardTest.CARDS_SCORE_4;
+    private static final List<Card> TWO_ACE = CardTest.TWO_ACE;
+    private static final List<Card> SCORE_13_WITH_ACE = CardTest.SCORE_13_WITH_ACE;
+    private static final List<Card> CARDS_SCORE_16 = CardTest.CARDS_SCORE_16;
+    private static final List<Card> CARDS_SCORE_17 = CardTest.CARDS_SCORE_17;
+    private static final List<Card> BLACKJACK = CardTest.BLACKJACK;
+    private static final List<Card> BUSTED = CardTest.BUSTED;
+    private static final Name DEFAULT_NAME = NameTest.DEFAULT_NAME;


재정의하는 이유가 궁금해요 💭

원래는 테스트에 적을 때 CardTest.CARDS_SCORE_16이라고 적으면 길어질 것 같아서 클래스마다 재정의했어요.

그런데 리뷰어 피드백 중에 "실제 값을 보기 위해서는 두번 타고 들어가야 볼수 있다"는 내용이 있더라구요. 그래서 클래스마다 재정의한 부분을 지웠습니다~~

…alue` 필드 추가 Co-authored-by: zangsu <[email protected]>

Co-authored-by: zangsu <[email protected]>

robinjoon

코드 잘 작성하셨네요.. 다른 분들이 다들 질문해주셔서 남길 부분이 없었습니다. ㅜㅜ

robinjoon · 2024-03-10T13:40:03Z

src/main/java/blackjack/domain/participant/Participant.java

+    public final void drawStartCards(Deck deck) {
+        if (!hand.isEmpty()) {
+            throw new IllegalStateException("이미 시작 카드를 뽑았습니다.");
+        }
+        for (int i = 0; i < START_CARDS_SIZE; i++) {
+            add(deck.draw());
+        }
+    }
+
+    public final void add(Card card) {
+        if (!isDrawable()) {
+            throw new IllegalStateException("더 이상 카드를 추가할 수 없습니다.");
+        }
+        hand.add(card);
+    }


[질문] 참가자의 첫 카드를 뽑는 로직은 Deck 을 Participant 내부에서 사용하고, 그 외에 카드를 뽑는 로직은 Participant 외부에서 Deck 의 Card 를 뽑도록 되어있는걸로 보입니다. 왜 다른 방식으로 구현하셨나요?

페어와 의견 충돌이 있었던 부분이에요. 저는 Participant와 Deck의 의존성을 분리하기 위해 Card로 넘겨야 한다고 했고, 페어는 Deck;으로 넘기는 것이 좀 더 주체적인 행동이 될 수 있다고 생각했어요.
제 의견에 따라 add()를 구현하고 난 후에 막상 drawStartCards()를 구현을 하려 하니, Deck을 넘기는 것이 합리적이라고 생각해서 drawStartCards()에서는 Deck을 사용했네요. 2단계에 들어가면서 다시 생각해보고 한 쪽으로 통일 할 것 같습니다~

robinjoon · 2024-03-10T13:43:52Z

src/main/java/blackjack/dto/MatchResultDto.java

+package blackjack.dto;
+
+public record MatchResultDto(int winCount, int loseCount) {
+}


[질문] 플레이어의 카드나 점수를 출력하는 것에서는 도메인을 직접 넘기셨는데, 딜러의 승패 결과는 DTO를 사용하신 이유가 궁금합니다!

승리 횟수와 패배 횟수를 묶은 객체를 넘겨주고 싶어 이렇게 했는데요. 다시 생각해보니까 Dealer에서 winCount와 loseCount를 제공하는 각각 제공하는 public method를 만드는 것이 더 좋았을 것 같네요.

Co-authored-by: zangsu <[email protected]>

leegwichan

로빈, 망쵸 리비 다들 적어주신 리뷰 잘 봤어요!
여러번 알람 울릴 것 같아서, 한번에 답변 달아봤습니다!
추가적으로 궁금하신 사항 등이 있다면 코멘트 남겨주세요! 화요일 리뷰 시간에 봐요!

leegwichan · 2024-03-11T03:21:33Z

src/main/java/blackjack/BlackJackGame.java

+    private void play(Players players, Dealer dealer, Deck deck) {
+        players.play(this::playTurn, deck);
+        while (dealer.isDrawable()) {
+            outputView.printDealerDraw();
+            dealer.add(deck.draw());
+        }


저는 정말 이 방법이 좋은 방법인지 고민되긴 해요... 제가 받았던 리뷰에는 이런 내용도 있었어요!

코드상으로는 의존성이 분리된 것 처럼 보이지만, 런타임에 도메인 객체 내에서 BiConsumer로 View의 영향을 받는 로직이 주입되어 수행될 수도 있겠어요.
BlackJackGame.playTurn 메서드에 핵심적인 도메인 로직이 노출되어 있는 것 같기도 하네요.
view에서 값을 받아온 후에 넘겨주어 이곳에 핵심 도메인 규칙들이 위치하도록 변경해보는 것은 어떤가요?

leegwichan · 2024-03-11T03:30:41Z

src/main/java/blackjack/domain/participant/Dealer.java

+
+public class Dealer extends Participant {
+


상속보다는 합성을 사용하라는 말, 저도 많이 들었던 것 같아요. 저는 "지금 코드에서 Participant에 따라 Dealer와 Player가 크리티컬하게 문제가 생길까?에 대해 고민해보고, 문제가 없어 사용한 것 같아요.

상속의 단점들을 보완하기 위해. 자체적인 규칙들을 도입해봤어요~

상속을 이용할 때는 '왠만하면' 추상 클래스를 이용한다.

직접 객체가 되면서 다른 클래스의 상위 클래스가 된다고 하면, 너무 많은 역할을 가지게 되어 유지보수가 힘들 것 같았어요.

추상 클래스의 모든 메서드는 final 또는 abstract로 선언한다.

외부로 제공하는 public method와 서로 다른 부분을 나타내는 abstract method를 구분해서 공용 메서드를 오버라이딩을 막아 최대한 예측되지 않는 일이 일어나는 것을 방지했어요.

leegwichan · 2024-03-11T03:33:52Z

src/main/java/blackjack/domain/participant/Dealer.java

+
+    public MatchResultDto match(Players players) {
+        int winCount = (int) players.getPlayers().stream()
+                .filter(this::isWin)
+                .count();
+        int loseCount = (int) players.getPlayers().stream()
+                .filter(this::isLose)
+                .count();
+        return new MatchResultDto(winCount, loseCount);
+    }
+


지금은 Dealer와 Players를 감싸는 도메인이 필요하지 않아서, 승부를 판단하는 메서드를 Dealer에 사용한 것 같아요. 조금 더 요구사항이 복잡해지고 Dealer의 역할이 무거워지는 것이 느껴진다면 다른 도메인 객체를 도입해야 겠네요 (정확하게는 2단계에 도입하게 되겠네요...)

leegwichan · 2024-03-11T03:39:22Z

src/main/java/blackjack/domain/participant/Participant.java

+    public final void drawStartCards(Deck deck) {
+        if (!hand.isEmpty()) {
+            throw new IllegalStateException("이미 시작 카드를 뽑았습니다.");
+        }
+        for (int i = 0; i < START_CARDS_SIZE; i++) {
+            add(deck.draw());
+        }
+    }
+
+    public final void add(Card card) {
+        if (!isDrawable()) {
+            throw new IllegalStateException("더 이상 카드를 추가할 수 없습니다.");
+        }
+        hand.add(card);
+    }


페어와 의견 충돌이 있었던 부분이에요. 저는 Participant와 Deck의 의존성을 분리하기 위해 Card로 넘겨야 한다고 했고, 페어는 Deck;으로 넘기는 것이 좀 더 주체적인 행동이 될 수 있다고 생각했어요.
제 의견에 따라 add()를 구현하고 난 후에 막상 drawStartCards()를 구현을 하려 하니, Deck을 넘기는 것이 합리적이라고 생각해서 drawStartCards()에서는 Deck을 사용했네요. 2단계에 들어가면서 다시 생각해보고 한 쪽으로 통일 할 것 같습니다~

leegwichan · 2024-03-11T03:43:24Z

src/main/java/blackjack/domain/participant/Player.java

+
+    public boolean isWin(Dealer dealer) {
+        return !dealer.isWin(this);
+    }
+


최대한 한쪽(Dealer)으로 책임을 몰아주다보니 Player에서는 역으로 Dealer에게 물어보는 코드가 작성된 것 같아요.
Controller에서 Player의 승패 여부를 출력할 때, Player에게 승리 여부를 물어보는 코드가 있는 것이 자연스럽다고 생각한 것 같아요.

leegwichan · 2024-03-11T03:58:01Z

src/main/java/blackjack/domain/Deck.java

+
+    private final Queue<Card> cards;
+


Deck에서는 기본적으로 card들을 만들어 놓고, 제일 위에 있는 카드를 뽑아쓰는 구조이다 보니, Queue구조를 사용했습니다. 만약 중간에 있는 카드가 필요했다면 LinkedList 등과 같은 다른 구조를 사용하지 않았을까 하네요.

leegwichan · 2024-03-11T03:59:24Z

src/test/java/blackjack/domain/CardTest.java

+    public static final List<Card> CARDS_SCORE_4 = List.of(
+            new Card(Value.TWO, Shape.HEART),
+            new Card(Value.TWO, Shape.SPADE)
+    );


여러 테스트에서 공용으로 사용되는 데이터들이어서 접근 제어자를 public으로 해주었습니다.
만약에 Card의 요구사항이 변경될 때는 단순히 CardTest의 픽스처만을 수정하도록 하기 위해서요.

leegwichan · 2024-03-11T04:26:40Z

src/test/java/blackjack/domain/participant/DealerTest.java

+    private static final List<Card> CARDS_SCORE_4 = CardTest.CARDS_SCORE_4;
+    private static final List<Card> TWO_ACE = CardTest.TWO_ACE;
+    private static final List<Card> SCORE_13_WITH_ACE = CardTest.SCORE_13_WITH_ACE;
+    private static final List<Card> CARDS_SCORE_16 = CardTest.CARDS_SCORE_16;
+    private static final List<Card> CARDS_SCORE_17 = CardTest.CARDS_SCORE_17;
+    private static final List<Card> BLACKJACK = CardTest.BLACKJACK;
+    private static final List<Card> BUSTED = CardTest.BUSTED;
+    private static final Name DEFAULT_NAME = NameTest.DEFAULT_NAME;


원래는 테스트에 적을 때 CardTest.CARDS_SCORE_16이라고 적으면 길어질 것 같아서 클래스마다 재정의했어요.

그런데 리뷰어 피드백 중에 "실제 값을 보기 위해서는 두번 타고 들어가야 볼수 있다"는 내용이 있더라구요. 그래서 클래스마다 재정의한 부분을 지웠습니다~~

leegwichan · 2024-03-11T04:35:25Z

src/main/java/blackjack/domain/Deck.java

+    public static Deck createShuffledDeck() {
+        List<Card> cards = Arrays.stream(Shape.values())
+                .map(Deck::makeCards)
+                .flatMap(List::stream)
+                .collect(toList());
+        Collections.shuffle(cards);
+        return new Deck(cards);
+    }


"52장을 랜덤으로 구성하는 부분"과 "카드를 생성하는 부분"을 분리하고 싶었습니다.
처음에는 DeckFactory라는 클래스를 만들어 생성을 담당하게 하려 했으나, 로직이 짧고 한 가지 종류밖에 없어 정적 팩토리 메서드를 이용했습니다.

leegwichan · 2024-03-11T05:12:37Z

src/main/java/blackjack/domain/Deck.java

+
+    public static Deck createShuffledDeck() {
+        List<Card> cards = Arrays.stream(Shape.values())
+                .map(Deck::makeCards)
+                .flatMap(List::stream)
+                .collect(toList());
+        Collections.shuffle(cards);
+        return new Deck(cards);
+    }
+
+    private static List<Card> makeCards(Shape shape) {
+        return Arrays.stream(Value.values())
+                .map(value -> new Card(value, shape))
+                .toList();
+    }
+


저는 java8에 익숙한 개발자가 읽는다면, flatmap() 정도는 사용해도 되지 않을까 합니다. java에서 제공해주는 라이브러리를 잘 사용하는 것, 잘 아는 것도 개발자가 알아야 할 의무이니까요~

위와 같은 시선에서 본다면, flatmap()도 이중 반복문처럼 보이지 않을까요?

leegwichan and others added 30 commits March 5, 2024 15:17

docs : 기능 명세 및 클래스 설계 작성

870ad1a

feat (Card) : 카드에 따라 점수를 반환하는 기능 구현

a0a1235

feat (Deck) : 카드를 한 장 뽑는 기능 구현

2b505f1

feat (Dealer) : 딜러의 점수 계산 및 카드를 뽑을 수 있는지 확인하는 메서드 구현

f774cc7

style : 가독성을 위해 클래스 상단에 개행 추가, 클래스 아래 불필요한 개행 제거

3e83ed2

feat (Dealer) : 딜러 턴 진행 구현

99baedb

feat (Player) : 점수 계산 기능 구현

de19b03

feat (Player) : 딜러의 점수 계산 및 카드를 뽑을 수 있는지 확인하는 메서드 구현

1da0e57

Co-authored-by : zangsu <[email protected]>

feat (Player) : 카드를 한 장 추가하는 기능 구현

ed6c932

Co-authored-by: zangsu <[email protected]>

refactor (Dealer) : 요구 사항을 반영을 위해 턴 진행 기능을 카드를 한 장 추가하는 기능으로 변경

e11dee2

Co-authored-by: zangsu <[email protected]>

refactor : 도메인 구분을 위한 participant 패키지 분리

877680b

Co-authored-by: zangsu <[email protected]>

refactor (Participant) : 참가자의 공통 책임을 묶기 위해, 추상 클래스 사용

d7e7685

Co-authored-by: zangsu <[email protected]>

docs : 이름에 대한 요구사항 추가

bcf2305

Co-authored-by: zangsu <[email protected]>

feat (Name) : 요구사항에 따른 이름 검증 기능 구현

4b8ca83

Co-authored-by: zangsu <[email protected]>

feat (Player) : 플레이어가 이름을 갖도록 구현

3a71ec4

Co-authored-by: zangsu <[email protected]>

refactor : 참가자 관련 코드 개선

717e0cb

- Player 가 Name 을 가지도록 생성자 변경 - 추상 클래스의 생성자 접근제어자 변경 - Dealer 기본 생성자 추가 Co-authored-by: zangsu <[email protected]>

feat (Participant) : 게임 시작 후 카드를 두 장 뽑는 기능 구현

940a3a7

Co-authored-by: zangsu <[email protected]>

feat (Dealer) : 딜러가 플레이어와 승패를 확인하는 기능 구현

2079c7a

Co-authored-by: zangsu <[email protected]>

feat (Player) : 플레이어가 딜러와 승패를 확인하는 기능 구현

541e6c2

Co-authored-by: zangsu <[email protected]>

refactor (Deck) : 사용되지 않는 생성자 제거

c5aab02

Co-authored-by: zangsu <[email protected]>

feat (Players) : 제한된 수의 플레이어만 가지는 Players 생성

3bc66d8

Co-authored-by: zangsu <[email protected]>

feat (Players) : 플레이어가 중복되지 않도록 검증 기능 구현

da419b2

Co-authored-by: zangsu <[email protected]>

feat (Deck) : 카드가 섞인 덱이 생성되도록 구현

fd9106d

Co-authored-by: zangsu <[email protected]>

feat (Players) : 모든 플레이어들이 시작 카드를 뽑는 기능 구현

aea2457

Co-authored-by: zangsu <[email protected]>

feat (BlackJackGame) : 전반적인 제어 기능 구현

0fdf520

Co-authored-by: zangsu <[email protected]>

feat (InputView) : 한장의 카드를 더 받을지 요청하는 기능 구현

a6af44f

Co-authored-by: zangsu <[email protected]>

feat (OutputView) : 시작 카드를 출력하는 기능 구현

1d288f1

Co-authored-by: zangsu <[email protected]>

feat (OutputView) : 카드 출력 기능 구현

fcef46a

refactor (Participant) : 카드 출력 개수를 정하는 책임을 View로 넘기기 위해 시작 카드를 건네는 기능 제거

f35da09

Co-authored-by: zangsu <[email protected]>

feat (Dealer) : 모든 플레이어를 상대로 승패를 계산하는 기능 구현

7cbb8ca

Co-authored-by: zangsu <[email protected]>

leegwichan and others added 8 commits March 7, 2024 21:29

refactor (CardTest) : 다양한 상황의 카드들을 픽스쳐로 추출

116a613

Co-authored-by: zangsu <[email protected]>

style : 테스트 이름에서 경계값 제거

0d2c7d5

Co-authored-by: zangsu <[email protected]>

refactor : 이름에 관한 픽스쳐 NameTest에 통합

7d5b4e4

Co-authored-by: zangsu <[email protected]>

style : 사용하지 않는 import 문 제거

10bb3a8

Co-authored-by: zangsu <[email protected]>

style : 의미 없는 공백 제거

1fa7850

feat : 생성 단계에서 null을 입력받았을 경우, NullPointerException을 던지도록 변경

4f2994a

refactor (Participant) : 외부에서 사용하지 않는 변수의 접근 제어자 변경

b2c1c91

docs : README에서 구현에 관련된 내용들 삭제

4c5d051

Co-authored-by: zangsu <[email protected]>

Libienz reviewed Mar 9, 2024

View reviewed changes

leegwichan and others added 2 commits March 10, 2024 15:55

style (OutputView) : 메서드 이름을 추상적으로 변경

efc2b15

Co-authored-by: zangsu <[email protected]>

refactor (PlayerTurn) : 가독성을 위해 BiConsumer -> PlayerTurn 으로 변경

d7c7f64

Co-authored-by: zangsu <[email protected]>

3Juhwan reviewed Mar 10, 2024

View reviewed changes

leegwichan and others added 14 commits March 10, 2024 16:06

refactor (Value) : 추후 유지보수의 용이성과 enum 인스턴스 자체가 값에 대한 책임을 가지기 위해 `maxV…

4465e8e

…alue` 필드 추가 Co-authored-by: zangsu <[email protected]>

refactor (Deck) : 패키지 변경

cf3878f

Co-authored-by: zangsu <[email protected]>

refactor : 테스트를 간결하게 작성하기 위해, Test용 생성자를 사용

f3a92c0

Co-authored-by: zangsu <[email protected]>

style : import 문 정리 및 TODO 제거

a1ca324

Co-authored-by: zangsu <[email protected]>

feat (Hand) : 손패 계산의 책임을 분리하기 위해 Hand 구현

cfe591e

Co-authored-by: zangsu <[email protected]>

feat (Hand) : 버스트 상태를 판단하는 기능 구현

ba33f16

Co-authored-by: zangsu <[email protected]>

feat (Hand) : 블랙잭 판단 기능 구현

cf5c481

Co-authored-by: zangsu <[email protected]>

feat (Hand) : 카드를 추가하는 기능 구현

6bccbbb

Co-authored-by: zangsu <[email protected]>

feat (Hand) : 빈 손패인지 확인하는 기능 구현

770c1a9

Co-authored-by: zangsu <[email protected]>

refactor (Participant) : 카드와 관련된 책임들을 Hand 로 위임

72369a7

Co-authored-by: zangsu <[email protected]>

feat (Dealer) : 승패 조건에 블랙잭 추가

be2535a

Co-authored-by: zangsu <[email protected]>

refactor : 가독성을 위해 테스트 픽스처 분리

79068a6

Co-authored-by: zangsu <[email protected]>

refactor : 도메인 규칙과 직결되는 경계값을 픽스처에서 제거

f255e57

Co-authored-by: zangsu <[email protected]>

refactor : 시작 카드가 몇 개 보여지는지의 책임을 뷰에서 도메인으로 이동

4d88fee

Co-authored-by: zangsu <[email protected]>

robinjoon reviewed Mar 10, 2024

View reviewed changes

leegwichan and others added 2 commits March 11, 2024 14:57

refactor : DTO 사용을 최소화하기 위해, 원시값을 전달

b9feef4

Co-authored-by: zangsu <[email protected]>

refactor (Hand) : 가독성 개선을 위해 점수 계산 기능 변경

c0f458b

Co-authored-by: zangsu <[email protected]>

leegwichan commented Mar 11, 2024

View reviewed changes

		this.value = Objects.requireNonNull(value);
		this.shape = Objects.requireNonNull(shape);

		System.out.println();
		System.out.println("딜러와 " + toPrintedFormat(players) + "에게 2장을 나누었습니다.");

		if (cards.isEmpty()) {
		throw new IllegalStateException("카드를 더 이상 뽑을 수 없습니다.");

[1단계 - 블랙잭 게임 실행] 커찬(이충안) 미션 제출합니다. #8

Are you sure you want to change the base?

[1단계 - 블랙잭 게임 실행] 커찬(이충안) 미션 제출합니다. #8

Conversation

leegwichan commented Mar 8, 2024

설계에서 고려했던 부분

딜러와 플레이어의 중복 코드 제거

Card의 최대 및 최소 점수 제공

이외 고려했던 부분

요구사항 최소화

단위 테스트의 픽스쳐 분리

Libienz 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

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

3Juhwan 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

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

robinjoon 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

leegwichan 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

`Card`의 최대 및 최소 점수 제공