🚀 사이클1 - 미션 (블랙잭 게임 실행) 와이제리 미션 제출합니다. by yj9107v · Pull Request #999 · woowacourse/java-blackjack

yj9107v · 2026-03-07T14:29:27Z

체크 리스트

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

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

테스트 단위에 대한 명확한 기준
TDD의 구현 설계

1️⃣ 올바른 TDD 구현이란?

제가 생각한 TDD는 기능을 구현하기 전에 테스트 코드를 먼저 작성해 안정성을 높이는 것입니다. 또한 Red–Green–Blue 단계로, 테스트가 실패하는 코드를 먼저 만든 뒤 → 테스트를 통과하도록 구현하고(테스트 코드에서 구현함) → 리팩토링한 후(테스트 코드에서 리팩토링) → 실제 기능을 구현하는 흐름으로 이해했습니다.

정답이 있는지는 모르겠지만, 이러한 방식이 올바른 TDD 구현인지 궁금합니다.

2️⃣ TDD의 우선순위

기능 요구 사항을 읽고, 프로그램의 흐름에 맞춰 기능 목록을 작성했습니다.

이후 TDD를 구현하기 위해 기초 도메인 설계가 필요하다고 판단해 블랙잭 덱(Cards)을 만들고 시작했고, 이 과정에서 프로그램 흐름에 맞춰 TDD를 작성하기보다는 블랙잭의 핵심 기능인 카드 합계와 ACE 1/11 처리에 대한 TDD를 먼저 작성했습니다.

그런데 카드 합계를 테스트하기 위해 Player를 만들게 되었고, Player에 draw() 기능이 추가되면서 TDD를 작성하려다 보니 Player의 TDD를 놓치게 되는 악순환이 반복되었습니다.

첫 TDD를 구현하며 느낀 점은, 프로그램의 흐름에 맞춰 TDD를 구현하는 것이 올바른 우선순위가 될 수 있겠다는 것입니다. 제 의견에 대해 어떻게 생각하시는지 궁금합니다!

3️⃣ 테스트 단위

이번 페어 프로그래밍에서 가장 고민했고 어려웠던 부분은 테스트 단위였습니다.

테스트 단위 규칙 중 하나로 “비즈니스 규칙이 들어간 메서드 단위로 테스트한다”를 정했지만, 그럼에도 혼란이 있었습니다.

예를 들어 비즈니스 출력을 위해 플레이어 이름이나 플레이어가 보유한 카드들을 저장한 List 반환값을 테스트해야 하는지, 플레이어가 카드 2장을 draw() 했을 때 카드를 올바르게 받았는지도 테스트해야 하는지 등 하나하나 의문이 생기다 보니 테스트 코드를 작성하는 과정이 매우 혼란스러웠습니다.

테스트를 해야 하는 명확한 기준이 있을까요? 테스트 제외 규칙은 입출력 관련 메서드로 정했는데, 이런 규칙들이 혼합되면서 오히려 무엇을 테스트해야 하는지 더 헷갈리게 되었습니다. 리뷰어님의 테스트 기준이 궁금합니다!

InputParser 클래스 추가 구분자(',') 기준 입력 값 분리 기능 구현

- 사용자 이름 입력 값이 공백인 경우 예외 처리 테스트

- 사용자 이름 입력 값이 공백인 경우 예외 처리

- Suit: 카드의 모양을 정의 - Rank: 카드의 숫자를 정의

- 정적 팩토리 메서드 생성 - 카드의 정보를 반환하는 메서드 getName() 생성

- 정적 팩토리 메서드 생성 - 카드 덱을 생성하는 메서드 generateCards() 생성

- 정적 팩토리 메서드 생성 - 카드를 추가하는 메서드 addCard() 생성 - player 점수 합계를 계산하는 메서드 calculateScore() 생성

- 플레이어 점수 합계가 21초과 시 ACE 포함한 경우 점수 보정 테스트

- 플레이어 점수 합계가 21초과 시 ACE 포함한 경우 점수 보정 메서드 isBustWithAce()

- 2장 드로우 drawInitialHand() 추가 - 1장 드로우 draw() 추가

- 카드 배분 기능 추가

- shuffle() 메서드 생성

- 정적 팩토리 메서드 - 점수 계산 메서드 calculateScore() - ace 판정 전 점수 계산 메서드 calculateRawScore() - ace 관련 메서드 countAce(), adjustForAce(), isBustWithAce() - 딜러 카드 정보 반환 메서드 getDealerInfo(), getDealerInitialInfo()

- name 필드 추가 - 점수 계산 로직 calculateScore() 수정 - 정보 반환 메서드 getPlayerInfo(), getCardsInfo()

- 플레이어들의 정보 반환 메서드 getUserNames(), getPlayerInfo()

- 사용자들에게 카드 추가 여부 기능 완성

- 플레이어의 카드가 1장이 추가되어 총 3장이 되었는지

- list의 size()를 직접 호출하지 않고, 크기를 비교할 수 있게 수정

- 코드 작성 위치 해결 (테스트 코드는 Red, Green blue는 프로덕션 코드 - list의 size()를 직접 호출하지 않고, 크기를 비교할 수 있게 수정, isEqualTo() -> hasSize() 수정

- 딜러 카드 추가 조건문을 Controller에서 도메인으로 옮김

- 플레이어 합계 21 초과 여부 - 플레이의 보유한 카드 종류가 올바른지 테스트 - 플레이어 점수가 21 초과인 경우 점수 처리 테스트

- 출력 format은 Controller 단에서 책임지게 코드 수정

verus-j

안녕하세요 와이제리~
테스트 코드 정리 아주 잘하셨네요! 이번 피드백에는 프로덕션 코드 위주로 남겨드렸습니다. 확인부탁드려요~

src/test/java/utils/InputParserTest.java

src/test/java/domain/BlackjackResultTest.java

src/main/java/domain/Dealer.java

verus-j · 2026-03-10T04:58:36Z

src/main/java/domain/Policy.java

+public class Policy {
+
+    public static final int FIRST_DRAW_SIZE = 2;
+    public static final int DEALER_HIT_THRESHOLD = 16;
+    public static final int BUST_THRESHOLD = 21;
+    public static final int ACE_HIGH_LOW_DIFF = 10;
+    public static final int BUST_SCORE = 0;
+}


전역 변수를 지정하셨군요. 객체 지향적인 코드를 위해 각 전역 변수를 연관된 객체로 옮겨서 전역 변수를 제거해보면 좋을 것 같습니다.

전역 변수를 지정하셨군요. 객체 지향적인 코드를 위해 각 전역 변수를 연관된 객체로 옮겨서 전역 변수를 제거해보면 좋을 것 같습니다.

연관된 객체로 옮겨 전역 변수를 제거하도록 수정했습니다.

딜러와 플레이어가 둘 다 사용하는 변수가 있었는데, 이전에 말씀해주신 Rule of Three를 떠올려 중복이 두 번 정도인 경우는 별도로 추출하지 않았습니다. 이런 판단이 괜찮을지 궁금합니다.

만약에 딜러와 플레이어가 별도로 Bust 조건이 변경된다면 지금처럼 중복을 제거하지 않는게 좋겠네요. 다른 케이스로 딜러와 플레이어가 같이 Bust 조건이 변경된다면 중복된 코드로 인해 함께 까먹지 않고 수정을 해야겠군요. 블랙잭 게임의 규칙이 어느정도 안정된 게임이 아닌 매주 기획자에 의해 변경된다면 어느 방향으로 진행될지 예측을 할 수 없을 것 같네요.
2가지 방향의 변경점을 고려했을 때 두 번 정도의 중복을 유지하는게 좋을까요? 아니면 중복을 제거하는게 좋을까요? 개인적으로는 이정도면 테스트 케이스도 충분하고 간단한 케이스니 중복을 유지하는 것도 좋은 방향이라고 생각합니다. 현재는 중복으로 인한 관리 비용을 충분히 감당할 수 있기 때문이라고 생각해요.
판단이 명확히 안 설 경우에는 Rule of Three를 떠올리며 적용하는 것도 좋지만, 한번씩은 Rule of Three를 지켜야하는 근거를 생각해보는 것도 좋은 훈련이 될 것 같아요~

src/main/java/domain/BlackjackResult.java

verus-j · 2026-03-10T05:12:43Z

src/test/java/domain/PlayerTest.java

+        List<String> expect = List.of(Rank.TEN.getName() + Suit.DIAMOND.getName(),
+                Rank.FIVE.getName() + Suit.CLOVER.getName());
+        assertThat(player.getCardsInfo()).isEqualTo(expect);


검증문을 살펴보니 문자열로 비교하고 있네요. 해당 문자열은 뷰에 가까운 정보인 것 같은데, Card 리스트 혹은 Rank, Suit로 비교하면 어떨까요?

검증문을 살펴보니 문자열로 비교하고 있네요. 해당 문자열은 뷰에 가까운 정보인 것 같은데, Card 리스트 혹은 Rank, Suit로 비교하면 어떨까요?

플레이어의 필드 값을 getter로 꺼내는 것을 지양하려고 했는데, 그 과정에서 도메인에 뷰와 연관된 로직이 들어가게 된 것 같습니다.

그래서 제 수정 방안으로는, 도메인과 뷰가 연관되는 것보다는 getter 지양을 일부 포기하는 트레이드오프를 선택하는 편이 낫다고 생각했습니다. 혹시 두 가지를 모두 포기하지 않는 방법이 있을까요?

우선은 getter를 사용해 Card 리스트로 비교하도록 수정했습니다.

이미 getter를 프로덕션 코드(Controller)에서 사용하고 있기 때문에 getter를 사용해도 된다고 생각합니다. 만약에 프로덕션 코드에도 getter가 없어 테스트 코드에서만 사용된다면 조금 번거롭지만 equals&hashcode를 오버라이드해서 isEqualTo로 검증하는 것도 방법이 되겠네요.

src/test/java/domain/PlayersTest.java

src/test/java/domain/DealerTest.java

- 도메인과 뷰의 연관성을 없앰

- 도메인과 뷰의 연관성을 없앰 - 카드의 확인을 사이즈로 하지 않고 Card 리스트로 테스트

- 플레이어들의 점수를 size로 테스트하지 않고, List<Integer>로 결과 값 테스트 수정

- Policy 상수화 옮김 - count 반환 값 삭제 - 접근지정자 public -> private 수정

- 추상화 수준을 높이기 위해 shuffled() 메서드 추가

- Player의 뷰와 연관된getCardsInfo() 호출하지 않고 getCards() 호출로 수정

- isBust() private -> public 변경 - getScoreZeroIfBust() 제거

- 플레이어가 버스트인 경우 딜러 점수와 상관없이 플레이어 패배 - 뷰 연관된 메서드를 제거 후 필드 getter로 변경

verus-j

안녕하세요 와이제리~
피드백 반영하느라 고생하셨습니다. 추가 피드백 남겨뒀으니 확인부탁드려요!

verus-j · 2026-03-11T12:15:31Z

src/main/java/domain/Cards.java

+
+public class Cards {
+
+    public static final int FIRST_DRAW_SIZE = 2;


보니깐 Controller에서 사용중이네요. FIRST_DRAW_SIZE를 private으로 바꿀 수 있는 구조가 있을지 고민해보면 좋을 것 같아요. 초기 손패 크기에 대한 정보가 public으로 유지해야한다면, 초기 손패와 관련된 로직이 이곳저곳에 흩어져 있을 가능성이 큰 것 같습니다.

verus-j · 2026-03-11T12:17:01Z

src/main/java/domain/BlackjackResult.java

+        return new BlackjackResult(dealer, players);
+    }
+
+    public void compareResult(Dealer dealer, Players players) {


접근 제어자 수정이 필요해보입니다~

verus-j · 2026-03-11T12:18:34Z

src/main/java/domain/BlackjackResult.java

+            playerResult.put(player, WinOrLose.LOSE);
+            return;
+        }
+        playerResult.put(player, WinOrLose.DRAW);


DRAW가 있는데 WinOrLose가 올바른 enum 클래스 이름일까요?

verus-j · 2026-03-11T12:18:59Z

src/main/java/domain/BlackjackResult.java

+        }
+    }
+
+    private void getJudgeResult(Dealer dealer, Player player) {


몇몇 부분 테스트 케이스가 부족합니다! 확인부탁드려요~

verus-j · 2026-03-11T12:19:56Z

src/main/java/domain/Player.java

+    public static final int ACE_HIGH_LOW_DIFF = 10;
+    public static final int BUST_THRESHOLD = 21;


접근제어자 확인 부탁드립니다~

verus-j · 2026-03-11T12:20:12Z

src/main/java/domain/Dealer.java

+    public static final int DEALER_HIT_THRESHOLD = 16;
+    public static final int ACE_HIGH_LOW_DIFF = 10;
+    public static final int BUST_THRESHOLD = 21;


접근제어자 확인 부탁드려요~

verus-j · 2026-03-11T12:21:43Z

src/test/java/domain/BlackjackResultTest.java

+    private final Cards deck = Cards.of();
+    private final List<Card> initialCards = new ArrayList<>(List.of(
+            Card.of(Rank.ACE, Suit.DIAMOND),
+            Card.of(Rank.FOUR, Suit.CLOVER)));
+
+    private final List<String> userNames = List.of("pobi", "jason");
+    private final Players players = Players.of(deck, userNames);
+    private final Dealer dealer = Dealer.of(initialCards);
+    private final BlackjackResult blackjackResult = BlackjackResult.of(dealer, players);


given 절이 여기에 다 있어서 테스트 케이스를 읽을 때 이해가 어려울 것 같습니다. 어떻게 하면 중복된 로직을 제거하면서 가독성 있는 테스트 케이스를 작성할 수 있을지 고민해보면 좋을 것 같아요~

verus-j · 2026-03-11T12:23:47Z

src/test/java/domain/DealerTest.java

+    @Test
+    @DisplayName("딜러 합계가 16이하이면 카드 1장 추가 테스트")
+    void add_one_card_test() {
+        List<Card> initialCards = new ArrayList<>(List.of(
+                Card.of(Rank.ACE, Suit.DIAMOND),
+                Card.of(Rank.FOUR, Suit.CLOVER)));
+        Dealer dealer = Dealer.of(initialCards);
+
+        dealer.drawUntilHit(deck);
+
+        assertThat(dealer.getCards()).hasSize(3);
+    }


다음에 뽑은 카드가 ACE이면 4장까지 갈 수 있지 않을까요?

verus-j · 2026-03-11T12:32:56Z

src/test/java/domain/PlayerTest.java

+        List<String> expect = List.of(Rank.TEN.getName() + Suit.DIAMOND.getName(),
+                Rank.FIVE.getName() + Suit.CLOVER.getName());
+        assertThat(player.getCardsInfo()).isEqualTo(expect);


이미 getter를 프로덕션 코드(Controller)에서 사용하고 있기 때문에 getter를 사용해도 된다고 생각합니다. 만약에 프로덕션 코드에도 getter가 없어 테스트 코드에서만 사용된다면 조금 번거롭지만 equals&hashcode를 오버라이드해서 isEqualTo로 검증하는 것도 방법이 되겠네요.

yj9107v added 30 commits March 5, 2026 10:48

docs: 기능 목록 초안 작성

1758c9f

test(InputParserTest): 입력 값 파싱 로직 TDD 검증 테스트

91bb4ef

feat(InputParser): 입력 값 분리 기능 추가

8585290

InputParser 클래스 추가 구분자(',') 기준 입력 값 분리 기능 구현

test(InputParserTest): 예외 기능 추가

ab09a1c

- 사용자 이름 입력 값이 공백인 경우 예외 처리 테스트

feat(InputParser): 예외 기능 추가

6a333ec

- 사용자 이름 입력 값이 공백인 경우 예외 처리

feat(Rank, Suit): enum 클래스 생성

bd60764

- Suit: 카드의 모양을 정의 - Rank: 카드의 숫자를 정의

feat(Card): 모양과 숫자를 가진 카드 객체 생성

e401f8b

- 정적 팩토리 메서드 생성 - 카드의 정보를 반환하는 메서드 getName() 생성

feat(Cards): Card의 일급 컬랙션

c58103c

- 정적 팩토리 메서드 생성 - 카드 덱을 생성하는 메서드 generateCards() 생성

refactor(Card): 생성자 매개변수 순서 변경

818c9d0

test(PlayerTest): 플레이어 점수 합계 테스트 작성

d3268ee

feat(Player): player 객체 생성

ccaaf4b

- 정적 팩토리 메서드 생성 - 카드를 추가하는 메서드 addCard() 생성 - player 점수 합계를 계산하는 메서드 calculateScore() 생성

feat(Rank, Card): Rank의 ACE 여부 판정 메서드 추가

4a29ac8

test(PlayerTest): ACE 1/11 판정 여부 테스트

3f4c228

- 플레이어 점수 합계가 21초과 시 ACE 포함한 경우 점수 보정 테스트

feat(Player): ACE 1/11 판정 기능 추가

15c172c

- 플레이어 점수 합계가 21초과 시 ACE 포함한 경우 점수 보정 메서드 isBustWithAce()

feat(InputView, InputMessage): 사용자 이름 입력 안내메시지 기능 추가

096b999

test(CardsTest): 카드 덱에서 2장 드로우 정상 테스트 추가

ccdba17

feat(Cards): 카드 덱에서 드로우 기능 추가

4730a4b

- 2장 드로우 drawInitialHand() 추가 - 1장 드로우 draw() 추가

docs(readme): 두번째 기능 목록 구현

699d783

- 카드 배분 기능 추가

feat(Cards): 카드 덱 셔플 기능 추가

0801f6b

- shuffle() 메서드 생성

test(DealerTest): 딜러 카드 개수 테스트

7a4224f

test(PlayerTest): ace 1/11 처리 및, 플레이어 카드 개수 테스트

3c5a69e

feat(Player): Player 점수 계산 및 정보 반환 기능 추가

4fe4121

- name 필드 추가 - 점수 계산 로직 calculateScore() 수정 - 정보 반환 메서드 getPlayerInfo(), getCardsInfo()

feat(Players): 플레이어 일급 컬랙션

56ec80f

- 플레이어들의 정보 반환 메서드 getUserNames(), getPlayerInfo()

docs(readme): 3번째 기능 목록 추가

38c7a6c

- 사용자들에게 카드 추가 여부 기능 완성

test(PlayerTest): 카드 1장 추가 테스트

9a2a72f

- 플레이어의 카드가 1장이 추가되어 총 3장이 되었는지

refactor(Player): private -> public 접근자 수정

1a92e72

fix(Cards): 불변 객체가 아닌 가변 객체로 반환 값 수정

63f3326

feat(Players): players의 필드 값(플레이어들) 반환 메서드 생성

a5b9cd6

docs(readme): 딜러 카드 추가 로직 완료

cfcaad7

yj9107v added 9 commits March 9, 2026 09:25

test(CardsTest,DealerTest): isEqualTo() -> hasSize() 수정

6d1915b

- list의 size()를 직접 호출하지 않고, 크기를 비교할 수 있게 수정

test(PlayerTest): 테스트, 프로덕션 중복 구현 코드 수정

0b544b5

- 코드 작성 위치 해결 (테스트 코드는 Red, Green blue는 프로덕션 코드 - list의 size()를 직접 호출하지 않고, 크기를 비교할 수 있게 수정, isEqualTo() -> hasSize() 수정

test(InputParserTest): 테스트 메서드명 및 중복 코드 수정

15ae751

test(DealerTest): 단위 테스트 작성

44a4d08

refactor(Dealer): 딜러의 카드 추가 기능 수정

4a9a232

- 딜러 카드 추가 조건문을 Controller에서 도메인으로 옮김

test(PlayerTest): 단위 테스트 추가

656f4be

- 플레이어 합계 21 초과 여부 - 플레이의 보유한 카드 종류가 올바른지 테스트 - 플레이어 점수가 21 초과인 경우 점수 처리 테스트

refactor(Player): 도메인에 뷰 연관된 OutputMessage 제거

d553d7f

- 출력 format은 Controller 단에서 책임지게 코드 수정

refactor(Players): 반복문 fori -> iter 형태 변경

2ff0508

test(PlayersTest): 단위 테스트 작성

ce9c6d2

verus-j requested changes Mar 10, 2026

View reviewed changes

yj9107v added 19 commits March 10, 2026 14:35

test(InputParserTest): 사용하지 않는 상수 제거

6c23870

refactor(Dealer): 뷰와 연관된 getCardsInfo() -> getCards() 수정

b8c2d8c

- 도메인과 뷰의 연관성을 없앰

test(Dealer): 뷰와 연관된 getCardsInfo() -> getCards() 수정

286b297

- 도메인과 뷰의 연관성을 없앰 - 카드의 확인을 사이즈로 하지 않고 Card 리스트로 테스트

test(PlayersTest): 플레이들의 점수 테스트 수정

45ef1f6

- 플레이어들의 점수를 size로 테스트하지 않고, List<Integer>로 결과 값 테스트 수정

refactor(Policy): 객체 지향적인 코드를 위해 연관된 객체로 옮김

acd8f52

refactor(Dealer): 상수화 추가 및 메서드 수정

cba1f83

- Policy 상수화 옮김 - count 반환 값 삭제 - 접근지정자 public -> private 수정

test(DealerTest): 필요없는 테스트 삭제 및 메서드 네임 수정

76c13fe

feat(Player): 상수화 이동, 필요없는 메서드 삭제, 도메인과 뷰 연관성 제거, 방어적 복사

257ebf2

test(Player): 메서드명 변경, 필요없는 메서드 삭제

9755283

refactor(Card): 뷰와 연관된 메서드 제거 및 값 객체이므로equals/hashCode 재정의

ac0de01

refactor(Cards): 셔플 내부 호출 메서드 추가 및 상수화 이동

3973617

- 추상화 수준을 높이기 위해 shuffled() 메서드 추가

refactor(Players): Player 호출 메서드 변경

14430fd

- Player의 뷰와 연관된getCardsInfo() 호출하지 않고 getCards() 호출로 수정

refactor(Dealer): isBust() 접근지정자 변경 . 필요없는 메서드 제거

ef08395

- isBust() private -> public 변경 - getScoreZeroIfBust() 제거

test(DealerTest): getScoreOrZeroIfBust() -> isBust()를 활용해 테스트

04a3937

refactor(BlackjackResult): 도메인과 뷰 연관된 메서드 수정 및 점수 결과 로직 수정

dd2b67c

- 플레이어가 버스트인 경우 딜러 점수와 상관없이 플레이어 패배 - 뷰 연관된 메서드를 제거 후 필드 getter로 변경

test(BlackjackResultTest): 플레이어/딜러 승패 여부 테스트 추가

6f28201

refactor(BlackjackController): 도메인 내부에 뷰 연관된 것을 전부 제거 후 Controller에서 해결

1d58ac7

refactor: 필요없는 메서드 및 주석 제거

c15c152

test(CardsTest): 1장 드로우 테스트 추가

6496aa5

verus-j requested changes Mar 11, 2026

View reviewed changes


		public class Cards {

		public static final int FIRST_DRAW_SIZE = 2;

		public static final int ACE_HIGH_LOW_DIFF = 10;
		public static final int BUST_THRESHOLD = 21;

Conversation

yj9107v commented Mar 7, 2026

체크 리스트

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

1️⃣ 올바른 TDD 구현이란?

2️⃣ TDD의 우선순위

3️⃣ 테스트 단위

Uh oh!

verus-j left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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!

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!

verus-j 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants