[1단계 - 블랙잭 게임 실행] 망쵸(김주환) 미션 제출합니다. #2

3Juhwan · 2024-03-08T09:05:43Z

안녕하세요~

페어 땡이와 함께 열심히 미션을 진행했어요. 저는 페어 프로그래밍 과정에서 얻을 수 있는 소프트 스킬 역량에 집중하는 편인데요! "함께 즐겁게 코딩하자"가 저의 목표랍니다. 그래서 1단계 미션 코드에 부족함이 여실 없이 드러나네요 😓
그래도 코드 한 줄 한 줄 고민하고 토론하며 작성했어요. 좋은 피드백, 따끔한 피드백 모두 감사합니다. 😀

Co-authored-by: [email protected]

Co-authored-by: J-I-H-O<[email protected]>

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

seokmyungham

망쵸 수고했엉 😊

seokmyungham · 2024-03-10T07:12:10Z

src/main/java/blackjack/domain/cardgame/CardDeck.java

+        for (final CardNumber cardNumber : CardNumber.values()) {
+            for (final CardShape cardShape : CardShape.values()) {
+                deck.push(new Card(cardNumber, cardShape));
+            }


indent를 1까지만 허용한다는 요구 사항이 있기도 하니까 메서드를 분리해보는 건 어떨까요?
아니면 Stream을 사용하는 것도 방법일 것 같습니다. 😄

seokmyungham · 2024-03-10T07:12:26Z

src/main/java/blackjack/domain/cardgame/CardDeck.java

+import java.util.Stack;
+
+public class CardDeck {
+    private final Stack<Card> deck;


많은 자료구조 중에 Stack을 선택하신 이유가 궁금합니다!!

seokmyungham · 2024-03-10T07:27:08Z

src/main/java/blackjack/domain/player/Dealer.java

+import blackjack.domain.card.Card;
+
+public class Dealer extends Player {
+    private static final String NAME = "딜러";


망쵸~ 저도 이렇게 Dealer 안에 상수로 "딜러"라고 정의하도록 구현했었어요.
그런데 이와 관련해서 리뷰어분께 리뷰를 받게 되었습니다.

리뷰 내용은

딜러에게 굳이 이름이 필요할까요?

입력과 관련없는 "딜러" 라는 딜러의 이름을 표현하는 책임은, 딜러가 맞을까요?

망쵸는 이와 관련해서 어떻게 생각하시나요~? 😊

seokmyungham · 2024-03-10T07:31:42Z

src/main/java/blackjack/domain/player/Hand.java

+            if (minimumSum + 10 <= 21) {
+                minimumSum += 10;
+            }


상수 분리의 필요를 TODO로 남겨 놓으셨군요!!
그런데 놓치신 것 같으니 제가 한 번더 리뷰로 알려드리겠습니다 ㅎㅎㅎ 😄

다른 개발자가 이 코드를 처음 보게되었을 때,
저 10을 바로 이해할 수 있을지 고민해보셔도 좋을 것 같아요 !

seokmyungham · 2024-03-10T07:32:43Z

src/main/java/blackjack/domain/player/Name.java

+        final String errorMessage = String.format("[ERROR] 이름의 길이는 %s 이상, %s 이하여야 합니다.",
+                MINIMUM_LENGTH, MAXIMUM_LENGTH);
+        throw new IllegalArgumentException(errorMessage);
+    }


저도 이렇게 상수를 활용해서 에러 메시지를 구체화 하는거 정말 좋아합니다 😊

seokmyungham · 2024-03-10T08:02:34Z

src/main/java/blackjack/domain/cardgame/CardGame.java

+    public void initializeHand(final Dealer dealer, final List<Player> players) {
+        giveCard(dealer);
+        giveCard(dealer);
+        giveTwoCardsEachPlayer(players);
+    }
+
+    private void giveTwoCardsEachPlayer(final List<Player> players) {


파라미터로 넘긴 값을 변경하지 않도록 수정해야할 것 같은데 망쵸는 어떻게 생각하시나요??

현재 값의 변경 책임이 참조로 전달된 파라미터 객체 외부에 있다는 생각이 듭니다.
이러면 final 키워드를 사용하는 의미도 퇴색된다고 생각하고
디버깅도 어려울 뿐만 아니라 유지보수 하기 힘들어질 것이라 생각해요.

가장 단순하게 제 생각에는 CardGame이 Dealer를 갖고 있도록 변경하거나 아니면 파라미터 객체에 직접 책임을 지도록 하는 방법이 있을 것 같아요.

players는 일급 컬렉션이 따로 존재하지 않으니 메서드 지역변수로 생성해서 반환하는 방법이 존재할 것 같습니다.

robinjoon

수고 많으셨습니다! 코드가 되게 간단하네요!

robinjoon · 2024-03-10T06:15:52Z

src/test/java/blackjack/domain/cardgame/CardGameJudgeTest.java

+    void 카드_합계가_딜러와_플레이어_모두_21_이하인_경우_숫자가_큰_사람이_승리한다() {
+        CardGameJudge cardGameJudge = new CardGameJudge();
+
+        Player player = player(new Card(ACE, HEART));
+        Dealer dealer = dealer(new Card(KING, SPADE));
+
+        var result = cardGameJudge.judge(dealer, List.of(player))
+                .getTotalResult();
+
+        assertThat(result.get(player)).isEqualTo(WinningStatus.LOSE);
+    }


테스트 터져요!! 😭

robinjoon · 2024-03-10T08:23:49Z

src/main/java/blackjack/view/PlayerChoice.java

+    HIT("y"),
+    STAND("n");


[질문] y, n 이라는 문자는 view의 관심사 같아요. view가 변경되면 같이 변경되어야 하는데, 대안은 없을까요?

robinjoon · 2024-03-10T08:26:44Z

src/test/java/blackjack/domain/player/HandTest.java

+    @Test
+    void 가진_패의_숫자의_합계를_구할_수_있다() {
+        Hand hand = new Hand();
+        hand.add(new Card(JACK, SPADE));
+        hand.add(new Card(QUEEN, SPADE));
+        hand.add(new Card(KING, SPADE));
+
+        int sum = hand.getSum();
+
+        assertThat(sum).isEqualTo(30);
+    }
+}


[의견] Ace를 처리하는 부분은 테스트가 수행되지 않네요!

robinjoon · 2024-03-10T08:28:18Z

src/test/java/blackjack/domain/player/PlayerTest.java

+    @Test
+    void 플레이어는_죽었는지_여부를_반환한다() {
+        Player player = player(
+                new Card(KING, CLOVER),
+                new Card(KING, SPADE),
+                new Card(KING, HEART));
+
+        boolean isAlive = player.isAlive();
+
+        assertThat(isAlive).isFalse();
+    }
+}


[의견] 플레이어가 죽는 시점은 합이 22가 되었을 때 입니다. 이 테스트는 합이 30이 되었을 때 죽는지 확인하는 것이므로, 22 ~ 29 에서 플레이어가 죽는지 알 수 없습니다!

다른 테스트 코드에서도 마찬가지로 경계값 테스트가 수행되지 않고 있습니다!

hyxrxn

흐름은 이해가 잘 되는 코드라고 생각해요!
다만 메서드 라인 제한, indent 제한 등 요구사항이 지켜지지 않은 부분이 종종 보이네요.
일단 요구사항이 있다면 잘 지켜야 한다고 생각합니다!
그리고 테스트에서 클래스 다음 줄이 공백인 경우도 있고, 아닌 경우도 있는데 규칙을 정해 통일하면 더 보기 좋겠네요!

hyxrxn · 2024-03-10T13:10:51Z

src/main/java/blackjack/BlackjackController.java

+import blackjack.domain.cardgame.CardGame;
+import blackjack.domain.cardgame.CardGameJudge;
+import blackjack.domain.cardgame.CardGameResult;
+import blackjack.domain.player.Dealer;
+import blackjack.domain.player.Player;
+import blackjack.view.InputView;
+import blackjack.view.OutputView;
+
+import java.util.List;


구글 자바 컨벤션에서는 non static import 사이에 엔터가 없는 게 규칙이더라구요~~~

hyxrxn · 2024-03-10T13:13:18Z

src/main/java/blackjack/domain/card/CardShape.java

+    SPADE("스페이드"),
+    HEART("하트"),
+    DIAMOND("다이아몬드"),
+    CLOVER("클로버");


출력 형식이 하트에서 HEART로 변경되면 도메인이 함께 바뀌어야 하네요.
이 부분은 View의 역할 아닐까요?
CardNumber과 WinningStatus도 마찬가지 의견입니다!

hyxrxn · 2024-03-10T14:57:50Z

src/main/java/blackjack/view/OutputView.java

+    }
+
+    private static String getCardInfo(final Card card) {
+        return card.getNumber() + card.getShape();


getNumber를 하면 getValue를 하고, j, q, k의 value는 모두 10이군요...!
에이스 또한 a로 출력되지 않고 1로 출력됩니다.

hyxrxn · 2024-03-10T15:00:53Z

src/main/java/blackjack/domain/cardgame/CardGame.java

+    public void initializeHand(final Dealer dealer, final List<Player> players) {
+        giveCard(dealer);
+        giveCard(dealer);
+        giveTwoCardsEachPlayer(players);
+    }


딜러에게 두장을 주는 부분은 그대로 두고 플레이어들에게 두장 주는 부분은 메서드로 따로 뺀 이유가 있나요?
그리고 initializeHand라는 메서드명으로는 두 장씩 준다는 것을 모를 수 있을 것 같은데 어떻게 생각하시나요?!

hyxrxn · 2024-03-10T15:05:40Z

src/main/java/blackjack/domain/player/Hand.java

+        final int aceCount = getAceCount();
+        for (int i = 0; i < aceCount; i++) {
+            if (minimumSum + 10 <= 21) {
+                minimumSum += 10;
+            }


에이스가 4장 있어도 한 장만 10점을 더할 수 있는데, 그러면 의미없는 for문을 세 번 더 돌게 되네요!

hyxrxn · 2024-03-10T15:35:36Z

src/main/java/blackjack/view/OutputView.java

+    }
+    public static void printPlayerCardWithScore(final Player player) {


Suggested change

}

public static void printPlayerCardWithScore(final Player player) {

}

public static void printPlayerCardWithScore(final Player player) {

공백이 없습니다

hyxrxn · 2024-03-10T15:40:07Z

src/main/java/blackjack/view/OutputView.java

+        final String initialDistributionMessage = String.format("%s와 %s에게 2장을 나누었습니다.", dealer.getName(), names);
+        System.out.println(initialDistributionMessage);


printf를 사용하면 될 듯 한데... 다른 메서드들도 그렇고 출력 부분을 변수설정 + 출력 이렇게 두 줄로 나눠서 출력하는 이유는 무엇인가요?
String 변수를 따로 이름 붙이는 것이 가독성이 좋다고 생각해서인가요?
아니면 다른 이유가 있을까요?

hyxrxn · 2024-03-10T15:43:11Z

src/main/java/blackjack/view/InputView.java

+    private static void printPlayerNamesInputMessage() {
+        System.out.println("게임에 참여할 사람의 이름을 입력하세요.(쉼표 기준으로 분리)");
+    }
+
+    private static void printAskingForAnotherCardMessage(final String name) {
+        printLineSeparator();
+        System.out.println(name + "는 한장의 카드를 더 받겠습니까?(예는 " + HIT.getMessage() +
+                ", 아니오는 " + STAND.getMessage() + ")");
+    }


InputView에 print로 시작하는 메서드가 있는 것이 조금 어색하네요.
저는 이 또한 출력에 해당한다고 생각해 OutputView에 넣었는데, 이 부분에 대해 망쵸는 어떻게 생각하나요?

hyxrxn · 2024-03-10T15:52:04Z

src/main/java/blackjack/view/PlayerChoice.java

+        this.message = message;
+    }
+
+    static boolean isDrawable(final String choice) {


메서드명만 보면 isAlive와 비슷한 역할을 할 것 같아요!

hyxrxn · 2024-03-10T16:02:59Z

src/test/java/blackjack/domain/cardgame/CardGameJudgeTest.java

+import blackjack.domain.cardgame.CardGameJudge;
+import blackjack.domain.cardgame.WinningStatus;


사용하지 않는 import가 있네요... ㅎㅎ
인텔리제이에 이것도 자동으로 정리해주는 기능이 있는 것으로 아는데 사용을 추천합니닷

- 정적 팩토리 메서드 추가 - 접근 제어자를 적절하게 수정

- 합계를 구하는 로직 수정 - 접근 제어자 수정

3Juhwan and others added 30 commits March 5, 2024 15:22

docs: 기능 요구 사항 작성

6a4844d

Co-authored-by: [email protected]

feat: 플레이어에게 카드를 분배하는 기능 구현

34f668e

Co-authored-by: J-I-H-O<[email protected]>

feat: 각 플레이어에게 2장씩 카드를 분배하는 기능 구현

8163280

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: 가진 패의 합계를 구하는 기능 구현

4d55c54

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: 플레이어가 죽었는지 여부를 반환하는 기능 구현

b180f62

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: 카드 덱에서 무작위 카드를 뽑는 기능 구현

5135ec3

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: 딜러와 단일 플레이어의 승패를 판단하는 기능 구현

49b5168

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: 딜러와 여러 플레이어의 승패를 판단하는 기능 구현

280ef53

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

refactor: 플레이어에 픽스처 적용

14abcca

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

refactor: Dealer 클래스 분리 및 픽스쳐 수정

987e856

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

refactor: 픽스쳐 내 중복 제거

12bbf04

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

docs: 게임 승패 결정 로직 작성

240e408

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: 승패 판단 기능 구현

933b422

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

chore: 불필요한 클래스 삭제

b42c767

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

docs: 뷰에 대한 기능 요구사항 추가

dc3387f

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: 딜러의 승패 횟수 계산 기능 구현

043a26f

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: InputView 구현

2ad912d

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: Player 이름 길이를 검증하는 기능 구현

f252caa

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: 딜러와 플레이어 전원에게 초기에 분배한 카드 출력하는 기능 구현

cdb2713

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: 플레이어가 보유한 모든 카드를 출력하는 기능 구현

710ec42

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: 딜러가 추가 카드를 발급 받았는지 여부 출력하는 기능 구현

972dee8

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: 보유한 모든 카드의 합을 출력하는 기능 구현

4371415

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: 최종 승패를 출력하는 기능 구현

8bb8fd2

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: controller에서 사용자에게 처음 카드 2장씩 부여하는 기능 구현

35de854

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

fix: 플레이어 카드를 출력하는 구문 수정

2b06dd1

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: Controller 전체 기능 구현

0f5681d

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

refactor: 딜러, 플레이어 생성자에 매개변수 최소화

5d02370

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

refactor: 딜러 클래스의 메서드명 수정

6e32df2

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

refactor: 도메인 용어로 변경 (draw -> hit)

4940a10

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

refactor: 패키지 분리

cbe46ac

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

3Juhwan and others added 5 commits March 8, 2024 16:09

refactor: view, controller 리팩터링

0972560

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

refactor: 도메인 로직 리팩터링

ac428c6

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

feat: 에이스 관련 처리 기능 추가

8ba77c3

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

docs: 구현한 사항 체크

c29212d

Co-authored-by: MangCho <[email protected]> Co-authored-by: J-I-H-O <[email protected]>

chore: 잘못된 변수명 수정

c5555e5

seokmyungham reviewed Mar 10, 2024

View reviewed changes

robinjoon reviewed Mar 10, 2024

View reviewed changes

hyxrxn reviewed Mar 10, 2024

View reviewed changes

3Juhwan added 21 commits March 11, 2024 10:59

fix: 카드 계산 로직 변경으로 테스트 결과 수정

a0e2241

refactor: Deck 리팩터링

c62821a

- 정적 팩토리 메서드 추가 - 접근 제어자를 적절하게 수정

test: Deck에 52장의 카드가 있는지 검증하는 테스트

78e94fd

refactor: Hand 클래스 수정

d1907c8

- 합계를 구하는 로직 수정 - 접근 제어자 수정

test: Hand 클래스 테스트

e23c9f5

refactor: 접근 제어자 수정

944578d

test: DealerTest 추가

c30e9e4

refactor: 메서드 분리

6de9287

refactor: Judge의 역할을 Dealer로 옮김

a4df5f4

refactor: 게임 로직 설정 정보 이동

1b1d5a0

refactor: 출력 메시지를 Outview로 위임

c205252

refactor: CardGameResult를 record로 변경

731dea0

refactor: OutputView에서 메시지를 생성하는 로직을 MessageResolver로 분리

adb40af

style: 불필요한 공백 제거

1b67385

refactor: 덱 생성 로직에 스트림 적용

6770804

refactor: 불필요한 위임 메서드 삭제

f682149

test: 게임 결과에서 플레이어가 순서대로 반환되는지 테스트

0279a2e

refactor: CardGame의 역할을 Deck으로 이동

e983f17

refactor: 승패 결과에 대한 결과 문자열을 뷰로 이동

c2aafef

refactor: BUST 조건에 대한 다른 클래스의 의존성 제거

07d47c6

refactor: 게임 결과를 생성하는 로직을 CardGameResult로 이동

3670388

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 블랙잭 게임 실행] 망쵸(김주환) 미션 제출합니다. #2

[1단계 - 블랙잭 게임 실행] 망쵸(김주환) 미션 제출합니다. #2

3Juhwan commented Mar 8, 2024 •

edited

Loading

seokmyungham left a comment

seokmyungham Mar 10, 2024

seokmyungham Mar 10, 2024

seokmyungham Mar 10, 2024

seokmyungham Mar 10, 2024

seokmyungham Mar 10, 2024

seokmyungham Mar 10, 2024 •

edited

Loading

seokmyungham Mar 10, 2024

robinjoon left a comment

robinjoon Mar 10, 2024

robinjoon Mar 10, 2024

robinjoon Mar 10, 2024

robinjoon Mar 10, 2024

robinjoon Mar 10, 2024

hyxrxn left a comment

hyxrxn Mar 10, 2024

hyxrxn Mar 10, 2024

hyxrxn Mar 10, 2024

hyxrxn Mar 10, 2024

hyxrxn Mar 10, 2024

hyxrxn Mar 10, 2024

hyxrxn Mar 10, 2024

hyxrxn Mar 10, 2024

hyxrxn Mar 10, 2024

hyxrxn Mar 10, 2024

		}
		public static void printPlayerCardWithScore(final Player player) {

		final String initialDistributionMessage = String.format("%s와 %s에게 2장을 나누었습니다.", dealer.getName(), names);
		System.out.println(initialDistributionMessage);

		import blackjack.domain.cardgame.CardGameJudge;
		import blackjack.domain.cardgame.WinningStatus;

[1단계 - 블랙잭 게임 실행] 망쵸(김주환) 미션 제출합니다. #2

Are you sure you want to change the base?

[1단계 - 블랙잭 게임 실행] 망쵸(김주환) 미션 제출합니다. #2

Conversation

3Juhwan commented Mar 8, 2024 • edited Loading

seokmyungham left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seokmyungham Mar 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robinjoon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hyxrxn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

3Juhwan commented Mar 8, 2024 •

edited

Loading

seokmyungham Mar 10, 2024 •

edited

Loading