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

haeseonlee · 2026-03-14T06:29:09Z

체크 리스트

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

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

블랙잭 테스트 코드 피드백 반영

step1에서 테스트 코드의 Then 부분에 관한 정보가 부족하여 테스트 의도가 명확히 드러나지 않는다는 피드백을 주셨는데요! 이를 반영하기 위해 기존에 isEqualTo(10000)처럼 상수를 직접 입력하던 방식을 expectedAmount라는 변수를 활용하는 방식으로 개선하였습니다. 이렇게 변수명을 통해 기댓값을 명시하는 것이 피드백을 적절히 반영한 것인지, 의도를 더 명확히 드러낼 수 있는 다른 방식이 필요한지 알고싶습니다.

Player가 수익을 반환하는 메서드를 가지는 것에 대해

이번 미션에 베팅 기능을 추가하면서 고민했던 부분은 최종 수익을 플레이어가 계산해서 알려주는 게 맞을까? 하는 점이었습니다. 배팅 금액을 플레이어가 가지고 있고, 게임에서 이기거나 져서 돈을 따고 잃는 것도 결국 플레이어의 일이라고 생각했습니다. 그래서 플레이어 스스로 자신의 수익이 얼마인지 계산해 반환하도록 만들었는데, 지금처럼 플레이어가 이 역할을 직접 맡아도 괜찮은지 궁금합니다

pci2676

안녕하세요~

빠르게 개선하실 수 있는 부분 위주로 코멘트를 남겨두었으니 확인해주세요~

pci2676 · 2026-03-14T07:05:55Z

src/main/java/blackjack/domain/Money.java

+    private final int bettingMoney;
+
+    public Money(int bettingMoney) {
+        this.bettingMoney = bettingMoney;


Money에 검증 로직이 없어서 음수나 0원으로도 배팅이 가능한 상태인데요. 값 객체로서 자기 자신의 유효성을 보장해주는 게 좋을것 같습니다.

그리고 배팅 금액이 int 타입인데, 21억을 넘는 금액이 입력되면 Integer.parseInt에서 NumberFormatException이 발생할 수 있어요. long 타입도 고려해보시면 좋을것 같습니다~

Money 클래스에 0이나 음수가 입력될 경우 예외를 발생시키도록 수정했습니다!

베팅 금액은 억 단위를 넘어갈 수도 있다는 점을 고려하지 못하고 int 타입을 사용했었는데, 피드백을 반영하여 더 큰 금액도 수용 가능한 long 타입으로 리팩터링을 진행했습니다.

pci2676 · 2026-03-14T07:06:00Z

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

+            return (int) (money.getBettingMoney() * 1.5);
+        }
+        if (isBust() || dealer.isBlackjack() || (calculateTotalScore() < dealer.calculateTotalScore() && !dealer.isBust())) {
+            return -money.getBettingMoney();


이 조건이 한 줄에 너무 많은 판단을 담고 있어서 읽기가 쉽지 않은것 같아요. 변경과 확장에 유리한 구조로 변경하는 방법은 없을까요?

복잡한 조건문을 조금 더 가독성있게 분리하였습니다!

플레이어가 베팅 금액을 돌려받는 경우, 1.5배의 수익을 얻는 경우, 베팅 금액을 잃는 경우를 각각 분리하여 하나의 조건문이 너무 많은 판단을 담지 않도록 리팩터링 했습니다.

pci2676 · 2026-03-14T07:06:06Z

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

+            return money.getBettingMoney();
+        }
+        if (isBlackjack()) {
+            return (int) (money.getBettingMoney() * 1.5);


이전단계에서 리뷰를 안드렸었나 싶은데요
(int) (money.getBettingMoney() * 1.5) 에서 1.5라는 매직 넘버가 등장하고 있는데요. 블랙잭 배당률이라는 도메인 의미를 담은 상수로 표현해보면 어떨까요?

그리고 수익 계산을 Money가 아닌 int로 직접 다루고 있는데, Money 객체에게 곱셈이나 부호 반전 같은 연산을 맡겨보는 건 어떨까요? 지금은 Money가 단순 getter만 제공하고 있어서 원시값 포장의 이점을 충분히 살리지 못하고 있는것 같아요.

매직 넘버 상수화는 이전 리뷰에서 확인 후 리팩터링을 진행하였는데, 이 숫자는 제가 놓친 것 같습니다... 이 부분도 상수화하여 수정하였습니다!

저도 Money 객체를 만들면서 단순히 값만 담고 있다 보니 굳이 필요한 클래스일까 생각했었는데, 이번 리뷰를 반영해서 Money 객체 안에서 곱셈이나 부호 반전 연산을 수행하고 베팅 결과 금액도 Money 객체로 반환하도록 리팩터링했습니다.

pci2676 · 2026-03-14T07:06:13Z

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


+    public int calculateFinalProfit(Participant dealer) {
+        return 0;
+    }


calculateFinalProfit이 추상 클래스에서 기본값 0을 반환하고 있는데요. 이건 의도하신 건가요? 꼭 이래야만 하는걸까요?

추상 클래스에 기본 구현을 두면 하위 클래스에서 오버라이드를 깜빡해도 컴파일 에러가 나지 않아서 실수를 잡기 어려워질 수 있어요. judgeResult도 마찬가지인데요, 빈 HashMap을 반환하는 기본 구현이 있으면 오버라이드를 빠뜨려도 조용히 넘어가게 됩니다.

기본메서드로 제공해야할때와 추상메서드로 제공해야할때를 어떻게 구분하셨나요?

pci2676 · 2026-03-14T07:06:23Z

src/main/java/blackjack/controller/BlackjackController.java

-
-        for (Entry<Participant, ScoreCompareResult> entry : playerResult.entrySet()) {
-            playerNameResult.put(entry.getKey().getName(), entry.getValue());
+    public void calculateFinalGameProfit(List<Participant> players, Participant dealer) {


calculateFinalGameProfit이 public으로 선언되어 있네요~

그리고 HashMap을 playersProfit의 구체 타입으로 선언하고 있는데, Map 인터페이스로 선언하는 것과 어떤 차이가 있을까요?

이 부분도 private으로 설정되어있는줄 알았습니다.. 주의하겠습니다!

playersProfit의 타입도 인터페이스로 선언하도록 변경하였습니다. Map 인터페이스를 사용하면 나중에 구현체를 바꾸더라도 실제 사용하는 코드는 고칠 필요가 없어서 훨씬 유연하다는 점을 배웠습니다. 인터페이스를 활용하는 이유를 다시 한번 생각해보게 된 것 같습니다!

pci2676 · 2026-03-14T07:06:27Z

src/test/java/blackjackTest/service/BlackjackTest.java

+        pobi.receiveOneCard(new Card(Rank.ACE, Shape.HEART));
+        pobi.receiveOneCard(new Card(Rank.TEN, Shape.SPADE));
+
+        assertThat(pobi.isBlackjack()).isEqualTo(true);


assertThat(pobi.isBlackjack()).isTrue() 같은 것 도 있습니다 그냥 참고만해주세요

pci2676 · 2026-03-14T07:06:35Z

src/test/java/blackjackTest/service/BlackjackTest.java

+        pobi.receiveOneCard(new Card(Rank.ACE, Shape.SPADE));
+        dealer.receiveOneCard(new Card(Rank.ACE, Shape.CLOVER));
+        dealer.receiveOneCard(new Card(Rank.QUEEN, Shape.SPADE));
+


앗.. 제가 리뷰드렸던건 테스트의 이름에 해당하는 부분이었습니다.

이름에서 의도가 잘 드러나지 않아서요 😓

그리고 지금도 이름에서 의도가 잘 드러나지 않는경우들이 보이는것 같습니다.

그리고 누락된 테스트 케이스가 몇 가지 보이는데요:

딜러가 버스트일 때 플레이어가 이기는 경우

플레이어와 딜러 점수가 동일한 경우 (무승부)

플레이어 점수가 딜러보다 낮은 경우

수익 계산이 핵심 비즈니스 로직인 만큼 경계값 테스트를 촘촘하게 작성해보시면 좋을것 같습니다~

아 테스트 이름이 조금 더 명확해야 하는군요ㅠㅠ 그동안 테스트 이름까지는 크게 신경 쓰지 않았는데, 앞으로는 더 주의를 기울여서 명명해 보겠습니다.

경계값 테스트에 대해서는 미처 생각하지 못하고 단순히 수익을 얻고 잃는지에 대해서만 집중했던 것 같습니다. 피드백 주신대로 놓쳤던 부분들을 인지하게 되었고, 말씀하신 사항들에 대해 추가 테스트를 작성했습니다!

pci2676 · 2026-03-14T07:06:46Z

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

 public abstract class Participant {

-    protected final String name;
+    private static final int BLACKJACK_SCORE = 21;


BLACKJACK_SCORE와 BLACKJACK_SIZE가 Participant와 Cards 양쪽에 중복으로 선언되어 있는데요. 이전에 상수가 가장 의미를 잘 드러낼 수 있는 곳에 위치하는 게 응집도를 높이는 방법이라고 이야기 나눴었는데요~

블랙잭 점수 21이라는 개념은 카드 합계를 다루는 Cards가 가장 잘 알고 있는 값이 아닐까요? 한쪽에서 관리하고 필요한 곳에서는 메서드, 혹은 변수 참조를 통해 물어보는 방식을 고려해보시면 좋을것 같습니다.

pci2676 · 2026-03-14T07:06:50Z

src/test/java/blackjackTest/service/GameTest.java

-        Player player = new Player(name);
+        Player player = new Player(name, new Money(10000));
        for (String card : cards) {
            String[] parts = card.split(":");


헬퍼 메서드 안에서 for 반복문을 사용하고 있는데요. 헬퍼 메서드의 반복문 정도는 크게 문제되지 않지만, card.split(":")으로 파싱하는 부분이 테스트 코드의 가독성을 조금 떨어뜨리는것 같아요.

junit을 사용하시면 테스트를 도와주는 여러 도구들이 있는데 이를 살펴보시면 좋을것 같네요

https://docs.junit.org/6.0.3/writing-tests/parameterized-classes-and-tests.html

pci2676 · 2026-03-14T07:07:00Z

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


-    public String getName() {
-        return name;
+    @Override


리뷰 요청 주신 두 번째 질문에 대한 의견인데요, 플레이어가 자신의 수익을 계산하는 것 자체는 나쁘지 않다고 생각합니다. 배팅 금액을 가지고 있고, 자기 카드 상태도 알고 있으니까요.

다만 지금 구조에서는 dealer를 파라미터로 받아서 딜러의 상태를 직접 조회하고 있는데요. 이러면 플레이어가 딜러의 내부 상태(블랙잭 여부, 버스트 여부, 점수)를 너무 많이 알아야 하는 구조가 됩니다.

반대로 딜러에게 "이 플레이어와 비교해서 결과를 알려줘"라고 물어보고, 그 결과를 바탕으로 수익을 계산하는 방식은 어떨까요? 이전 단계에서 dealer.judgeResult로 승패를 판정하는 구조를 만들어두셨는데, 이걸 활용해볼 수 있을것 같아요~

haeseonlee added 30 commits March 12, 2026 15:34

refactor: 불필요한 import문 삭제

42b0ddd

refactor: Player 불필요한 필드 & 메서드 삭제

1efb71c

docs: 배팅 추가 기능 목록 작성

bfcb4f2

refactor: printDealerCardDrawnResult 출력 문구 수정

85f5769

feat: 플레이어 배팅 금액 입력 기능 구현

fca8b22

docs: 추가 기능 구현 목록 수정

af2d198

refactor: 배팅 금액 입력 타입 변경

1c0939a

feat: 플레이어 배팅 금액 저장

318db5b

feat: 배팅 금액 입력값 검증

3235007

test: 플레이어 초기 카드 2장 합이 21인지 판단하는 메서드 테스트

ebd112a

test: 플레이어 초기 카드 2장 합이 21이 아닌 테스트 케이스 추가

b0cf709

feat: 플레이어 초기 카드 2장 합이 21인지 판단하는 기능 구현

11bb5f7

test: 플레이어 초기 카드 2장 합이 21이면 배팅 금액의 1.5배 지급 테스트

ce7705a

feat: 플레이어 초기 카드 2장 합이 21이면 배팅 금액의 1.5배 지급 기능 구현

b3e49dc

test: 플레이어 카드 합 21 넘어가면 배팅 금액 잃는 기능 테스트 구현

2e3e0f2

test: 딜러 초기 카드합 21이면 배팅 금액 잃는 테스트 케이스 추가

e4824d7

feat: 플레이어가 배팅 금액을 잃는 경우 구현

1b492ba

docs: 플레이어가 배팅 금액을 그대로 받는 경우 목록 추가

9be7e57

test: 플레이어와 딜러 모두 초기 카드합 21인 경우 배팅 금액 돌려받는 기능 테스트 구현

414e765

feat: 플레이어와 딜러 모두 초기 카드합 21인 경우 배팅 금액 돌려받는 기능 구현

081cb1c

test: 플레이어와 딜러보다 카드 점수 합이 크면 배팅 금액 돌려받는 기능 테스트

61ef165

feat: 플레이어와 딜러보다 카드 점수 합이 크면 배팅 금액 돌려받는 기능 구현

0f1a896

feat: 최종 수익 결과 출력 구현

9f2eba0

refactor: 수익 결과 계산 분기 로직 리팩터링

ca323a5

docs: 기능 목록 띄어쓰기 수정

81dbce6

refactor: Participant 이름 String에서 Name 객체로 변경

ca3350e

refactor: Participant 클래서 메서드 위치 조정 및 상수 설정

72d918e

refactor: addCard 제약 사항 설정 및 getCardNames stream 사용 리팩터링

9d4143d

refactor: BlackjackController의 calculateFinalScore 메서드 stream 사용 리팩터링

2d0dfb1

test: BlackjackTest 코드 매직넘버 제거

7caf5d0

pci2676 changed the title ~~[🚀 사이클1 - 미션 (블랙잭 베팅)] 주니 미션 제출합니다.~~ [🚀 사이클2 - 미션 (블랙잭 베팅)] 주니 미션 제출합니다. Mar 14, 2026

pci2676 requested changes Mar 14, 2026

View reviewed changes

Conversation

haeseonlee commented Mar 14, 2026

체크 리스트

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

블랙잭 테스트 코드 피드백 반영

Player가 수익을 반환하는 메서드를 가지는 것에 대해

Uh oh!

pci2676 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!

haeseonlee Mar 15, 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

haeseonlee Mar 15, 2026 •

edited

Loading