[🚀 사이클2 - 미션 (블랙잭 게임 실행)] 에덴 미션 제출합니다. by RealTake · Pull Request #1111 · woowacourse/java-blackjack

RealTake · 2026-03-13T09:09:41Z

안녕하세요, 기론!

주신 피드백을 여러 번 곱씹어 보면서 다양한 방식도 시도해 보느라 PR과 답장을 늦게 올리게 되어 죄송합니다ㅜㅜ

피드백을 바탕으로 다른 크루분들과도 의견을 나누고, 여러 크루들의 코드를 살펴보면서 제가 미처 생각하지 못했던 방식이나 키워드들을 많이 알게 되었습니다. 덕분에 많은 것을 배우는 시간이 되었고, 그러다 보니 일주일이 정말 빠르게 지나간 것 같습니다.

특히 오랜기간 개발을 해오면서 주석 또한 관리의 대상이 될 수 있다는 점은 생각하지 못했던 부분이라 아직도 띵..합니다..!

PR에 앞서 다시 좋은 피드백 주셔서 다시 한 번 감사드립니다..!

체크 리스트

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

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

일급 컬렉션(Players)과 수익 계산 위치

상황
사이클 2에서 배팅 기능이 추가되면서 각 참가자의 수익을 계산해야 하는 요구사항이 생겼습니다.
현재는 BlackjackGame의 calculateAllPlayerRevenue() 메서드에서 전체 플레이어의 수익을 계산하고 있습니다.

이 과정에서 Players 내부의 플레이어들을 순회해야 하기 때문에 Players.getPlayerList() 메서드를 만들게 되었습니다.

고민
getPlayerList()는 내부 컬렉션을 외부로 노출하게 되므로 일급 컬렉션의 취지에 다소 어긋나는 것이 아닌지 고민이 되었습니다.

그래서 calculateAllPlayerRevenue() 메서드를 Players 내부로 이동시키는 것도 하나의 방법이라고 생각했습니다.
마침 Player가 betMoney 필드를 가지고 있기 때문에 Players가 각 Player를 순회하며 수익을 계산하는 구조도 가능해 보입니다.

다만 수익 계산이라는 행위가 Player나 Players보다는 게임의 규칙과 흐름을 담당하는 BlackjackGame에서 처리하는 것이 더 자연스러운 것은 아닐지 고민이 됩니다.

리뷰 요청
1. BlackjackGame이 Players.getPlayerList()로 컬렉션을 가져와 순회하면서 수익을 계산하는 방식
2. Players 내부에 수익 계산 메서드를 두고 BlackjackGame에서는 해당 메서드를 호출만 하는 방식

두 가지 구조 중 일급 컬렉션의 의도와 도메인 책임 분리 측면에서 어떤 구조가 더 적절할지 기론의 의견을 듣고싶습니다!

2. Service와 Game의 역할 경계

상황

blackjackGame에 대해서 아래 규칙을 적용해보는건 어떨까요? getter를 왜 제거하라고 하는지도 생각해보시죠!
객체지향 생활 체조 원칙
규칙 9: 게터/세터/프로퍼티를 쓰지 않는다.

-> getter를 제거하는 대신 서비스가 왜 게터를 사용해서 쓰게 됐는지를 보고, 그 이유를 다시 game에서 하도록 하다보니, 대부분의 서비스 로직들이 Game으로 이동하게 되었습니다. 지금은 Game이 게임 진행 로직”을, Service가 “Game 생성 + Game 위임 + DTO 변환”을 맡고 있습니다.

blackJackRule이 비즈니스 규칙에 해당하지 않을까요? 그렇다보니 BlackjackGame이 단순히 껍데기 역할로만 바뀌는것 같네요. blackJackGame내부에서 처리해보는건 어떨까요?
-> 마찬가지로 상수만 남겨놓고 각 필요한 도메인으로 메스드를 이동시켰습니다.

현재 서비스 클래스의 역할은 도메인을 직접 컨트롤러에 반환 하기 보단 DTO를 변환해서 주는데 의미를 두고있습니다.

고민
Service의 역할이 DTO 변환과 blackjackGame.xxx() 한 번만 호출하는 단순 위임이라, 처음부터 서비스 레이어를 두지 말았어야 했나 싶습니다.

리뷰 요청

지금 구조를 유지한 채 “Service = 진입점 + DTO” 역할만 명시할지
Service 레이어를 제거하고 Controller가 Game을 직접 쓸지

기론의 의견을 듣고 싶습니다.!

시간 내어 리뷰해 주셔서 다시 한 번 감사드립니다..!

- score() 메소드 의존성 변경 - controller 기능별 메소드 분리 - dto 패키지 변경

- Deck.cards필드를 Set으로 전환해서 중복에 대한 문제 방지 - Deck.draw() 에서 finally 문을 사용하는 대신 코드 흐름으로 개선

fix: hit&Stand 로직 오류 수정

- DefaultBlackjackRule: judge를 Participant 기반으로 변경, BLACKJACK 결과 추가 - 각 플레이어 별 배팅 금액 입력 받음

Gyuchool

안녕하세요 에덴
사이클2 미션 진행하시느라 고생하셨습니다!! 잘 구현해주셨네요👍

몇 가지 코멘트를 남겨두었으니 반영 및 의견 주시면 좋을 것 같습니다.🙏
궁금한 점은 DM이나 PR 커멘트로 언제든지 여쭤봐주세요!

BlackjackGame이 Players.getPlayerList()로 컬렉션을 가져와 순회하면서 수익을 계산하는 방식

Players 내부에 수익 계산 메서드를 두고 BlackjackGame에서는 해당 메서드를 호출만 하는 방식
두 가지 구조 중 일급 컬렉션의 의도와 도메인 책임 분리 측면에서 어떤 구조가 더 적절할지 기론의 의견을 듣고싶습니다!

blackJackGame은 카드 게임 진행을 다루고 있기에 수익 계산에 대한 책임은 다른 객체에 맡겨보는건 어떨까요? 리뷰 반영하면서 어떤 객체가 다뤄야할지 함께 개선해가보시죠..!

Service의 역할이 DTO 변환과 blackjackGame.xxx() 한 번만 호출하는 단순 위임이라, 처음부터 서비스 레이어를 두지 말았어야 했나 싶습니다.

지금 구조를 유지한 채 “Service = 진입점 + DTO” 역할만 명시할지

Service 레이어를 제거하고 Controller가 Game을 직접 쓸지

리뷰에도 남겼는데 저도 모호하다고 느끼고 있어요😊 에덴은 처음 설계할때 service blackjackGame을 각각 어떤 역할로 활용하려고 service를 추가했었을까요?

Gyuchool · 2026-03-14T07:17:20Z

src/main/java/team/blackjack/control/BlackJackController.java

+    }
+
+    private boolean hasDuplicatedName(List<String> playerNames) {
+        return playerNames.size() != playerNames.stream().distinct().count();


중복 확인을 위해 자료구조를 활용해보면 어떨까요?

Gyuchool · 2026-03-14T07:41:00Z

src/main/java/team/blackjack/service/BlackJackService.java

        final List<Player> players = playerNames.stream()
                .map(Player::new)
                .toList();


player객체 생성 책임을 BlackjacService보다 Players에게 넘기는건 어떠실까요?

딜러도 마찬가지로 적절한 곳에서 객체 생성하면 좋을 것 같네요:)

Gyuchool · 2026-03-14T07:48:44Z

src/main/java/team/blackjack/control/BlackJackController.java

+    private int readPlayerBattingMoneyRetry(String playerName) {
+        int battingMoney = -1;
+
+        while (battingMoney <= 0) {
+            battingMoney = readPlayerBattingMoney(playerName);
+        }
+        return battingMoney;
+    }


do-while문을 활용하면 어떨까요? 지금은 -1의 의미를 해석하기 어렵네요..!

Gyuchool · 2026-03-14T07:56:38Z

src/main/java/team/blackjack/control/BlackJackController.java

+import team.blackjack.view.OutputView;
+
+public class BlackJackController {
+    private final BlackJackService blackJackService;


에덴이 생각하시기에 blackJackController와 blackJackService의 각각 역할이 무엇일까요?
현재는 로직이 Controller에 더 많은 것 같은데 어떻게 생각하실까요?

Gyuchool · 2026-03-14T07:57:28Z

src/main/java/team/blackjack/control/BlackJackController.java

+        OutputView.printAskDrawCard(playerName);
+        String hitYn = InputView.readHitDecision();
+
+        while (!"y".equalsIgnoreCase(hitYn) && !"n".equalsIgnoreCase(hitYn)) {


"y","n"이 반복되는 상수로 관리하는건 어떠실까요?

Gyuchool · 2026-03-14T08:05:31Z

src/main/java/team/blackjack/domain/BlackjackGame.java

+    public void drawInitialCards() {
+        this.players.initPlayerHands(deck);
+
+        this.dealer.hit(dealer.draw(deck));


Suggested change

this.dealer.hit(dealer.draw(deck));

this.dealer.hit(deck.draw());

이렇게도 구현할 수 있을 것 같은데 dealer.draw()메서드를 만든 이유가 있을까요?

Gyuchool · 2026-03-14T08:06:36Z

src/main/java/team/blackjack/domain/BlackjackGame.java

+    private double calculateMoney(Player player) {
+        final Result result = judge(player);
+        return result.getOdds() * player.getBatMoney();
+    }
+
+    private Result judge(Player player) {
+        if (player.isBust()) {


점수 계산 및 판단하는 로직은 중요한 로직인데요. 테스트가 가능한 구조가 아닌것 같네요..! 적절한 객체에게 책임을 넘겨보는건 어떨까요?

Gyuchool · 2026-03-14T08:06:59Z

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


-    public List<Integer> getScore() {
-        return this.number.score;
+    public Set<Integer> getScore() {


Set자료구조 활용👍

Gyuchool · 2026-03-14T08:11:04Z

src/test/java/team/blackjack/domain/BlackjackGameTest.java

+    public Dealer dealer;
+    public Player pobi;
+    public Player jason;
+    public Players players;
+    public BlackjackGame game;


public일 필요가 있을까요?

Gyuchool · 2026-03-14T08:24:23Z

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

+public class Player extends Participant {
    private final String name;
-    private final List<Hand> hands;
+    private double batMoney;


규칙 3: 모든 원시값과 문자열을 포장한다.

배팅금액이 음수가 안되도록 view에서 잘 막으셨어요..!👍
다만, 배팅 금액이 음수가 안되는건 도메인 규칙에 해당된다고 생각하는데 도메인에서도 검증하는건 어떠실까요?

wonsang.choi added 30 commits March 4, 2026 17:33

feat(Rule): 블랙잭규칙에 대한 Rule 인터페이스 정의 및 생성

40f9388

test(rule): 블랙잭규칙에 대한 테스트 케이스 추가

7038a8f

feat(domain): 블랙잭 관련 엔티티 클래스 생성

9f743a8

feat(config): 앱 레이어 설정 파일 작성

b6b2ac6

refactor(Test): 블랙젝 테스트 페키지 이동

5445549

feat: 블랙잭 랜덤 카드 배정 로직 구현

51bda21

docs(README.md): 블랙잭 게임 기본 기능 구현 명세서 작성

36cc427

feat(Service): 딜러/플레이어 hit&stand 기능 추가

9c4d316

feat(Service): 딜러/플레이어 결과 출력

9c90fdd

chore: import문 최적화

2d76139

feat: 블랙잭 게임 결과 출력 기능 추가

7c456f2

test(BlackJackRule): 블랙잭 게임 규칙 클래스, score 계산 로직 테스트 추가

0dffe2e

fix(Result): 누락된 승패 열거형 클래스 추가

4232f37

refactor:

919643b

- score() 메소드 의존성 변경 - controller 기능별 메소드 분리 - dto 패키지 변경

test(Player): 플레이어 테스트 케이스 추가

16dd07a

test(Hand): 핸드, 카드점수 계산 테스트 케이스 추가

61e37fa

test(Deck): 덱, 카드 발급 테스트

c4ac74a

test(Dealer): 딜러 테스트 케이스 추가

f3ecebf

test(Service): 서비스 테스트 케이스 추가

de5db9e

test(rule): 규칙 테스트 케이스 추가

36eda30

refactor(Players): BlackjackGame.players를 일급 컬렉션으로 변경

966d2a8

refactor(OutputView): 사용하지 않는 메서드 정리

08ee405

feat(InputView): Hit 여부 입력에 대해, 잘못된 입력은 재입력 받도록 함

c1601e1

refactor(Card): Number 열거형을 추가하여 분리

55f8cda

feat(Controller): 중복된 이름 입력에 대한 재처리 추가

5bcf8f8

refactor(GameResult): GameResult 클래스명 MatchResult로 변경

34f1527

refactor(DefaultBlackjackRuleTest): 불필요한 필드 제거

2589ab0

refactor(DefaultBlackjackRuleTest): @ParameterizedTest를 사용하여 테스트 간소화

e907205

refactor(Service, Players): 불필요한 주석 제거

95e3e59

refactor(InputView): 불필요한 주석 제거

33dbf99

wonsang.choi added 21 commits March 10, 2026 13:45

refactor(AppConfig): 어플리케이션 설정 클래스 간소화

7c155aa

refactor(DefaultBlackjackRule): 블랙잭 상수 접근 제한자 변경

afc7369

refactor: 불필요한 주석 제거

8462cd9

refactor(Card): red 필드 명 변경

68c8912

refactor(Number): Score 필드 자료형 Set으로 변경

cd972c9

refactor(Deck): cards Set 자료형으로 변경

d6500d3

refactor(Deck): cards Set 자료형으로 변경

a72b9f3

- Deck.cards필드를 Set으로 전환해서 중복에 대한 문제 방지 - Deck.draw() 에서 finally 문을 사용하는 대신 코드 흐름으로 개선

refactor(MatchResult): countBy 제거 및 딜러 승/패/무 카운트를 생성자로 받도록 수정

881c109

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

37fc6b8

refactor(Deck): draw 역할 분리

2ed59cc

refactor(BlackjackGame, BlackJackService): 책임 이동 및 패키지명 변경

ccbf2e7

chore: 패키지명 변경

9fda00a

refactor: 2depth 컨벤션 수정

69f2bc2

refactor: isBust 메소드 명으로 변경

f9e30ff

refactor: equalsIgnoreCase 위치 변경

29c2b02

fix: hit&Stand 로직 오류 수정

refactor: Participant playrer/dealer 공통로직 분리

49b176b

feat: 플레이어 배팅 기능 추가

62e43c5

- DefaultBlackjackRule: judge를 Participant 기반으로 변경, BLACKJACK 결과 추가 - 각 플레이어 별 배팅 금액 입력 받음

test: BlackjackGameTest 추가, DefaultBlackjackRuleTest 케이스 추가

445c95e

refactor: 불필요해진 DTO 및 메서드 제거

34c8f47

refactor: Rule 로직 도메인로직으로 이동

1a4fcf3

Merge remote-tracking branch 'upstream/realtake' into step2

b5f2d3c

Gyuchool requested changes Mar 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[🚀 사이클2 - 미션 (블랙잭 게임 실행)] 에덴 미션 제출합니다.#1111

[🚀 사이클2 - 미션 (블랙잭 게임 실행)] 에덴 미션 제출합니다.#1111
RealTake wants to merge 51 commits intowoowacourse:realtakefrom
RealTake:step2

RealTake commented Mar 13, 2026

Uh oh!

Gyuchool left a comment

Uh oh!

Gyuchool Mar 14, 2026

Uh oh!

Gyuchool Mar 14, 2026

Uh oh!

Gyuchool Mar 14, 2026

Uh oh!

Gyuchool Mar 14, 2026

Uh oh!

Gyuchool Mar 14, 2026

Uh oh!

Gyuchool Mar 14, 2026

Uh oh!

Gyuchool Mar 14, 2026

Uh oh!

Gyuchool Mar 14, 2026

Uh oh!

Gyuchool Mar 14, 2026

Uh oh!

Gyuchool Mar 14, 2026

Uh oh!

Gyuchool Mar 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	this.dealer.hit(dealer.draw(deck));
	this.dealer.hit(deck.draw());

Conversation

RealTake commented Mar 13, 2026

체크 리스트

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

2. Service와 Game의 역할 경계

Uh oh!

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

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants