[🚀 사이클1 - 미션 (블랙잭 게임 실행)] 파도 미션 제출합니다 by choubung · Pull Request #1011 · woowacourse/java-blackjack

choubung · 2026-03-08T06:51:28Z

안녕하세요 웨지!
페어랑 얘기를 나눌 부분이 많기도 했고, 많이 헤매기도 해서 생각보다 첫 PR이 늦어지게 되었어요...
어쩌다 보니 질문도 좀 많이 나왔네요😅
리뷰 잘 부탁드립니다!

체크 리스트

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

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

1. 데이터 단순 조회의 getter와 잘못된 설계에 사용된 getter의 차이

객체 지향에서 getter와 setter를 지양해야한다고 알고 있었습니다. 그런데 데이터 조회를 할 때는 필연적으로 getter가 필요한데 어떻게 그게 가능한가 고민을 좀 하다가, 단순 데이터를 조회하는 건 괜찮지만 데이터를 꺼내 활용하는 로직이 메소드 바깥에 떨어져 있으면 안 된다는 것을 의미함을 알게 되었습니다. 그래서 도메인 데이터 처리 관련 로직은 최대한 가장 원초적인 도메인 안으로 밀어 넣었습니다. 이렇게 하니 호출하는 상위 객체에는 관련 로직이 없긴 하지만, 해당 메소드를 호출하기 위해 상위객체-서비스-컨트롤러단까지 동명(혹은 유사한 이름의) 메소드로 해당 메소드를 체인처럼 호출해야했는데(처리 로직은 도메인에만 존재), 이 방식이 맞는 걸까요?

2. 일급컬렉션의 사용

이번 미션에서 일급 컬렉션을 처음 사용해보았습니다. 플레이어 그룹과 플레이어가 손에 쥔 카드 목록을 각각의 일급컬렉션으로 만들었습니다. 일급컬렉션의 사용 목적 중 하나에는 불변성이 있던데, 카드 목록은 계속해서 값이 추가되는데도 일급컬렉션으로 사용해도 괜찮은 걸까요? 현재는 값이 변동이 있지만, 카드 목록에 이름을 붙여 컬렉션으로 만드는 것이 가독성에 좋다고 생각하였고, 요구사항에 되도록 모든 원시값을 감싸고 일급 컬렉션을 활용하라고 했기에 일급 컬렉션으로 작성했습니다.

3. dto 구현의 선

현재는 돌아가는 구현에 집중하느라 DTO를 작성하지 못했습니다. 그런데 name(String) 처럼 정말 단순한 데이터도 dto로 감싸야 하는지, 오버엔지니어링은 아닌지, 그 선이 어디인지가 고민됩니다.
이번 미션을 진행하면서 DTO에 대해 깨달은 바가 아래와 같은데, 제대로 이해한 것이 맞는지 궁금합니다! 그리고 dto를 사용하도록 리팩토링할 예정인데, 특히 dto를 반드시 사용해야만 하는 부분이 있다면 어디일까요?
- ResultView에 객체를 직접 전달해주면 안되나?
  - 그러면 메소드를 통해 값을 변경할 수 있다. 그래서 객체를 직접 전달해주면 안된다.
- 그러면 DTO를 꼭 써야하는건가? LIst과 같은 컬렉션을 전달해주면 안되나?
  - String의 값이 어떤 걸 의미하는 지 정확히 알기 어렵다.
  - DTO를 사용한다면 어떤 값이 필수적인지, 어떤 것을 의미하는지 알 수 있다.

4. Player와 Dealer

원래 Dealer 객체를 따로 생성하려다, 그렇게 하려니 설계의 진도가 나가지 않아 일단 구현 우선으로 딜러를 플레이어의 일환으로 생각해 구현했습니다… 겹치는 부분을 부모클래스로 만들지, 아니면 추상 클래스로 만들어 각각 구현할 지, 아니면 새로운 접근법으로 겹치는 로직을 새로운 객체로 빼서 해야할지 고민이었는데 이 부분을 추후 리팩토링에 어떻게 적용해야할 지 고민입니다.
둘은 행위가 비슷할 뿐 본질적인 역할(플레이어는 게임 참여, 딜러는 게임 중앙 관리자에 가까운 느낌이라고 생각했습니다)이 다르다고 생각합니다. 그렇기에 부모클래스를 만들어 상속하기보다는 인터페이스나 기능 객체 생성이 낫다고 저는 생각하고 있는데, 웨지는 어떻게 생각하시는 지 궁금합니다.
그리고 둘을 분리하면 대규모 리팩토링이 발생할 것 같은데,,, 대규모 리팩토링에 슬기롭게 대처하는 웨지만의 팁이 있으실까요?

5. TDD

초반 이후에는 구현에 쫓겨 거의 TDD를 활용하지 못했습니다. 현재 상태에서 보충해야할 테스트도 많고요... 저는 원래 TDD가 그래도 기본 도메인 구조와 메소드 이름, 메소드 인자 및 리턴값까지는 생각을 해서 스켈레톤 코드로 만들어 놓고 테스트 RED를 만든 다음 이후 로직을 짜 GREEN으로 만들고 리팩토링을 하는 거라고 생각했습니다. 그런데 다른 크루들과 이야기를 나누다보니 컴파일 에러도 RED의 일종이며, 실제로 엄격한 TDD에서는 정말 테스트 코드 작성 전에는 한 줄의 기능 코드도 안 쓰는 경우가 있다는 걸 알게 되었습니다. 그러나 저는 미리 생각해서 도메인의 인스턴스 변수나 커다란 기능(요구사항에서 하나의 입출력값 쌍이 명확히 존재하는 가장 윗 단계의 기능)정도는 스켈레톤 코드로 작성해놓는 걸 선호하는데, 이러면 TDD에 본질에 어긋나는 건지도 궁금합니다…
테스트 코드를 작성 → 기능 구현 → 통과 확인을 과정을 반복하는데, 책임을 다른 객체에게 주어야 한다는 것을 알았을 땐 테스트 코드의 변경이 필요했습니다.
- Q. TDD에서 기능 구현도중 테스트 코드의 변경은 괜찮은 행위일까요? 설계 부족일까요?

6. git commit의 역할

git commit에는 1) 버전 저장과 2) 내가 아닌 타인이 작업흐름/코드 흐름을 일목요연하게 이해할 수 있는지 도움을 주는 것의 두 가지 역할이 있다고 생각합니다. 코드를 어느정도 안정적으로 짤 수 있을 때는 버전 저장의 용도로 쓰더라도 후자가 자연스럽게 병행된다고 생각하는데, 아직 저는 그런 수준이 아니라 단순 버전 저장 용도로만 커밋을 하면 노력을 해도 중간부터는 커밋 로그가 깔끔하지 않게 생성되는 것 같습니다. 그래서 지금 수준의 단계에서는 완전히 다 구현하고 커밋을 정리하는 게 나은지, 아니면 그래도 실제로 코드 짤 때 편하도록 바로바로 커밋하는 게 나은지 궁금합니다. 찾아보니 일단 커밋을 하고 나중에 커밋 단위를 정리하는 방법도 있다고 하던데, 실무에서는 어떤 방식으로 많이 하는지도 궁금합니다.

페어: @Eian1106

이름 리스트 공백 검증은 이름을 담을 vo에 존재해야하는 로직이라고 판단하여 InputParserTest에서 해당 테스트를 삭제함

플레이어 히트 로직 구현 버스트 판별 구현 카드 점수 계산 로직 구현 에이스 점수 결정 구현

플레이어의 승패 계산 구현 딜러의 결과 계산 구현 최종 결과 출력 구현

sihyung92

안녕하세요 파도, 리뷰어 웨지입니다.

본격적인 첫 미션하느라 수고 많으셨습니다.
리뷰 남겼으니 확인해주시고, Q&A 답변이에요.

Q1. 데이터 단순 조회의 getter와 잘못된 설계에 사용된 getter의 차이
A1. 학습 내용 자체는 공감이 가는데, "메소드를 체인처럼 호출"이 어느 부분을 말씀하시는지 모르겠네요. 다음 PR 때 file changed에서 코멘트로 남겨주셔요.

Q2. 일급컬렉션의 사용
A2. 일급컬렉션의 사용 목적 중 하나에는 불변성에 있는데 -> 컬렉션의 불변성이라는 건 애매한 측면이 있습니다. element의 요소를 추가하거나 삭제하지 못한다면 불변인가요? 혹시 element가 가변객체여서 내용물을 수정할 수 있다면 컬렉션은 불변인가요 아닌가요? 토론하기엔 좋은 주제지만 저는 그냥 컬렉션을 재할당 못 하고, 내부 컬렉션 자체를 끄집어내서 다른 객체에서 수정 못 하는 수준이면 만족합니다.

컬렉션의 불변성에 관해 주위 크루 및 ai와 토론해보시고, 본인의 결론을 알려주세요.

Q3. Dto 팁
A3. 영향범위를 구분짓고 싶을 때 활용하시면 됩니다. 기계적인 선을 그은것이 레이어드 아키텍처이니 레이어간 코드가 왔다갔다 할 때 DTO 이야기가 많이 나오는 것이고요.

예로들면 Player를 직접 의존했을 때 장단점을 볼게요.
장점 : 하나 고치면 코드 전체에 다 적용된다 (의도함)
단점 : 하나 고치면 코드 전체에 다 적용된다 (버그)

웹 API를 만들었다고 하면, 제가 내부 코드를 고쳤는데 API 응답이 바뀌어버리면 해당 API 코드를 의존하는 전세계 동포들이 고장납니다. 반드시 격리해줘야겠죠.
그런데 미션 수준에서 DTO로 따면 한번 수정하면 될 거 두 번 세 번 고쳐야 합니다.
도메인은 고쳤는데 뷰는 깜박해서 오히려 버그가 날 수 도 있고요.
이런 감은 경험과 배움에서 나오는 거라 일단 겪어보시는 걸 추천드립니다. 다양한 자료를 접하고 참고하시고요. 대부분 케이스가 다르고 시대가 다르니 공신력 있는 자료라고 맹신하지 마시고요.

Q4. Player와 Dealer의 분리
A4. 이건 리뷰 중 분리에 대한 코멘트가 있으니 참고해주세요. 방법은 본인이 원하는 것 하나 선택해서 선택의 이유를 알려주시고요, 대규모 리팩토링은 원래 힘듭니다. 원래 잘 되는걸 왜 건드리는지 비개발자에게 이해받기도 힘들고요. 그렇다고 가만히 있으면 점점 불편이 커져가니 오늘이 가장 빠르단 걸 기억해주시고요. 화이팅입니다.

Q5. TDD
A5. 본문 리뷰 중 답변했습니다.

Q6. git commit의 역할
A6. 말씀하신대로 사람마다 다릅니다. commit은 또 원자성 (하나하나의 커밋이 동작 가능한 상태)을 갖춰주는 것도 중요한데요. 언제 어느 커밋에서 브랜치를 딸지 모르니까요.
그런데 원자성과 단위별 커밋을 하려면 구현단계부터 작게 작게 커밋을 하는 습관이 필요합니다. 전체 기능개발을 하는 경우엔 나중에 커밋을 정리해도 원자성을 갖추기 힘들거든요. 그래도 말씀하신 것처럼 남들 파악에 도움이 되니 좋은 습관인 것은 맞습니다. 우선 커밋에 너무 신경쓰지 마시고, 나중에 정리해서 올려보세요.
정리 할때도 클래스를 몇개 씩 묶어서 본인이 생각하는 방식으로 올리게 될텐데요.
해보시면 가장 의존이 적은 코드부터 의존이 많은 코드 순서로 커밋을 정리하게 될거에요. 코드 자체를 그 순서로 짜는 연습을 하다보면 자연스럽게 깔끔하게 커밋이 될거에요.

추가 질문 있으면 편히 주셔요. 화이팅입니다

sihyung92 · 2026-03-08T07:31:34Z

src/test/java/domain/vo/NameTest.java

+//    @Test
+//    void 이름이_딜러일때_예외 (){
+//        String input = "딜러";
+//
+//        Assertions.assertThatThrownBy(() -> new Name(input))
+//                .isInstanceOf(IllegalArgumentException.class);
+//    }


프로덕션에선 VCS (git)에서 흔적을 찾을 수 있으니, 중요하지 않은 테스트는 삭제하는 편이에요.
미션에서 질문하기 위한 목적이라면 file changed 탭에서 suggestion 기능을 활용해보세요

ex) 리뷰어님 이런 테스트가 필요할까요?

Suggested change

// @Test

// void 이름이_딜러일때_예외 (){

// String input = "딜러";

//

// Assertions.assertThatThrownBy(() -> new Name(input))

// .isInstanceOf(IllegalArgumentException.class);

// }

@Test

void 이름이_딜러일때_예외 (){

String input = "딜러";

Assertions.assertThatThrownBy(() -> new Name(input))

.isInstanceOf(IllegalArgumentException.class);

}

이 부분은 원래 플레이어의 이름으로 딜러를 사용하지 못하도록 하는 요구사항을 적용하고자 만든 테스트였는데, 기존 코드는 플레이어와 딜러 객체가 분리되어있지 않아 해당 테스트를 사용할 수가 없어 주석처리를 했었습니다. 해당 경우에는 Deprecated 주석을 남기면 될까요?

별개로 suggestion 기능은 처음 알게 되었는데, 앞으로 잘 활용해보겠습니다. 감사합니다!

"@disabled" 라는 기능이 있긴 한데요. 현재 불완전한 코드면 주석 상태로 두지 말고 삭제하는 것을 추천해요.
그런 식으로 은근히 주석쳐진 코드가 쌓이거든요. (정리정돈 습관 정도라고 봐주세요)

sihyung92 · 2026-03-08T07:34:28Z

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

+    // 카드 value 반환
+    public int getValue(){


다른 리뷰어에게 남긴 리뷰를 복붙 한번 하겠습니다. 메서드명은 다르지만 요지를 참고하셔요

코드를 통해 파악할 수 있는 내용은 주석으로 달아두면 중복코드 처럼 안 좋은 역할을 합니다. 주석 또한 유지보수 대상인데, 이처럼 단순히 코드 내용을 반복하는 것이면 당장은 큰 문제는 아니지만 이후 코드 내용은 달라졌는데 주석 내용은 그대로면 문제가 생깁니다.

메서드 시그니처 등으로 부족하다는 생각이 들면 코드를 가다듬어보세요
calculatePlayerWinDefeatDraw
PlayerResultInfo

좋은 주석은 코드를 통해 파악할 수 없는 "의도"나 "역사"를 담는 것입니다. 예시입니다.
// 승무패를 딜러 기준으로 계산할 때 시간복잡도가 높아 유저기준으로 계산함
// 이 메서드는 Deprecated 이지만 하위 호환을 위해 유지해야 함

주석이 중복 코드가 되면 안 된다는 걸 알긴 했는데, 페어와 코드를 마구 오가며 코드를 짜다 보니 식별을 위해 저도 모르게 주석을 달았던 것 같아요...

원래는 단지 주석은 코드와 중복되면 안 된다는 것만 알고 있었고, 주석을 효과적으로 쓰는 방법은 몰랐는데 웨지의 코멘트 덕분에 주석의 목적에 대해 다시 생각해볼 수 있었어요. 감사합니다!

sihyung92 · 2026-03-08T07:34:58Z

src/main/java/domain/player/PlayerGroups.java

+        return status;
+    }
+
+    // 이번 라운드의, 해당 플레이어 반환


전체적으로 프로그램에 남긴 주석들을 점검해주셔요.

넵! 전반적으로 다듬었습니다.

sihyung92 · 2026-03-08T07:36:25Z

src/main/java/domain/vo/Name.java

+//    private static void validateNotDealer(String name){
+//        if (name.equals("딜러")) {
+//            throw new IllegalArgumentException("[ERROR] 딜러는 플레이어 이름으로 사용할 수 없습니다.");
+//        }
+//    }


미사용 코드는 제거해주세요.

해당 코드 리팩토링하며 플레이어/딜러 객체 분리로 사용하게 되었습니다!

sihyung92 · 2026-03-08T07:43:52Z

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

+    // 카드 value 반환
+    public int getValue(){


다른 리뷰어에게 남긴 리뷰를 복붙 한번 하겠습니다. 메서드명은 다르지만 내용에 참고하세요

코드를 통해 파악할 수 있는 내용은 주석으로 달아두면 중복코드 처럼 안 좋은 역할을 합니다. 주석 또한 유지보수 대상인데, 이처럼 단순히 코드 내용을 반복하는 것이면 당장은 큰 문제는 아니지만 이후 코드 내용은 달라졌는데 주석 내용은 그대로면 문제가 생깁니다.

메서드 시그니처 등으로 부족하다는 생각이 들면 코드를 가다듬어보세요
calculatePlayerWinDefeatDraw
PlayerResultInfo

좋은 주석은 코드를 통해 파악할 수 없는 "의도"나 "역사"를 담는 것입니다. 예시입니다.
// 승무패를 딜러 기준으로 계산할 때 시간복잡도가 높아 유저기준으로 계산함
// 이 메서드는 Deprecated 이지만 하위 호환을 위해 유지해야 함

sihyung92 · 2026-03-08T08:01:10Z

src/main/java/service/GameService.java

+    }
+
+    public void initAllPlayerCard() {
+        for (int i = 0; i < 2; i++) {


"2"는 도메인 적으로 의미가 있는 횟수입니다. (카드분배 최솟값)
상수로 관리해주세요.

넵! 전반적으로 점검하며 매직넘버들 정리했습니다.

sihyung92 · 2026-03-08T08:03:11Z

src/main/java/domain/player/PlayerGroups.java

+                continue;
+            }
+
+            result.put(playerName, player.isWin(dealer.getCardSum()));


playerName에 대한 중복검사가 도메인에 없습니다. 버그가 발생하는 코드이니 적절한 key가 무엇일지 고민해보세요.

플레이어 네임에 대한 중복 검사를 inputParser에서 진행했는데, 혹시 그래도 도메인 안에 검증 로직이 존재하는 게 좋을까요? 아니면 아예 해당 검증 로직을 파서에서 PlayerGroups로 옮기는 게 좋을까요?

넵, 뷰라는 건 여러 군데에서 붙을 수 있는 거니까요.

도메인 쪽 코드를 뷰 단의 로직으로부터 보호하라는 지침을 지속적으로 받으실텐데요, 이건 뷰 코드는 쉽게 변하는 데 비해 도메인 규칙이라는 게 비교적 불변이기 때문입니다.
(만약 뷰 출력 형식이 10년에 한번씩 법으로 제정되어야만 바뀔 수 있는 영역이었다면, 거꾸로 뷰를 보호하기 시작할거에요.)

콘솔 컨트롤러 외에 웹 컨트롤러도 추가하라는 요청을 받았다고 생각해보셔요. 동일한 중복검사를 양쪽에서 관리해야 합니다. 뷰가 3개, 4개, 5개 늘어나다 보면 점점 관리가 어렵겠죠. 그러다 하나 validation을 잊은 뷰가 있다면 예외가 발생하는거고요.

sihyung92 · 2026-03-08T08:20:10Z

src/main/java/domain/player/HandCards.java

+    private int calculateAce(int currentSum){
+        int aceCase10 = Math.max(21 - (currentSum + 11), 0);
+        int aceCase1 = Math.max(21 - (currentSum + 11), 0);
+
+        if (aceCase10 < aceCase1) {
+            return 10;
+        }
+
+        return 1;
+    }


aceCase10와 aceCase1는 산식이 같아서 58번은 무조건 false일거 같은데요.

핵심로직 중 하나인데 테스트 코드 찾아보니 없네요.
해당 로직을 테스트할 수 있는 방법에 대해 고민해보세요. 반드시 구현해주시고요.

5번 TDD에 대한 답변은 여기서 대신합니다.

TDD는 극단적으로 말하면 "테스트 나중에 짜보니까 힘들더라 미리 짜보자"하는 방법론에 불과합니다. 학습단계에선 일단 배워보는 것도 필요하겠지만 개발에선 특히 무언가를 배울 때 이 방법이 왜 나왔는가 부터 알고 넘어가면 도움이 많이 됩니다.
어떤 방법을 사용하시든 근본적인 핵심은 테스트를 잘 구성하는 것이 유지보수에 도움이 된다는 것이고, 그럼 집중해야할 것은 '어떻게' 보다는 테스트 코드의 커버리지를 늘려보려는 쪽이라고 생각합니다.
뭐가 좋더라는 자주 시도하다 보면 자연스럽게 배우게 되실거고요.

TDD에서 기능 구현도중 테스트 코드의 변경은 괜찮은 행위일까요? 설계 부족일까요?

테스트 코드가 왜 바뀔까요? 구현이 바뀌어서 그런 것 아닐까요?
그럼 질문을 바꾸면, 구현을 안 고치고 프로그램을 완성하실 수 있나요?
테스트 없이 의식의 흐름으로만 짜도 계속 구현이 바뀌는데, 익숙하지 않은 TDD 까지 매고 뛰시려면 더 어려우시겠죠. 제가 지금까지 겪어본 개발자 중 머릿속에 있는 내용을 한 호흡에 모두 구현하는 사람은 천명 중에 한 2명 본거 같네요.

소프트웨어 공학은 절대 진리가 존재하는 자연과학과 달리 유지보수성 하나의 철학만 존재합니다. (적어도 웹 앱 개발은요) 내가 불편하다면 항상 의심하시고, 부딪히세요. 그럼에도 좋다고 한다면 어떤 부분이 좋은지 보고, 그 부분만 따오세요. 본인만의 TDD를 만들어보시라 권하고 싶네요.

우테코에서 TDD를 해야 한다는 얘기를 듣고 한 생각은 TDD를 왜 하는가, 보다는 TDD를 어떻게하면 잘할 수 있을까 였던 것 같습니다. 잘하고 싶다는 마음이 너무 앞섰던 것 같아요. 생각해보면 리뷰어님의 말대로 테스트의 본질은 유지보수를 잘 하기 위해서이고, 이 프로젝트 코드의 목적은 결국 프로덕션 코드를 잘 짜는 것이니까요. TDD를 마냥 어렵게 생각하고 배워야하는 무언가라고 생각했는데, 앞으로는 저에게 도움을 줄 수 있는 도구라는 관점에서 접근하며 고민을 해보아야겠습니다. 감사합니다.

sihyung92 · 2026-03-08T08:22:32Z

src/main/java/view/ResultView.java

+    public static void printStartPlayersCards(Map<String, List<String>> playerCardList) {
+        List<String> playerNames = new ArrayList<>();
+        for (String playerName : playerCardList.keySet()) {
+            if (playerName.equals("딜러")) {


"딜러"라는 상세 구현에 의존하고 있어요. 도메인 단에서 딜러를 구분할 수 있는 방법을 고안해보세요.

넵!! 다른 코멘트에 구현 방법과 선택 이유 남겨놓았습니다!

sihyung92 · 2026-03-08T08:25:15Z

src/main/java/domain/player/Player.java

+
+import java.util.List;
+
+public class Player {


딜러와 참가자의 구분은 핵심 도메인 로직인데, 도메인 영역에서 책임지지 않으니 컨트롤러와 뷰에서 "딜러" 라는 상세 구현으로 처리하는 방향으로 가고 있어요.
플레이어와 딜러를 명확히 구분할 수 있는 방안을 고안해주세요.

이러면 딜러를 수정할 때 수정해야 하는 코드들이 많아집니다. 객체지향은 "뭐 수정할 때 최대한 조금 수정하고 싶은" 철학이에요. 항상 이 관점에 유념해주세요.

넵!

현재는 Participant 추상 클래스를 만들고, Dealer와 Player가 상속받는 방식으로 구현했습니다.

추상 클래스를 상속받는 방식을 택한 이유는

아예 연산 로직이 겹치는 메소드들이 있음 -> 일반 메소드

승리 저장 로직은 다름 -> 추상 메소드로 구현
을 하고자 하였기 때문입니다.

카드 덱 소모시 셔플해 초기화하는 기능 명세를 추가함

HandCards를 외부에서 주입할 수 있도록 변경함

카드 리스트를 외부에서 받아 생성할 수 있도록 생성자 추가

HandCards를 외부에서 주입받도록 생성자를 변경함

choubung · 2026-03-09T15:44:07Z

질문이 많았는데 다 정성껏 답변해주셔서 정말 감사합니다. 공부하는데 정말로 많은 도움이 되었어요!!

질문 1번의 경우 리팩토링을 하면서 코드가 정리되어 이번 미션에서는 해당 부분을 찾기가 어렵네요...
예를 들어 딜러 도메인에 isDealerHit()이라는 메소드가 있는데, Controller에서 딜러 히트 여부를 알기 위해 (단지 dealer.isDealerHit()를 반환하는 게 전부일 뿐인) Service 계층인 isDealerHit() 메소드를 호출하는 경우가 괜찮은 건지, 너무 과하게 포장되어 있는 건 아닌지 궁금합니다.

이번 일급컬렉션을 사용하기 전까지 저는 객체를 그냥 할당하는 것과 복사해 할당하는 것의 차이를 인지하지 못했던 것 같습니다. 그래서 일급 컬렉션의 불변성이라는 말이 이해하기가 어려웠고, 사용하기에는 더 난해했던 것 같아요. 하지만 웨지의 말을 듣고, 컬렉션에 대해 조금 더 찾아보고 내린 저의 결론은 결국 일급 컬렉션은 밖에서 해칠 수 없는 튼튼한 바구니에 가깝다는 생각입니다. 필요하면 안에서 가공도 하고요. 정확히 써야한다는 생각에 실용성이나 목적성을 간과했던 것 같습니다.

DTO는 DealerDto와 PlayerDto를 추가했습니다. 웨지의 코멘트를 보고 고민을 해봤는데요, 특정 상황에서의 DTO가 아니라 포괄적인 객체 DTO라면 오히려 존재하는 것이 ResultView에 값을 전달할 때 더 깔끔하게 전달할 수 있다고 판단하여 추가하는 것으로 선택했습니다.

한 미션임에도 불구하고 많은 내용을 얻어갈 수 있도록 도와주셔서, 리뷰에 소중한 시간을 내어주셔서 감사합니다! 시간이 늦어서 리뷰 요청은 화요일 오전 중으로 드릴게요!!

자식 클래스들에서 Participant로 이동

>> 기존: 딜러 객체 생성 시 '딜러'를 name 필드에 저장 >> 변경 후: 메소드 오버라이딩으로 getName 호출 시 '딜러'를 반환하도록 함

sihyung92

안녕하세요 파도, 리뷰어 웨지입니다.

질문 1번 이어서 보면, 현재의 GameService는 단순히 같은 메서드를 체인하기보다는 무언가 역할을 하고 있네요.
하지만 질문 자체는 체이닝만 하는 것의 피로감에서 주신걸로 이해했어요.
반대로 체이닝을 하지 않는다면, 사용처에서 내부 클래스에 대한 의존이 필요하겠죠.

서로의 의존이 자연스러운 클래스 간에는 내주어도 될 것이고, 상세 구현을 몰라야 하는 관계에선 끊어주어야 할 거에요.
GameService를 보는 Controller의 입장에서 생각해보셔야 하는데요, Controller가 도메인 패키지의 클래스들을 상세히 알아야 한다는 것에 동의한다면 클래스 의존을 추가하는 거고요. 그렇지 않아도 된다면 Service가 끊어주는 거고요. 결합도(Coupling)에 대한 논의로 이어질 수 있는데, 여러 미션들을 해나가면서 조금씩 기준을 쌓아보셔요.

새로운 리뷰들을 남겼으니 반영하시고 또 요청주셔요. 감사합니다.

sihyung92 · 2026-03-10T14:26:45Z

src/main/java/domain/participant/player/PlayerGroups.java

+    }
+
+    public Player nextPlayer() {
+        return players.get(playIndex++);


nextPlayer를 사용하기 전 반드시 hasNextPlayer 검사를 해야만 정상 동작하는 코드에요.

방어로직을 작성해주시거나 예외를 발생시키거나, 두 가지 방법이 있겠네요

sihyung92 · 2026-03-10T14:27:55Z

src/main/java/domain/participant/player/PlayerGroups.java

+
+    public boolean hasNextPlayer() {
+        if (playIndex > players.size() - 1) {
+            playIndex = 0;


hasNextPlayer는 단순히 checker 처럼 느껴져서, index를 0으로 되돌릴거라 예측하기가 어려워요.

아무 조작 없이 hasNextPlayer를 2번 호출했는데 false -> true로 내려오면 버그처럼 느껴질 거여서요.
index를 초기화 하는 로직은 여기서 제외해주시고 다른 방법을 고안해보시죠.

sihyung92 · 2026-03-10T14:30:21Z

src/main/java/domain/vo/Name.java

+        }
+    }
+
+    private static void validateNotDealer(String name){


포멧팅 오류가 있습니다! ㅎㅎ

Suggested change

private static void validateNotDealer(String name){

private static void validateNotDealer(String name) {

포맷팅이 중요한 이유는 가독성도 있지만, 서로 다른 포맷팅을 사용하는 유저 끼리 reformat을 할 때 비즈니스 변경이 아닌 내용이 변경으로 잡혀 conflict를 유발하는 것에도 있어요. 코드를 push 하기전에 코드 전체 포맷팅 단축키를 이용하시거나, 저번에 안내드린 것처럼 IDE의 도움을 받아 auto reformat를 활성화하는 것도 방법입니다.

sihyung92 · 2026-03-10T14:35:03Z

src/main/java/service/GameService.java

+    private PlayerGroups playerGroups;
+    private Dealer dealer = new Dealer(new HandCards());
+    private CardDeck cardDeck = new CardDeck();
+    private static final int START_CARD_COUNT = 2;


static final 변수는 멤버 변수 들보다 위쪽으로 배치하는 게 널리 퍼진 컨벤션입니다.

sihyung92 · 2026-03-10T14:42:53Z

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

+        return number == TrumpNumber.ACE;
+    }
+
+    public String cardToKorName() {


이런 것도 사실 뷰 로직에 해당하긴 하는데요.

제가 엄격하게 리뷰한 리뷰이의 경우엔 이런 것도 분리하라고 리뷰한 크루도 있어요. (view 단에서 도메인과 한국 명칭의 맵핑을 관리)

그런데 저는 결론적으론 이 미션에서 그렇게까지 분리해야되나 라고 생각하긴 해서 공통 리뷰로 안 드리는 중인데요.

cardToKorName라는 메서드 명명이 절 살짝 흔드네요 ㅋㅋ
EngName랑 ChinaName 요구사항이 생기면 계속 메서드가 하나씩 추가될까요?
TrumpNumber와 TrumpSuit에서 계속 locale이 추가되고요?

이런 상황이 생길 수 있어 뷰를 도메인에서 격리하고자 하는건데요,
기왕 코멘트를 남겼으니 한글 출력의 관한 부분을 view 단으로 분리하라는 리뷰도 남기겠습니다.

sihyung92 · 2026-03-10T14:47:11Z

src/main/java/service/GameService.java

+import java.util.List;
+import java.util.Map;
+
+public class GameService {


저번에 제가 왜 명칭이 Service인지 리뷰를 안 남겼나봐요. "Service" 네이밍이 있는 분들 다 시비걸고 다니는 중인데요,
여기도 남겨둘게요.

왜 명칭이 service인가요? 또 service 패키지는 뭐하는 패키지인가요?

sihyung92 · 2026-03-10T14:48:18Z

src/test/java/domain/participant/HandCardsTest.java

+        assertThat(handCards.calculateCardsScore()).isEqualTo(exceptResult);
+    }
+
+    static Stream<Arguments> provideHandCardsData() {


다양한 케이스에 대해 테스트를 만드셨네요. 좋습니다.
좀 더 좋은 테스트를 만들려면, 케이스를 경계값으로 설정하는 것이 좋아요.

예를들어 ACE 1 Test Case에선 SEVEN + NINE + ACE 조합으로 17이 되는 걸 보는 것보다,
SEVEN + FOUR + ACE로 12를 만들고 (최솟값) TEN + TEN + ACE로 21이 되는지 (최댓값) 보는게 좋습니다.

sihyung92 · 2026-03-10T14:52:29Z

src/test/java/domain/player/PlayerGroupsTest.java

+                        new Player(new Name("파도"), new HandCards()), new Player(new Name("이안"), new HandCards()), new Player(new Name("슈크림"), new HandCards())));
+
+        assertThatThrownBy(() -> new PlayerGroups(players))
+                .isInstanceOf(IllegalArgumentException.class);


메시지 내용까지 검사하시면 혹시나 다른 사유로 예외가 발생했는데 놓치고 지나가는 불상사를 막을 수 있겠죠.

choubung added 23 commits March 7, 2026 23:47

Docs(readme): 기능 목록과 요구사항 작성

bdc07f3

Test(InputParser): 참가자 이름 규칙 검증 테스트 작성

75abc22

Test(InputParser): 이름 빈 문자열 예외 테스트 삭제

1a2c6a7

이름 리스트 공백 검증은 이름을 담을 vo에 존재해야하는 로직이라고 판단하여 InputParserTest에서 해당 테스트를 삭제함

Feat(Input): 플레이어 이름 입력 받아 파싱하기 구현

a57504f

Test(Name): 플레이어 이름 규칙 검증 테스트 작성

ff47665

Feat(Name): 플레이어 이름 Value Object 구현

7f6ab09

Docs(readme): 입력 기능 구현 완료에 따른 문서 업데이트

c8a707a

Feat(Input): 히트 여부 입력 메소드 구현

89db8c5

Docs(readme): 히트 여부 입력 구현 완료에 따른 문서 업데이트

fc5407f

Test(PlayerGroups): PlayerGroup 테스트 작성

b6f96a4

Feat(Player): Player와 PlayerGroups 도메인 생성

26ee0b8

Test(GameService): 플레이어 등록 테스트 작성

0ccde13

Test(Name): '딜러' 입력방지 테스트 삭제

c54f6a6

Refactor(Name): 딜러 예외 처리 삭제

162a945

Feat(GameService): 플레이어 등록 구현

410a9f6

Feat(Card): 카드덱 구현

b1866f0

Feat: 플레이어에게 초기 카드 나눠주기 구현

98241a4

Feat: 딜러와 플레이어들의 카드 보유 출력 구현

712d5c9

Feat: 플레이어 히트 구현

c58443b

플레이어 히트 로직 구현 버스트 판별 구현 카드 점수 계산 로직 구현 에이스 점수 결정 구현

Docs(readme): 플레이어 히트 구현에 따른 문서 업데이트

0fae0a9

Feat: 딜러 히트 구현

536adff

Feat: 최종 카드 내역과 합산 결과 출력 구현

ede4769

Feat: 승패 계산 및 출력 구현

9df5f4f

플레이어의 승패 계산 구현 딜러의 결과 계산 구현 최종 결과 출력 구현

sihyung92 requested changes Mar 8, 2026

View reviewed changes

choubung added 6 commits March 9, 2026 11:26

Docs(readme): 주요 기능 명세 추가

befa556

카드 덱 소모시 셔플해 초기화하는 기능 명세를 추가함

Chore: 중복 주석 제거

5161cea

Refactor(name): 정규식 상수화

2a526ab

Refactor(GameController): run()에서 메소드 분리

4442e9e

Feat(Participant): 딜러와 플레이어 공통부분을 위한 추상 클래스 작성

8be960e

Chore(CardDeck): 카드덱 셔플에 대한 주석 추가

ea06c4f

choubung added 25 commits March 9, 2026 17:19

Test(PlayerGroup): 정원 변동에 따른 테스트 수정

1a66fb0

Test(Card): 카드 정보 출력용 문자열로 변환 테스트 작성

8ca6ef2

Fix(Card): 출력용 문자열이 객체로 반환되는 오류 수정

50c657e

Fix(HandCards): Ace 계산 로직 수정

0e2429d

Refactor(ResultView): 매개 변수로 dto를 받도록 수정

b65a096

Refactor(GameService): dto 사용으로 인한 리팩토링과 게임 결과 로직 리팩토링

8481075

Test(HandCards): 손패 계산 로직 테스트 작성

c2e0bde

Move(HandCardsTest): 테스트 위치를 player 아래에서 participant 아래로 변경

b4670fb

Feat(Participant): 승리 여부 계산 추상 메소드 추가

368f0d1

Move(WinStatus): 파일 위치를 player 아래에서 participant 아래로 변경

de9a23e

Refactor(PlayerGroups): 매직 넘버 제거

e6a2d35

Test(GameService): 플레이어와 딜러 분리에 따른 테스트 변경사항 반영

d37ff24

Test(Player): 플레이어 승리 여부 판별 테스트 작성

27e731a

Refactor(Player): 생성자 변경

b1e2715

HandCards를 외부에서 주입할 수 있도록 변경함

Refactor(HandCards): 생성자 추가

a54c4c2

카드 리스트를 외부에서 받아 생성할 수 있도록 생성자 추가

Refactor(Participant): 생성자 변경

a1e106c

HandCards를 외부에서 주입받도록 생성자를 변경함

Test(PlayerGroups): 생성자 변경으로 인한 코드 수정

0606e66

Test(Dealer): 딜러 전적 계산 테스트 작성

2c59557

Feat(Dealer): 딜러 승패 전적 계산 로직 구현

e76bec3

Test: 테스트 이름 변경

f4d825f

Refactor(controller): 함수 이름 변경에 의한 리팩토링

562e002

Refactor(service): player 생성자 변경에 의한 리팩토링

a41bae1

Feat(Name): 딜러라는 이름 금지 검증 로직 추가

625fa64

Test(Name): 딜러 금지 검증 테스트 추가

fcd1186

Refactor(PlayerGroups): 예외 메시지 수정

15cc249

choubung added 3 commits March 10, 2026 15:51

Docs(readme): 기능 명세에 베팅 기능 내용 추가

c96a89c

Move(Participant): getCards 메소드 이동

19e2af4

자식 클래스들에서 Participant로 이동

Refactor(dealer): 딜러 이름 반환 방식 변경

1c5c380

>> 기존: 딜러 객체 생성 시 '딜러'를 name 필드에 저장 >> 변경 후: 메소드 오버라이딩으로 getName 호출 시 '딜러'를 반환하도록 함

sihyung92 requested changes Mar 10, 2026

View reviewed changes

	private static void validateNotDealer(String name){
	private static void validateNotDealer(String name) {

Conversation

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

Uh oh!

체크 리스트

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

1. 데이터 단순 조회의 getter와 잘못된 설계에 사용된 getter의 차이

2. 일급컬렉션의 사용

3. dto 구현의 선

4. Player와 Dealer

5. TDD

6. git commit의 역할

Uh oh!

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

sihyung92 Mar 10, 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!

choubung commented Mar 9, 2026

Uh oh!

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

choubung commented Mar 8, 2026 •

edited

Loading

sihyung92 Mar 10, 2026 •

edited

Loading