[🚀 사이클2 - 미션 (블랙잭 베팅)] 나무 미션 제출합니다. by symflee · Pull Request #1137 · woowacourse/java-blackjack

symflee · 2026-03-17T12:43:19Z

체크 리스트

미션의 필수 요구사항을 모두 구현했나요?
Gradle test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?
애플리케이션이 정상적으로 실행되나요?

어떤 부분에 집중하여 리뷰해야 할까요?

게임 진행을 Controller에서 어디까지 다루는 것이 허용되는지

현재는 playPhase(Dealer, Players) 안에서

플레이어들을 순회하며 각 플레이어 턴을 진행하고
플레이어 턴에서는 사용자 입력을 받아 카드를 더 받을지 결정하고
딜러는 별도 while 문으로 턴을 진행하는

형태입니다.

그런데 이 구조는 Controller가 게임의 흐름을 너무 많이 직접 다루는 것 같기도 하고, 반대로 플레이어 턴은 사용자 입력이 필요해서 도메인 안으로 넣기도 애매하게 느껴집니다.

제가 궁금한 점은 다음과 같습니다.

현재의 플레이어 순회와 턴 진행은 꼭 Controller가 아니더라도 외부에서 맡는 것도 허용되는 수준인지, 아니면 도메인 쪽으로 더 옮겨가는 것이 좋은지 궁금합니다.
결과적으로는 playPhase(Dealer, Players)를 readyPhase(Participants)처럼 playPhase(Participants)으로 수정하고 싶었는데, 이런 방향이 괜찮은 것인지도 궁금합니다.
(https://github.com/woowacourse/java-blackjack/pull/1137/changes#diff-bf343d0582705538ce76c37ebbe336f7a98051e3b9b8386f9bb31329d6deb2eaR55)

배팅 정보를 Player가 직접 가져야 하는지, 별도 객체가 관리해야 하는지

처음에는 Player가 배팅 금액을 직접 가지는 방향도 생각했는데, 배팅 상황을 BettingTable 같은 별도 객체가 관리하는 것이 더 낫지 않을까 고민했습니다.

이렇게 하면 플레이어 자체와 배팅 상황을 분리할 수 있고, 나중에 게임을 여러 번 진행하는 구조로도 확장하기 쉬울 것 같다고 생각했습니다.

Player가 배팅 금액을 직접 가지는 것보다, BettingTable이나 PlayerBet 같은 별도 객체가 관리하는 쪽이 더 좋은걸까요?
PlayerBet이 Player 자체를 들기보다 Name과 BettingAmount만 가지도록 분리하는 방향은 적절한지도 궁금합니다.

추가 기능 구현과 리팩토링의 우선순위는 어떤 기준으로 정해야할지

배팅 기능을 추가하는 과정에서 기존 코드의 많은 부분을 수정했습니다. 그러다보니 너무 많은 변경점이 생기게 되었고, 어디까지 수정을 해야할 지 또 거기서 어디서부터 기능을 추가할 지 풀어나가는 것이 조금 힘들었습니다.

그러다보니 이런 의문이 생겼습니다

리팩토링을 수행한 뒤, 기능을 추가하는 것
기능을 추가한 뒤, 리팩토링을 수행하는 것

둘 중 우선되는 방향이 있을까요? 또는 두 방식 중 선택할 때 참고할 만한 기준이 있을까요?

질문을 위해서 어디까지 스스로 고민해보아야 하는지

고민이 얕으면 질문이 너무 가볍고, 고민이 깊으면 오히려 꼬리 질문이 계속 생겨 초기 의문의 방향을 잃어버리곤 했습니다. 사실 이 문제 때문에 더욱 정신없는 미션 진행이 되었던 것 같습니다. 쏟아지는 새로운 개념과 새롭게 보이는 문제점들을 감당하기 어려웠던 것 같습니다.

주노는 이런 상황에서 질문의 적절한 깊이와 타이밍을 잡는 기준으로 있는 지 여쭤보고 싶습니다.

kang-hyungu

나무 몇 가지 피드백 남겼어요
확인해보시고 궁금한 점 있으면 질문주세요~

kang-hyungu · 2026-03-18T00:59:03Z

@@ -28,17 +37,64 @@ public GameController(InputView inputView, OutputView outputView, GameManager ma
    public void run() {


클래스명에 컨트롤러라고 붙인 이유가 뭔가요?

kang-hyungu · 2026-03-18T01:00:15Z

+
+    private void readyPhase(Dealer dealer, Players players) {
+        gameManager.dealStartingCards(dealer);
+        gameManager.dealCardTo(players, 2);


2가 뭘 의미하죠? 상수로 추출해서 의미 있는 이름을 부여해주세요
다른 팀원이 코드를 보고 무엇을 의미하는지 바로 파악할 수 있게 하세요

넵
INITIAL_CARD_COUNT 라는 이름을 부여하도록 수정했습니다.

kang-hyungu · 2026-03-18T01:00:55Z

+        gameManager.dealStartingCards(dealer);
+        gameManager.dealCardTo(players, 2);
+
+        Map<String, CardsSnapshot> result = new LinkedHashMap<>();


게임 결과를 처리하는 객체를 새로 추가해서 그 객체에게 이 작업을 맡기는 것도 괜찮겠네요

kang-hyungu · 2026-03-18T01:02:01Z

+        outputView.printInitialDealingResult(initialDealingResult);
    }

    private void resultPhase(Dealer dealer, Players players) {


이 코드도 게임 결과를 처리하는 객체에서 처리해도 되지 않을까요?
지금 같은 구조라면 결과를 다루는 로직을 다른 코드에서는 재사용을 못하겠네요

kang-hyungu · 2026-03-18T01:02:22Z

+        this.value = value;
+    }
+
+    public static BettingAmount of(int value) {


팩터리 메서드를 추가한 이유가 뭔가요?

솔직하게 말씀드리면, 정적 팩터리 메서드에 대해 학습하고 적용했던 것은 아니었습니다. 단순히 '정적 팩터리 메서드가 일반 생성자보다 의도를 명확히 전달하고 가독성을 높여주기 때문에 우선적으로 사용하는 것이 좋다' 정도로만 얕게 알고 기계적으로 적용했던 것 같습니다.

알아보고나니, 해당 클래스처럼 파라미터가 하나뿐이고 생성과정이 단순한 클래스에서는 팩토리 메서드가 주는 이점이 사실상 없다는 것을 느꼈습니다. 오히려 팩토리 메서드로 생성자를 불필요하게 한 번 더 감싸 코드가 늘어났다고 생각됩니다.

kang-hyungu · 2026-03-18T01:05:34Z

+        return new BettingTable(bettingTable);
+    }
+
+    public void add(PlayerBetting playerBetting) {


함수명을 add 보다 게임 용어로 적용해보면 어떨까요?

게임에서 베팅할 때 사용하는 표현인 'place a bet'에서 메서드명을 place로 수정해 보았습니다.

외부의 placeBet이라는 메서드 안에서 bettingTable.place(playerBet) 형태로 호출하게 되는데 이러한 네이밍이 적절할까요?

kang-hyungu · 2026-03-18T01:06:19Z

+    }
+
+    private int calculateDealerProfit(List<PlayerProfitResult> playersProfitResult) {
+        return (-1) * playersProfitResult.stream()


여기는 왜 -1을 곱하나요?

딜러의 수익은 플레이어들 수익 합의 반대라는 점을 직관적으로 나타내고 싶었습니다.

처음에 - 기호만 붙이는 것보다 보다 명시적으로 보이도록 * (-1)을 사용했었는데, -1 역시 문맥을 모르면 이해하기 힘든 매직 넘버라는 점을 간과했습니다.

코멘트 남겨주신 덕분에 알아보는 과정에서 부호를 반전시키는 Math.negateExact() 메서드를 새롭게 알게 되었습니다. 해당 메서드를 사용하는 것이 의도가 더 명확하게 드러나고 매직 넘버도 제거할 수 있어 해당 방식으로 수정해 보았습니다.

kang-hyungu · 2026-03-18T01:07:27Z

+
+import java.util.List;
+
+public record ParticipantsProfitResult(


result를 클래스명에 붙일 필요가 있을까요?

게임이 종료된 이후에 profit이 결정되기 때문에 profit만으로 결과라는 의미를 충분히 전달할 수 있어, result는 붙일 필요가 없다고 생각합니다. 수정하였습니다.

kang-hyungu · 2026-03-18T01:08:02Z

+import domain.participant.Name;
+import domain.participant.Player;
+
+public record PlayerBetting(Name playerName, BettingAmount bettingAmount) {


플레이어 베팅은 어떤 의도로 붙인 이름일까요? 베팅하다? 그냥 말 그대로 베팅?

플레이어의 배팅이라는 의미를 담고 싶었습니다.

PlayerBetting보다 PlayerBet이 더 의도에 가깝다고 생각돼 이름을 변경해 보았습니다.

다만 이 이름이 실제로도 자연스러운지에는 아직 확신이 없습니다.
혹시 보시기에 더 적절한 네이밍이 있다면 의견 부탁드립니다.

이런 식으로 플레이어와 배팅 금액을 함께 묶는 현재 구성 자체도 괜찮은 방향인지 궁금합니다!

kang-hyungu · 2026-03-18T01:09:41Z

@@ -14,7 +14,7 @@ public record CardsSnapshot(List<Card> cards) {

    public String getFormattedCards() {


이 메서드가 도메인 객체에 필요한가요?

필요하지 않다고 생각합니다.
출력용 문자열을 만드는 책임은 해당 클래스가 아니라 출력 계층에 있어야 한다고 봤습니다. 다만 이를 수정하는 과정에서 변경 범위가 커졌고, 사이클2의 배팅 기능 구현과 함께 진행하기에는 부담이 크다고 판단했습니다.
그래서 별도 클래스로 분리해 점진적으로 옮기는 방향을 택했고, 그 과정에서 기존 메서드가 남아 있는 상태로 제출하게 되었습니다.

Choi-JJunho

나무~ 미션 수행하느라 고생많으셨습니다!
구구가 리뷰를 같이 남겨주셔서 깜짝 놀랐네요 ㅎㅎ 😅
구구의 코멘트에도 같이 답변을 남겨주시면 좋을 것 같습니다 👍

step2도 화이팅입니다!

Choi-JJunho · 2026-03-18T12:50:42Z

+    private void readyPhase(Participants participants) {
+        gameManager.dealInitialCards(participants);
+
+        InitialDealingResult initialDealingResult = InitialDealingResult.from(participants);
+        outputView.printInitialDealingResult(initialDealingResult);
    }


현재의 플레이어 순회와 턴 진행은 꼭 Controller가 아니더라도 외부에서 맡는 것도 허용되는 수준인지, 아니면 도메인 쪽으로 더 옮겨가는 것이 좋은지 궁금합니다.
결과적으로는 playPhase(Dealer, Players)를 readyPhase(Participants)처럼 playPhase(Participants)으로 수정하고 싶었는데, 이런 방향이 괜찮은 것인지도 궁금합니다. commit

허용되는 수준 혹은 정답은 없습니다!
나무가 이렇게 수정하게된 이유와 이렇게 수정했을 때 어떤 차이가 있었는지 한번 이야기해주실 수 있나요~?

기존에는 초기 카드 지급과 출력용 데이터 조립을 하나의 클래스가 담당하고 있었습니다. 이를 초기 카드 지급은 GameManager가, 출력용 데이터 생성은 별도의 객체가 담당하도록 분리했습니다.

그 결과 게임 흐름 제어에 더 집중할 수 있게 되었고, 초기 카드 지급 규칙도 한 곳에 모여 응집도가 높아졌습니다. 또한 dealInitialCards(participants)처럼 가독성도 좋아졌다고 생각합니다.

좋습니다~! 분리한 이유와 그에 따른 이점을 잘 설명해주셨네요!
나무가 이야기해주신 dealInitialCards(participants) 형태로 가독성이 좋아진 부분도 동의합니다. 👍

다른 크루들과도 이야기를 나눠보면서 나무가 생각하는 타당한 의견을 견고히 다져가보면 좋을 것 같습니다 💪

Choi-JJunho · 2026-03-18T12:56:13Z

    private static final int HIT_THRESHOLD = 16;

    public Dealer() {
        super(new Name(DEFAULT_NAME));


지난 리뷰에서 제가 미처발견 못한 부분이였는데요 🥹
지금 Name의 정적 메서드에만 이름 validate가 들어가고있어요

public static Name from(String rawName) { validate(rawName); return new Name(rawName); }

이 경우 여기 딜러에서 생성하는 것 처럼 new Name(..) 으로 생성된 건에 대해서는 검증이 안될거같아요!
어떻게 수정해보면 좋을까요?

말씀해 주신 것 처럼 기본 생성자로 생성할 경우 검증이 누락되는 문제가 있었습니다.

기본 생성자에 검증 메서드를 호출하도록 하고, 정적 메서드에서 기본 생성자를 호출해 반환하도록 수정하였습니다.

좋습니다~ record에서는 기본생성자에 대한 검증을 다음과 같이 사용할 수 있습니다

public Name { validate(value); }

그런데 이 수정으로 Name.from()은 단순히 new Name()을 감싸기만 하게 되었는데, 위에서 구구가 남긴 팩터리 메서드 관련 코멘트와 같은 고민을 Name에서도 동일하게 해볼 수 있을 것 같아요!

#1137 (comment)

record의 기본생성자 방식에 대해서는 새로 알았습니다!

사실 Name.from을 제거하려 했으나 테스트 메서드에서 많이 사용되고 있어서 new Name()으로 변경하는 것이 번거로워 보류한 상태였습니다.

다만 다시 방법을 찾아보니 인텔리제이에 변경해주는 기능이 존재하는 것을 알게되어, 팩토리 메서드를 제거하였습니다.

Choi-JJunho · 2026-03-18T12:58:32Z

+        for (int i = 0; i < count; i++) {
+            drawnCards.add(cards.getLast());
+            cards.removeLast();
+        }


count가 60이 넘어가는 등 count가 비정상적으로 커지면 여기에서는 어떤 문제가 발생할까요?

말씀하신 상황처럼 Deck에 남은 카드보다 많은 수의 카드를 요청할 경우, 예외가 발생하며 비정상적인 종료가 될 위험이 있습니다. 이를 방지하기 위해 카드를 실제로 뽑기 전 수량이 충분한지 검증하는 과정(ValidateCardsCount)를 추가했습니다.

좋습니다! 방어 코드를 잘 추가해주셨네요 👍

한 가지 더 이야기해보자면 예외 메시지가 비어있는 부분을 한번 볼 수 있을 것 같아요

private void validateCardsCount(int drawCount) { if (cards.size() < drawCount) { throw new IllegalArgumentException(); // 예외메시지가 비어있음. }

에러가 났을 때 디버깅을 손쉽게하려면 에러에 의미 있는 메시지를 담아주는 것이 좋습니다!
예를 들어 요청한 카드 수와 남은 카드 수를 메시지에 포함하면 문제 원인을 더 빠르게 파악할 수 있을 것 같아요 👍

추가로 카드를 제거하는 로직은 이미 있는 draw 메서드를 활용해보면 좋을 것 같아요 :)

public List<Card> draw(int count) { validateCardsCount(count); List<Card> drawnCards = new ArrayList<>(); for (int i = 0; i < count; i++) { drawnCards.add(draw()); } return List.copyOf(drawnCards); }

예외 메세지를 추가하고 darw()를 재사용하도록 수정했습니다.

Choi-JJunho · 2026-03-18T13:19:20Z

@@ -11,6 +12,10 @@ public class Players implements Iterable<Player> {
    private static final int MAX_PLAYER_COUNT = 7;
    private final List<Player> players;


Player가 배팅 금액을 직접 가지는 것보다, BettingTable이나 PlayerBet 같은 별도 객체가 관리하는 쪽이 더 좋은걸까요?
PlayerBet이 Player 자체를 들기보다 Name과 BettingAmount만 가지도록 분리하는 방향은 적절한지도 궁금합니다.

각각의 상황마다 장단점이 있어보이네요~!
정리해서 이야기해보면 다음과 같이 이야기해볼 수 있곘군요

Player가 배팅 금액을 직접 가지기 vs 분리하기

플레이어가 가진다면? 직관성과 단순함을 챙길 수 있을 것 같네요

분리한다면? 단일 책임 원칙(SRP)을 지키는 시도를 해볼 수 있을 것 같아요

PlayerBet이 Player 참조 vs Name 참조

플레이어를 참조하면? 최종 결산 때 결과와 배팅 금액을 조합하기 수월하겠네요!

이름을 참조하면? 객체의 결합도를 낮출 수 있겠네요

이번 미션에서 관리해야 할 상태의 복잡도를 고려해 보고, 나무가 생각했을 때는 어떤 방법이 더 타당할것 같은지 이야기를 들려주세요!

위의 제 의견은 단순 참고용일뿐이니 나무의 생각과 근거를 덧붙여서 아래 코멘트를 남겨주세요

Player가 배팅 금액을 직접 가지기 vs 분리하기
저는 배팅 금액을 Player에 직접 포함하기보다 분리하는 쪽이 더 타당하다고 판단했습니다
배팅 금액은 플레이어 자체의 책임이라기보다, 플레이어와 연관된 별도의 정보라고 생각했습니다. 분리하면 게임을 다시 진행하거나 배팅 관련 규칙이 변경되는 경우에도 Player는 유지한 채 배팅 정보만 새로 생성할 수 있어 더 대응하기 더 좋을 것이라 생각했습니다.

PlayerBet이 Player 참조 vs Name 참조
PlayerBet이 필요한 정보는 Player 전체가 아니라 Player를 식별할 수 있는 최소한의 정보와 배팅 금액이라고 판단했습니다. 그래서 Player를 직접 참조하기보다 Name을 참조하도록 했습니다. Player를 참조하고 있으면 Player의 배팅과 무관한 다른 상태나 행위에도 접근이 가능해져 결합도가 높아질 수 있다고 생각했습니다.
String과 Name사이에서도 고민을 했었는데, 이미 플레이어를 식별하는 Name이 존재하므로 PlayerBet에서도 Name을 사용하는 편이 일관성을 유지하는 데 더 적절하다고 판단했습니다.

각 선택지에 대한 근거를 명확하게 정리해주셨네요

나무가 선택해주신 근거에 충분히 납득이갑니다 👍
게임을 다시 진행하거나 배팅 관련 규칙이 변경되는 경우에도 Player는 유지한 채 배팅 정보만 새로 생성할 수 있어
위 부분이 좋은 근거로 다가왔다고 생각해요

아래 PlayerBet이 Player 참조 vs Name 참조 부분에서 한가지만 생각해보면 좋을 것 같은게 플레이어를 식별하는 Name이 존재한다고 말씀해주셨는데 동명이인의 경우 어떻게 처리될까요?

현재는 Players의 생성 과정에서 중복 이름을 허용하지 않고 있고, Participant의 동등성도 Name 기준으로 판단하고 있어서 Name이 플레이어를 식별하는 역할을 수행할 수 있습니다. 따라서 PlayerBet이 Player 전체 대신 Name과 BettingAmount만 가지는 방향이 가능하다고 판단했습니다.

그런데 언급하신 부분을 보고 생각해보니, Name을 식별자로 사용할 수 있는가?는 결국 이름의 중복을 어떻게 처리할 지에 달려 있다고 생각이 됐습니다.

만약 요구사항이 바뀌어 동명이인을 허용하게 된다면 Name만으로는 식별이 어려워지므로, PlayerId 같은 별도의 식별자가 필요해질 것 같습니다.

Choi-JJunho · 2026-03-18T13:30:07Z

 - [x] 안내 메세지 출력 후 참여할 플레이어 이름들을 입력을 받는다
    - [x] 널값이나 빈 공백 예외처리
    - [x] 쉼표 기준으로 입력 분리
-     


고민이 얕으면 질문이 너무 가볍고, 고민이 깊으면 오히려 꼬리 질문이 계속 생겨 초기 의문의 방향을 잃어버리곤 했습니다. 사실 이 문제 때문에 더욱 정신없는 미션 진행이 되었던 것 같습니다. 쏟아지는 새로운 개념과 새롭게 보이는 문제점들을 감당하기 어려웠던 것 같습니다.
주노는 이런 상황에서 질문의 적절한 깊이와 타이밍을 잡는 기준으로 있는 지 여쭤보고 싶습니다.

이전 미션에서 제가 너무 많은 질문을 드렸던거같네요 😅

질문의 타이밍에 대한 제 기준을 말씀드리면, 내가 이해한 것, 시도해 본 것, 막힌 지점이 스스로 명확해졌을 때가 가장 좋은 타이밍이라고 생각해요. ㅎㅎ

지금의 질문으로는 나무가 구체적으로 언제, 어떤 어려움을 겪으셨는지 파악하기가 조금 어렵네요 🥹
이번에 겪으셨던 어려움 중 하나를 골라 위 세 가지 기준으로 정리해서 다시 들려주시면 더 좋은 방향을 함께 고민해 볼 수 있을 것 같아요!

symflee

리뷰 해주신 부분들 반영하여 일부 수정하였습니다.
이번에도 잘 부탁드립니다.

symflee · 2026-03-20T04:28:10Z

+    private void readyPhase(Participants participants) {
+        gameManager.dealInitialCards(participants);
+
+        InitialDealingResult initialDealingResult = InitialDealingResult.from(participants);
+        outputView.printInitialDealingResult(initialDealingResult);
    }


기존에는 초기 카드 지급과 출력용 데이터 조립을 하나의 클래스가 담당하고 있었습니다. 이를 초기 카드 지급은 GameManager가, 출력용 데이터 생성은 별도의 객체가 담당하도록 분리했습니다.

그 결과 게임 흐름 제어에 더 집중할 수 있게 되었고, 초기 카드 지급 규칙도 한 곳에 모여 응집도가 높아졌습니다. 또한 dealInitialCards(participants)처럼 가독성도 좋아졌다고 생각합니다.

symflee · 2026-03-20T05:19:50Z

@@ -11,6 +12,10 @@ public class Players implements Iterable<Player> {
    private static final int MAX_PLAYER_COUNT = 7;
    private final List<Player> players;


Player가 배팅 금액을 직접 가지기 vs 분리하기
저는 배팅 금액을 Player에 직접 포함하기보다 분리하는 쪽이 더 타당하다고 판단했습니다
배팅 금액은 플레이어 자체의 책임이라기보다, 플레이어와 연관된 별도의 정보라고 생각했습니다. 분리하면 게임을 다시 진행하거나 배팅 관련 규칙이 변경되는 경우에도 Player는 유지한 채 배팅 정보만 새로 생성할 수 있어 더 대응하기 더 좋을 것이라 생각했습니다.

PlayerBet이 Player 참조 vs Name 참조
PlayerBet이 필요한 정보는 Player 전체가 아니라 Player를 식별할 수 있는 최소한의 정보와 배팅 금액이라고 판단했습니다. 그래서 Player를 직접 참조하기보다 Name을 참조하도록 했습니다. Player를 참조하고 있으면 Player의 배팅과 무관한 다른 상태나 행위에도 접근이 가능해져 결합도가 높아질 수 있다고 생각했습니다.
String과 Name사이에서도 고민을 했었는데, 이미 플레이어를 식별하는 Name이 존재하므로 PlayerBet에서도 Name을 사용하는 편이 일관성을 유지하는 데 더 적절하다고 판단했습니다.

symflee · 2026-03-20T06:08:34Z

+        for (int i = 0; i < count; i++) {
+            drawnCards.add(cards.getLast());
+            cards.removeLast();
+        }


말씀하신 상황처럼 Deck에 남은 카드보다 많은 수의 카드를 요청할 경우, 예외가 발생하며 비정상적인 종료가 될 위험이 있습니다. 이를 방지하기 위해 카드를 실제로 뽑기 전 수량이 충분한지 검증하는 과정(ValidateCardsCount)를 추가했습니다.

symflee · 2026-03-20T11:52:06Z

+
+    private void readyPhase(Dealer dealer, Players players) {
+        gameManager.dealStartingCards(dealer);
+        gameManager.dealCardTo(players, 2);


넵
INITIAL_CARD_COUNT 라는 이름을 부여하도록 수정했습니다.

symflee · 2026-03-20T12:01:18Z

+        return new BettingTable(bettingTable);
+    }
+
+    public void add(PlayerBetting playerBetting) {


게임에서 베팅할 때 사용하는 표현인 'place a bet'에서 메서드명을 place로 수정해 보았습니다.

외부의 placeBet이라는 메서드 안에서 bettingTable.place(playerBet) 형태로 호출하게 되는데 이러한 네이밍이 적절할까요?

symflee · 2026-03-20T12:03:21Z

    private static final int HIT_THRESHOLD = 16;

    public Dealer() {
        super(new Name(DEFAULT_NAME));


말씀해 주신 것 처럼 기본 생성자로 생성할 경우 검증이 누락되는 문제가 있었습니다.

기본 생성자에 검증 메서드를 호출하도록 하고, 정적 메서드에서 기본 생성자를 호출해 반환하도록 수정하였습니다.

symflee · 2026-03-20T12:08:39Z

+
+import java.util.List;
+
+public record ParticipantsProfitResult(


게임이 종료된 이후에 profit이 결정되기 때문에 profit만으로 결과라는 의미를 충분히 전달할 수 있어, result는 붙일 필요가 없다고 생각합니다. 수정하였습니다.

Choi-JJunho

나무~! 리뷰 반영 및 코멘트를 잘 남겨주셨네요!
몇가지 확인해볼만한 부분이 조금 남아있어 Request changes를 남깁니다!
화이팅입니다 💪

Choi-JJunho · 2026-03-21T07:15:08Z

+public class RandomDeckShuffler implements DeckShuffler {
+    @Override
+    public List<Card> shuffle(List<Card> cards) {
+        Collections.shuffle(cards);
+
+        return cards;


Collections.shuffle(cards); 을 수행하고 반환하고있네요!
cards가 immutable이라면 어떤 일이 일어날까요?

UnsupportedOperationException예외가 발생하게됩니다..!

인터페이스 DeckShuffler 는 List<Card> shuffle(List<Card> cards)로 선언되어있습니다. 이 시그니쳐를 보면, 어떠한 List<Card>든 받아서 반환해줄 것으로 기대됩니다.
그러나 실제로는 ‘전달받은 리스트를 직접 변경하기에 mutable을 요구하는’ Collections.shuffle()를 사용하고 있습니다.
즉, 기대되는 동작과 실제 동작의 간극이 존재하는 상황이었습니다.

이를 해결하기 위해서 구현체 RandomDeckShuffler.shuffle(List<Card>)은 이전처럼 전달받은 리스트의 복사본을 만들어 Collections.shuffle()에게 전달하도록 되돌렸습니다.

다만, 되돌리게되면 복사 책임이 불분명하고 중복으로 수행되는 문제가 다시 생기게 됩니다. 따라서, Deck이 자신의 리스트의 복사본을 만들어 전달하던 것을, 복사 비용이 발생하지 않는 unmodifiableList를 이용해 읽기전용 뷰를 넘기도록 하였습니다.

이를통해 이중 복사 문제를 해결하면서, Deck이 자신의 cards를 외부에서 변경 가능한 상태로 넘기지 않도록 수정해보았습니다.

즉, 이전 수정에서 unmodifiableList를 사용했으면 한 번에 해결될 문제였습니다.

제가 이해하고 수정한 것이 의도하신 방향이 맞을까요?

네~! 의도한대로 잘 수정해주셨어요 👏
정리해보자면 제가 드렸던 리뷰에는 다음과 같은 의도가 있었다고 이해해주시면 좋을 것 같아요

Deck → unmodifiableList로 읽기 전용 뷰를 전달 (내부 상태 보호, 불필요한 복사 제거)

RandomDeckShuffler → 전달받은 리스트의 복사본을 만들어 shuffle (어떤 List든 안전하게 처리)

unmodifiableList로 이중 복사 문제까지 잘 해결해주셨네요👍

Choi-JJunho · 2026-03-21T07:28:06Z

+    private void readyPhase(Participants participants) {
+        gameManager.dealInitialCards(participants);
+
+        InitialDealingResult initialDealingResult = InitialDealingResult.from(participants);
+        outputView.printInitialDealingResult(initialDealingResult);
    }


좋습니다~! 분리한 이유와 그에 따른 이점을 잘 설명해주셨네요!
나무가 이야기해주신 dealInitialCards(participants) 형태로 가독성이 좋아진 부분도 동의합니다. 👍

다른 크루들과도 이야기를 나눠보면서 나무가 생각하는 타당한 의견을 견고히 다져가보면 좋을 것 같습니다 💪

Choi-JJunho · 2026-03-21T07:30:41Z

    private static final int HIT_THRESHOLD = 16;

    public Dealer() {
        super(new Name(DEFAULT_NAME));


좋습니다~ record에서는 기본생성자에 대한 검증을 다음과 같이 사용할 수 있습니다

public Name { validate(value); }

그런데 이 수정으로 Name.from()은 단순히 new Name()을 감싸기만 하게 되었는데, 위에서 구구가 남긴 팩터리 메서드 관련 코멘트와 같은 고민을 Name에서도 동일하게 해볼 수 있을 것 같아요!

#1137 (comment)

Choi-JJunho · 2026-03-21T07:32:07Z

+        for (int i = 0; i < count; i++) {
+            drawnCards.add(cards.getLast());
+            cards.removeLast();
+        }


좋습니다! 방어 코드를 잘 추가해주셨네요 👍

한 가지 더 이야기해보자면 예외 메시지가 비어있는 부분을 한번 볼 수 있을 것 같아요

private void validateCardsCount(int drawCount) { if (cards.size() < drawCount) { throw new IllegalArgumentException(); // 예외메시지가 비어있음. }

에러가 났을 때 디버깅을 손쉽게하려면 에러에 의미 있는 메시지를 담아주는 것이 좋습니다!
예를 들어 요청한 카드 수와 남은 카드 수를 메시지에 포함하면 문제 원인을 더 빠르게 파악할 수 있을 것 같아요 👍

Choi-JJunho · 2026-03-21T07:35:45Z

@@ -11,6 +12,10 @@ public class Players implements Iterable<Player> {
    private static final int MAX_PLAYER_COUNT = 7;
    private final List<Player> players;


각 선택지에 대한 근거를 명확하게 정리해주셨네요

나무가 선택해주신 근거에 충분히 납득이갑니다 👍
게임을 다시 진행하거나 배팅 관련 규칙이 변경되는 경우에도 Player는 유지한 채 배팅 정보만 새로 생성할 수 있어
위 부분이 좋은 근거로 다가왔다고 생각해요

아래 PlayerBet이 Player 참조 vs Name 참조 부분에서 한가지만 생각해보면 좋을 것 같은게 플레이어를 식별하는 Name이 존재한다고 말씀해주셨는데 동명이인의 경우 어떻게 처리될까요?

Choi-JJunho · 2026-03-21T07:42:14Z

+    public String name() {
+        return name.value();
+    }
+
+    public Name getName() {
+        return this.name;
+    }


name과 getName의 차이는 무엇인가요?

처음에는 외부(View 등)에서 문자열 이름이 바로 필요한 경우 편하게 사용하려고 name() 메서드를 추가해 두었습니다.
지금은 getName()과 네이밍이 비슷해 호출하는 입장에서 혼란을 줄 수 있고, 원시값을 포장하고 있는 Name의 역할을 흐리게 한다는 것을 깨달았습니다.
Name을 전달하고 Name의 밸류를 꺼내도록 하는 것이 더 좋은 방향일까요?

다만 현재 변경 범위가 커질 것 같아서, name() 메서드는 삭제하고, 문자열 값이 필요한 곳에서는getName().value()형태로 Name 객체를 통해 직접 꺼내 쓰도록 코드를 수정하였습니다.

Choi-JJunho · 2026-03-21T07:43:25Z

+    public List<PlayerMatchResult> playersBettingResult() {
+        List<PlayerMatchResult> playersBettingResult = new ArrayList<>();
+
+        for (Player player : players) {
+            playersBettingResult.add(PlayerMatchResult.from(dealer, player));
+        }
+
+        return playersBettingResult;
+    }


배팅 결과를 계산하는 행위는 Participants의 역할로 적절하다고 생각하시나요?
BettingTable의 역할은 무엇인가요?

플레이어가 네츄럴 블랙잭인지, 승리인지, 무승부인지 등 게임 종료 시 카드에 따른 승패 결과를 BettingResult라는 네이밍으로 사용해 혼동이 있었고 개선을 올바르게 하지 못하였습니다!

Participants가 딜러와 플레이어를 가지고 있고, 딜러와 플레이어가 핸드를 가지고 있으니 Participants가 ENUM 승패 규칙에 딜러와 플레이어의 점수를 넘겨 승패 결과를 받아오는 것은 적절하다고 판단했습니다.

따라서, 네이밍만 실제 게임 용어인 Showdown으로 다시 수정해보았습니다.

좋습니다~! 나무가 말씀해주신 기준으로 잘 납득이 가네요 👍
지금 규모에서는 좋은 구조라고 생각합니다!

지금 규모에서는 괜찮지만 승패 판정 규칙이 복잡해지면 이 로직에 대한 분리를 검토해볼 수 있겠네요 :)

Choi-JJunho · 2026-03-21T07:46:30Z

+        for (int i = 0; i < count; i++) {
+            drawnCards.add(cards.getLast());
+            cards.removeLast();
+        }


추가로 카드를 제거하는 로직은 이미 있는 draw 메서드를 활용해보면 좋을 것 같아요 :)

public List<Card> draw(int count) { validateCardsCount(count); List<Card> drawnCards = new ArrayList<>(); for (int i = 0; i < count; i++) { drawnCards.add(draw()); } return List.copyOf(drawnCards); }

Choi-JJunho

나무~! 블랙잭 미션 완주하시느라 너무 고생 많으셨습니다! 👍
이번 미션을 통해 한 단계 더 크게 성장하신 모습이 돋보여서 리뷰하는 저도 같이 배울점이 많아서 좋았습니다 💪
조급해하지 않고 지금처럼 나무만의 속도로 차근차근 나아가다 보면 빠르게 성장해있을것이라 생각합니다

요구사항과 코멘트 잘 반영해주셔서 이번미션은 여기서 Approve & Merge 하겠습니다.
다음 미션도 화이팅입니다! 🚀

Choi-JJunho · 2026-03-21T13:26:04Z

+public class RandomDeckShuffler implements DeckShuffler {
+    @Override
+    public List<Card> shuffle(List<Card> cards) {
+        Collections.shuffle(cards);
+
+        return cards;


네~! 의도한대로 잘 수정해주셨어요 👏
정리해보자면 제가 드렸던 리뷰에는 다음과 같은 의도가 있었다고 이해해주시면 좋을 것 같아요

Deck → unmodifiableList로 읽기 전용 뷰를 전달 (내부 상태 보호, 불필요한 복사 제거)

RandomDeckShuffler → 전달받은 리스트의 복사본을 만들어 shuffle (어떤 List든 안전하게 처리)

unmodifiableList로 이중 복사 문제까지 잘 해결해주셨네요👍

Choi-JJunho · 2026-03-21T13:29:08Z

+    public List<PlayerMatchResult> playersBettingResult() {
+        List<PlayerMatchResult> playersBettingResult = new ArrayList<>();
+
+        for (Player player : players) {
+            playersBettingResult.add(PlayerMatchResult.from(dealer, player));
+        }
+
+        return playersBettingResult;
+    }


좋습니다~! 나무가 말씀해주신 기준으로 잘 납득이 가네요 👍
지금 규모에서는 좋은 구조라고 생각합니다!

지금 규모에서는 괜찮지만 승패 판정 규칙이 복잡해지면 이 로직에 대한 분리를 검토해볼 수 있겠네요 :)

symflee added 12 commits March 15, 2026 00:34

refactor: 클래스 및 메서드 책임 분리

e7c1d33

refactor: 게임 준비 과정 관여 클래스 및 메서드 책임 분리

96f2fa0

feat: 배팅 금액 유효성 검사 기능 추가

c8f78ee

docs: readme 변경된 출력 예시 반영

159006e

feat: 각 플레이어 별 배팅 금액 기록 추가

b77fe3b

feat: 최종 수익 계산 기능 추가

def8a9c

feat: 최종 수익 출력 기능 추가

f5fbbcd

fix: 내츄럴 블랙잭 판정 오류 수정

6779514

refactor: findFirst를 findAny로 변경 및 스트림 테스트 추가

1209523

refactor: 클래스 패키지 이동

d221a3c

refactor: 셔플 전략 수정

1345a5f

test: 실패하는 테스트 수정

f25fb8b

kang-hyungu requested changes Mar 18, 2026

View reviewed changes

symflee added 4 commits March 18, 2026 16:58

refactor: 사용되지 않는 메서드 제거

bd5839c

refactor: 사용되지 않는 클래스 제거

f00bb1a

refactor: 클래스 패키지 이동

d7798eb

refactor: 클래스와 메서드 네이밍 개선

d5cfec4

symflee commented Mar 18, 2026

View reviewed changes

Comment thread src/main/java/domain/DeckShuffler.java

Comment thread src/main/java/domain/Command.java

Choi-JJunho requested changes Mar 18, 2026

View reviewed changes

symflee added 11 commits March 19, 2026 13:20

refactor(Name): 유효성 검사 로직을 정적 팩토리 메서드에서 생성자로 이동

64c1b5d

test: 카드 지급 테스트 수정

528703e

refactor: 매직넘버 제거

3f827cb

feat: Deck 에서 카드 한장을 뽑을때 카드 잔량 검사 추가

f5fcacb

refactor: draw 메서드 반환 시 방어적 복사 적용

ab89e2e

refactor: 플레이어 턴 진행결과 출력 방법 개선

7029dfc

refactor: 방어적 복사 책임을 Deck만 가지도록 수정

e929361

refactor: 게임 용어에 맞추어 상수 네이밍 변경

ba38047

refactor: 게임 용어에 맞추어 메서드 네이밍 수정:

0b826a8

refactor: 게임 용어에 맞추어 메서드 네이밍 수정:

93d1890

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

25823df

symflee commented Mar 20, 2026

View reviewed changes

refactor: 네이밍 변경

5e113ff

Choi-JJunho requested changes Mar 21, 2026

View reviewed changes

symflee added 7 commits March 21, 2026 18:38

refactor: Deck은 읽기 전용 뷰를 넘기고 Shuffler는 복사본을 섞도록 변경

85f3d40

refactor: 덱에서 카드를 뽑을 때 카드 수량이 부족할 경우 예외 메세지 구체화

b3eebae

refactor: Name의 불필요한 팩토리 메서드 제거

70f3399

refactor: 게임 진행 클래스 네이밍 변경

192357c

refactor: DTO 클래스 및 네이밍 변경

d87efdd

refactor: BettingTable에 존재하는 view 의존 제거

1f4d68b

refactor: name메서드 제거

fa96660

Choi-JJunho approved these changes Mar 21, 2026

View reviewed changes

Choi-JJunho merged commit 76e56ea into woowacourse:symflee Mar 21, 2026

		@@ -28,17 +37,64 @@ public GameController(InputView inputView, OutputView outputView, GameManager ma
		public void run() {


		import java.util.List;

		public record ParticipantsProfitResult(

		@@ -14,7 +14,7 @@ public record CardsSnapshot(List<Card> cards) {

		public String getFormattedCards() {

		@@ -11,6 +12,10 @@ public class Players implements Iterable<Player> {
		private static final int MAX_PLAYER_COUNT = 7;
		private final List<Player> players;

Conversation

symflee commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

체크 리스트

어떤 부분에 집중하여 리뷰해야 할까요?

게임 진행을 Controller에서 어디까지 다루는 것이 허용되는지

배팅 정보를 Player가 직접 가져야 하는지, 별도 객체가 관리해야 하는지

추가 기능 구현과 리팩토링의 우선순위는 어떤 기준으로 정해야할지

질문을 위해서 어디까지 스스로 고민해보아야 하는지

Uh oh!

kang-hyungu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choi-JJunho left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

symflee Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

symflee commented Mar 17, 2026 •

edited

Loading

symflee Mar 20, 2026 •

edited

Loading

symflee Mar 20, 2026 •

edited

Loading