[2단계 - 블랙잭 베팅] 로빈(임수빈) 미션 제출합니다! #9

robinjoon · 2024-03-14T04:05:29Z

기능 변경 사항

승부 판단 규칙 변경

플레이어와 딜러가 모두 bust(21점이 넘는 경우) 플레이어의 승리

베팅 기능

플레이어는 양의 정수 만큼의 배팅을 할 수 있다.
게임의 결과에 따라 상금이 달라진다.
- 블랙잭(처음 뽑은 두개의 카드로 21점)인 경우 : 베팅 금액의 1.5배
- 블랙잭이 아닌 상태로 승리한 경우 : 베팅 금액의 1배
- 패배한 경우 : 베팅 금액을 잃는다.
- 비긴 경우 : 베팅 금액을 그대로 돌려 받는다

고민과 선택

정말 컨트롤러에는 도메인 로직 vs 도메인에 간접적인 뷰의 로직 주입

1단계 머지된 시점에서의 BlackJackController의 일부 코드 입니다.

private boolean playerTryDrawOnce(Deck deck, Player player) {
      boolean needToDraw = YesOrNoInputView.getYNAsBoolean(player.getRawName());
      DrawResult drawResult = null;
      if (needToDraw) {
          drawResult = playerDraw(deck, player);
      }
      printPlayer(player);
      if (drawResult == null) {
          return false;
      }
      return drawResult.hasNextChance();
}

컨트롤러에 플레이어가 카드를 뽑기 원하는지 입력을 받고, 이를 이용해 카드를 뽑는 비즈니스 로직이 포함되어 있습니다.

저는 컨트롤러의 역할이 단순히 모델과 뷰 사이의 통신을 대리하는 것이라 생각합니다. 따라서, 컨트롤러에는 비즈니스 로직이 포함되면 안된다(도메인 로직은 모델에서 수행하므로)고 결론 내렸습니다. 그 결과 다음과 같은 코드가 작성되었습니다.

public class BlackJackGame {
// 생략
    public void playersDraw(Deck deck, PlayerDrawAfterCallBack playerDrawAfterCallBack,
                            DrawConfirmation drawConfirmation) {
        players.draw(deck, playerDrawAfterCallBack, drawConfirmation);
    }
}

public class BlackJackController {
// 생략
    private void playersTryDraw(Deck deck, BlackJackGame blackJackGame) {
        blackJackGame.playersDraw(deck, this::printPlayer, YesOrNoInputView::getYNAsBoolean); 
    }
}

위와 같이 도메인에 함수형 인터페이스를 통해 뷰의 로직이 전달되는 구조가 되었습니다. 코드상에서 도메인이 뷰를 의존하지 않지만 결국 런타임에 도메인이 뷰의 로직을 주입받아 수행한다는 점에서 여전히 의존성이 남아있다고 생각됩니다. 덤으로 매개변수의 개수가 늘어나는 단점도 함께 가지게 되었습니다.

지금 시점에는 도메인 객체의 매개변수 개수가 아직은 감당 가능하고, 잘 동작하며, 코드상에 명시적인 의존성은 사라지기 때문에 도메인에 함수형 인터페이스를 통해 뷰의 로직을 간접적으로 주입받는 것을 선택했습니다.

질문

클래스 설계를 하지 않고 TDD를 하는게 맞나??

이번 미션에서도 네오 강의에서 배운대로 TDD로 미션을 수행했습니다. 네오 강의에서는 별도의 클래스 설계(어떤 클래스가 있고, 어떤 클래스와 어떤 클래스가 어떤 메세지를 주고 받는지) 없이 생각의 흐름대로 진행했습니다.

저번 사다리 타기 미션에서는 이 방식으로 잘 했는데, 이번 블랙잭 미션에서는 이 방식대로 했더니 코드를 리팩토링하는 것이 힘들 정도로 이상한 코드가 작성되었습니다.

코드를 리팩토링하는 것이 힘들 정도로 이상한 코드가 작성된 것이 제 리팩토링 능력 부족이라 그런건지, 애초에 리팩토링이 어렵게 될 수 밖에 없는 방식인건지 궁금합니다.

* docs: 요구사항 분석 및 기능 목록 작성 * feat: 덱 관리 기능 구현 * feat: 카드 점수 계산 기능 구현 * refactor: CardValue 를 CardName 으로 변경 * feat: 보유한 카드 점수 합계 계산 기능 구현 * feat: 플레이어 및 딜러 점수 계산 기능 구현 * fix: 플레이어가 카드를 뽑을 수 없는 경우에도 카드를 뽑을 수 있던 오류 수정 * fix: 딜러가 카드를 뽑을 수 없는 경우에도 카드를 뽑을 수 있던 오류 수정 * refactor: Player 와 Dealer 통합 * feat: 플레이어 이름 입력 기능 구현 * feat: 카드 받을지 여부 입력 기능 구현 * feat: 플레이어 및 딜러간 승부 계산 기능 구현 * refactor: 플레이어 및 딜러간 승부 계산 기능 Gamer 에서 분리 * feat: 게임 결과 출력 기능 구현 * docs: 기능 목록 정리 * feat: 플레이어 및 딜러 손패 출력 기능 구현 * feat: 게임 승부 결과 출력 기능 구현 * feat: 전체 게임 진행 기능 구현 * fix: 게이머가 죽었을 경우 승부가 잘못 판단되는 오류 수정 * feat: Ace 카드 점수 보정 기능 구현 * docs: TODO 목록 정리 * fix: Main 메서드에서 플레이어 이름을 입력받지 않는 오류 수정 * docs: 딜러 Ace 카드 점수 보정 기능 추가 * feat: 딜러 Ace 카드 점수 보정 기능 구현 * fix: 게임 도중 플레이어의 손패를 출력하지 않는 오류 수정 * fix: 딜러의 카드 한장 숨기도록 수정 * refactor: TODO 처리 * refactor: domain 하위 패키지 추가 * refactor: 클래스 및 패키지 접근지정자 조이기 * refactor: 메서드 분리 * style: 클래스와 제일 위에 위치한 필드(메서드) 사이에 공백 라인 제거 * refactor: 일부 매직 값을 상수 혹은 의미있는 변수로 추출 * fix: 플레이어 이름을 잘못 출력하는 버그 수정 * chore: 람다표현식으로 변경 * docs: 1단계 피드백 반영 예정 목록 추가 * refactor: 드로우가 가능한지 확인하는 책임 컨트롤러에서 Gamer로 이동 * refactor: 드로우 여부 결정하는 정책과 드로우 방식을 결정하는 정책 분리 * refactor: Card Enum화 * refactor: 테스트 코드에서 사용하는 테스트 객체 생성 역할을 분리 * test: 게이머의 점수가 잘 계산되는지 검증하는 테스트 추가 * refactor: view만을 위한 enum 추가 * chore: 필요 없는 TODO 주석 제거 * refactor: Controller 내부 메서드 분리 * fix: 플레이어가 딜러의 전략을 사용하는 오류 수정 * refactor: 딜러와 플레이어 분리 상속이 아니라 조합을 이용함 * refactor: 딜러의 카드 하나 숨기는 기능 Dealer로 이관 * refactor: Main, Controller, View 역할 명확하게 분리 * refactor: Dealer 에서 이름 제거, View로 이관 * refactor: GamerOutputView를 DealerOutputView 와 PlayerOutputView로 분리 * refactor: BlackjackController 내부 메서드 정리 * refactor: Players 일급 컬렉션 추가 * refactor: Players 생성자 변경 * chore: 사용하지 않는 메서드 제거 * refactor: RandomCardSelectStrategy 싱글톤으로 변경 * fix: 출력문구 누락 수정 * refactor: BlackJackGameMachine 내부 메서드 이름 및 접근 지정자 변경 --------- Co-authored-by: BurningFalls <[email protected]>

미션 페이지의 요구사항 문장을 잘못 해석했었다.

…변수 수정

robinjoon · 2024-03-16T03:57:34Z

리뷰어 피드백 반영해서 코드 수정했습니다.

3Juhwan

로빈~~~ 리뷰가 늦었지? 😢
코드 잘 읽었어! 특히 BlackJackGame이 인상적이었어. 나도 시도해 보고 싶었던 설계였거든.
다만 이번에 리뷰를 꼼꼼하게 모든 클래스를 보진 못했는데, 코드가 내가 읽기에 너무 수준이 높달까..

코드를 읽으면서 느낀 점은 다음과 같아. 이 점을 개선해 주면 좋을 것 같아! 로빈 코드 꼭 완벽히 이해하고 싶어 😀

패키지 분리의 이점을 못 느꼈어. blackjack 패키지가 사실상 모든 로직을 담고 있다고 생각해. 패키지 내부에 존재하는 클래스에 package-default 등의 적절한 접근 제어자를 썼더라고. 하지만 패키지가 사실상 전부이고 이런 접근 제어자가 의미가 있을까? 라는 생각이 들더라고
코드를 읽는 진입점을 찾기 어려웠어. 위와 비슷한 맥락이야. 로빈이 세세하게 추상화를 해뒀는데 구현에 의존하는 코드가 많아서 결국 모든 코드를 읽어야 되더라고 😢 적절하게 패키지를 분리하고 인터페이스에만 의존하게 코드를 개선한다면 더 읽기 편할 것 같아!

블랙잭 2단계 고생 많았어!! 머지까지 힘내자구 💪💪

3Juhwan · 2024-03-17T01:31:00Z

src/main/java/domain/blackjack/Gamer.java

+
+    public final List<Card> getRawHoldingCards() {
+        return getRawHoldingCards(AllCardShowStrategy.INSTANCE);
+    }
+


AllCardShowStrategy 구현에 의존하고 있는데, 이렇게 작성한 로빈의 의도가 궁금해요 💭
비슷하게 WithOutFirstCardShowStrategy도 컨트롤러에서 의존하고 있네요!
CardShowStrategy 인터페이스로 분리한 이점이 무엇이 있나요?

3Juhwan · 2024-03-17T01:34:39Z

src/main/java/domain/blackjack/DealerCardDrawCondition.java

+
+public final class DealerCardDrawCondition implements CardDrawCondition {
+    public static final int RAW_DEALER_DRAW_THRESHOLD_POINT = 16;
+    private final BlackJackGameMachine blackJackGameMachine;
+


이 부분도 상수를 외부에서 사용하고 있네요!
로빈의 의도를 조금은 이해가 되는데요~ 비즈니스 정책을 관리하기 위함이 맞나요?
맞다면, 이를 위해 인터페이스를 분리하고 여러 구현체를 만든 것으로 파악되는데요.
과한/잘못된 추상화가 아닌가? 라는 생각이 드네요! 외부에서 구현에 의존하기도 하고요.
클래스, enum 등으로 비즈니스 정책을 관리하는 것은 어떨까요? 로빈의 의견이 굉장히 궁금해요 💭

3Juhwan · 2024-03-17T01:57:02Z

src/main/java/domain/card/CardPool.java

+public class CardPool {
+    static final Map<CardType, Map<CardName, Card>> pool = new ConcurrentHashMap<>();
+


동시성 👍

3Juhwan · 2024-03-17T01:58:56Z

src/main/java/domain/card/CardPool.java

+
+    public static Card get(CardType cardType, CardName cardName) {
+        return pool.get(cardType).get(cardName);
+    }
+}


테스트에서만 사용하는 것 같은데, 접근 제어자를 수정하면 좋을 것 같아요 💭

reddevilmidzy

마지막까지 화이팅해봅시당!!

reddevilmidzy · 2024-03-17T02:33:16Z

src/main/java/domain/blackjack/BettingMoney.java

+        if (rawBattingMoney < 0) {
+            throw new IllegalArgumentException("배팅 금액은 음수일 수 없습니다.");
+        }
+    }


0원도 배팅가능하게 하신건가요?

reddevilmidzy · 2024-03-17T02:45:05Z

src/main/java/view/BattingMoneyInputView.java

+        if (rawNumber <= 0) {
+            throw new IllegalArgumentException("입력 형식이 올바르지 않습니다.");


0이 입력되면 여기서 걸리겠군요

요 예외메시지는 그럼 형식이 올바르지 않다기보다 범위가 올바르지 않은 느낌 아닌가요?!

reddevilmidzy · 2024-03-17T02:46:37Z

src/main/java/view/OutputView.java

+    }
+
+    public static void printDealerDrawDone() {
+        print("딜러는 %d이하라 한장의 카드를 더 받았습니다.\n".formatted(RAW_DEALER_DRAW_THRESHOLD_POINT));


여기도 /n 대신 %n로 변경해주면 좋을 거 같아요

reddevilmidzy · 2024-03-17T02:47:40Z

src/main/java/view/OutputView.java

+    private static void print(String output) {
+        System.out.println(output);
+    }
+
+    public static void printDealerDrawDone() {


요 둘 메서드 순서도 public 이 위에 있는게 좋아보여요..!

요 둘 메서드 순서도 public 이 위에 있는게 좋아보여요..!

메서드 선언 순서는 종속된 메서드가 바로 아래에 오도록 설정했어요.

reddevilmidzy · 2024-03-17T02:57:09Z

src/main/java/domain/blackjack/DrawResult.java

+    static DrawResult success(boolean hasNextChance) {
+        return new DrawResult(null, hasNextChance);
+    }
+
+    static DrawResult fail(Exception drawFailCause, boolean hasNextChance) {
+        return new DrawResult(drawFailCause.getMessage(), hasNextChance);
+    }
+
+    static DrawResult fail() {
+        return new DrawResult("카드를 더이상 뽑을 수 없습니다.", false);
+    }
+
+    boolean hasNextChance() {
+        return hasNextChance;
+    }
+
+    boolean isSuccess() {
+        return failCause == null;
+    }


success 한다면 failCause에 null 을 넣고 확인을 failCause == null 로 확인하는 것 말고, null 을 사용하지 않는 방법은 없을까요?

success 한다면 failCause에 null 을 넣고 확인을 failCause == null 로 확인하는 것 말고, null 을 사용하지 않는 방법은 없을까요?

이것 저것 시도해봤는데 null 을 사용하는 것이 가장 명확하고 간단했어요.

reddevilmidzy · 2024-03-17T03:01:46Z

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

+import domain.card.RandomCardSelectStrategy;
+
+public class Player extends Gamer {
+    private final String name;


요 이름도 객체 만들어서 포장해주는건 어떨까요?>

robinjoon and others added 30 commits March 12, 2024 20:09

docs: 리팩토링 목록 정리

6fdbd66

refactor: GameResultCalculator가 Gamer를 매개변수로 받도록 리팩토링

22395e0

refactor: BlackJackController 에서 BlackJackGame 추출 및 도메인 클래스 접근 지정자 수정

cdbf2b8

docs: 2단계 요구사항 정리

9e6fab1

docs: 2단계 기능 목록 추가

737140a

feat: 게임 결과 계산 기능(블랙잭인 경우 포함) 구현 및 필요 없는 기능 삭제

56f35f8

feat: 배팅 금액 등록 기능 및 상금 계산 기능 구현

5da19d0

docs: 딜러의 카드 공개 관련 규칙 설명 추가

4db6b46

chore: 잘못된 todo 주석 제거

6542633

feat: 최종 수익 계산 및 출력 기능 구현

fd60504

feat: 배팅 금액 입력 기능 구현

96e59f6

fix: 딜러와 플레이어가 모두 블랙잭인 경우 요구사항 및 기능 수정

1388e32

미션 페이지의 요구사항 문장을 잘못 해석했었다.

fix: 발생하지 않는 예외를 catch 하는 코드 삭제

8d4ba57

refactor: 내부 메서드 정리

1e334c0

refactor: 상금 계산 기능 Player로부터 분리

40dda18

refactor: 블랙잭 판단 위치 BlackJackGameMachine으로 이동, DrawResult.fail 메서드 매개…

f758291

…변수 수정

refactor: 딜러의 드로우 조건을 표현하는 정수 상수화

1023f23

refactor: 상금 계산 관련 테스트 코드 위치 변경

9c34ffe

test: 블랙잭 여부 판단 테스트 추가

35b4953

refactor: 카드 일반 클래스로 변경

02383e8

docs: 피드백 반영 예정 목록 정리

138c61f

docs: 피드백 반영 예정 목록 추가 정리

f6fc4fd

refactor: 딜러의 손패를 보여주는 행동을 전략패턴을 이용하도록 변경

bbf318a

refactor: enum 활용해 if문 제거

66cc1e5

fix: WithOutFirstCardShowStrategy 내부 메서드 구현이 안된 것 수정

15b064b

refactor: stream map 내부 출력문 변경

68acdb6

refactor: 메서드 이름 변경

e35d1e8

refactor: 필요없는 메사드 삭제

5a4e3e8

refactor: Card pool 코드 분리

3ba5bfe

robinjoon added 8 commits March 15, 2024 17:01

refactor: BlackJackGame 의 테스트 코드에서만 사용하는 생성자 삭제

517db60

refactor: 플레이어와 딜러에서 카드를 선택하는 전략을 선택할 수 없도록 리팩토링

0e97502

test: 필요 없는 테스트 삭제

d6a2b91

test: BettingMoney, EarningMoney 클래스 분리

444388d

chore: 메서드 선언 순서 변경

6ba4d5a

refactor: DealerOutputView, PlayerOutputView 통합

8e68929

chore: BlackJackController의 메서드 네이밍 수정

9e81067

refactor: 표준 함수형 인터페이스 대신 의미를 부여한 인터페이스 정의

46323ae

3Juhwan reviewed Mar 17, 2024

View reviewed changes

reddevilmidzy reviewed Mar 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2단계 - 블랙잭 베팅] 로빈(임수빈) 미션 제출합니다! #9

[2단계 - 블랙잭 베팅] 로빈(임수빈) 미션 제출합니다! #9

robinjoon commented Mar 14, 2024 •

edited

Loading

robinjoon commented Mar 16, 2024

3Juhwan left a comment

3Juhwan Mar 17, 2024 •

edited

Loading

3Juhwan Mar 17, 2024 •

edited

Loading

3Juhwan Mar 17, 2024

3Juhwan Mar 17, 2024

reddevilmidzy left a comment

reddevilmidzy Mar 17, 2024

reddevilmidzy Mar 17, 2024

reddevilmidzy Mar 17, 2024

reddevilmidzy Mar 17, 2024

robinjoon Mar 17, 2024

reddevilmidzy Mar 17, 2024

robinjoon Mar 17, 2024

reddevilmidzy Mar 17, 2024

		public class CardPool {
		static final Map<CardType, Map<CardName, Card>> pool = new ConcurrentHashMap<>();

		if (rawNumber <= 0) {
		throw new IllegalArgumentException("입력 형식이 올바르지 않습니다.");

[2단계 - 블랙잭 베팅] 로빈(임수빈) 미션 제출합니다! #9

Are you sure you want to change the base?

[2단계 - 블랙잭 베팅] 로빈(임수빈) 미션 제출합니다! #9

Conversation

robinjoon commented Mar 14, 2024 • edited Loading

기능 변경 사항

승부 판단 규칙 변경

베팅 기능

고민과 선택

정말 컨트롤러에는 도메인 로직 vs 도메인에 간접적인 뷰의 로직 주입

질문

클래스 설계를 하지 않고 TDD를 하는게 맞나??

robinjoon commented Mar 16, 2024

3Juhwan left a comment

Choose a reason for hiding this comment

3Juhwan Mar 17, 2024 • edited Loading

Choose a reason for hiding this comment

3Juhwan Mar 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reddevilmidzy 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

robinjoon commented Mar 14, 2024 •

edited

Loading

3Juhwan Mar 17, 2024 •

edited

Loading

3Juhwan Mar 17, 2024 •

edited

Loading