[2단계 - 블랙잭 베팅] 알파카(최휘용) 미션 제출합니다. #714

slimsha2dy · 2024-03-14T08:51:57Z

안녕하세요 현구막!
2단계에서는 1단계의 구조를 수정하지 않고, 베팅을 관리하는 클래스를 하나 추가해서 처리하도록 구현했습니다.
해당 클래스는 처음 베팅 금액을 입력받아 생성되고, 이후 결과를 인자로 받아 최종 결과를 Map으로 반환하는 기능만 처리하고 있습니다.
또한 이를 위해 정수 값을 포장한 베팅 금액 클래스를 구현했습니다.
이번 2단계 미션 리뷰도 잘 부탁드립니다!

그리고 저번 1단계 때 제가 의견을 남기지 않은 리뷰가 있었습니다..

첫번째 턴인지... 등도 사실 controller 와 관계없는 비즈니스 로직이죠!
해당 내용을 BlackJackGame 에게 질문할 수도 있겠네요!

못 본 것은 아니고 비슷한 내용의 리뷰에 남긴 의견에 포함되어 있었어서 지나쳤던 것 같습니다.
해당 내용을 언급했어야 했는데 죄송합니다ㅜㅜ 열심히 하겠습니다 🫡

Hyeon9mak

안녕하세요 알파카! 최소한의 구현으로 요구사항을 충족하고자 하셨군요~ 👍
그러나 요구사항에 만족되지 않은 부분들이 있는 것 같아요.
README.md 파일에 요구사항을 추가로 정리해보면 좋겠습니다!

그 외 몇 가지 코멘트 남겼으니 확인해주세요.
궁금하신 점 언제든지 질문 남겨주세요~!

Hyeon9mak · 2024-03-15T08:07:12Z

README.md

+- [ ] 최종 수익을 출력한다
+    - [ ] 이름과 수익 액수를 출력한다
+    - [ ] 승리하면 베팅 금액만큼 수익을 거둔다
+    - [ ] 패배하면 베팅 금액만큼 손해를 거둔다
+    - [ ] 무승부이면 최종 수익은 0이다


👀 블랙잭은 상금이 다르다는 요구사항도 있었던거 같아요.

기본적인 요구사항을 지나쳐버렸네요 죄송합니다.. 🙇
BlackJackResult에서 매개변수로 점수 대신 핸드의 정보를 받도록 수정해 블랙잭인지 판단하고 결과로 반환할 수 있도록 기능을 추가했습니다!

Hyeon9mak · 2024-03-15T08:08:30Z

src/main/java/domain/BettingAmount.java

+    public BettingAmount(String input) {
+        int amount = parseAmount(input);
+        validatePositive(amount);
+        this.amount = amount;
+    }
+
+    private int parseAmount(String input) {
+        try {
+            return Integer.parseInt(input);
+        } catch (NumberFormatException e) {
+            throw new IllegalArgumentException("베팅 금액은 양의 정수여야 합니다");
+        }
+    }
+
+    private void validatePositive(int input) {
+        if (input <= 0) {
+            throw new IllegalArgumentException("베팅 금액은 양의 정수여야 합니다");
+        }
+    }


베팅 금액이 문자열로 전달된다는 것까지도 비즈니스 규칙인지 고민해볼 수 있겠네요!
정수인지 문자열인지... 까지는 view 의 책임으로도 볼 수 있겠어요.

필요한 타입으로 변경해주는 것까지 View에서 처리하는게 맞다고 생각했는데 예외 처리를 한 번에 하기 위해서 위의 코드처럼 짰던 것 같습니다.. view의 책임이 맞는 것 같아 말씀하신 것처럼 수정했습니다!

Hyeon9mak · 2024-03-15T08:12:43Z

src/test/java/domain/BettingTableTest.java

+public class BettingTableTest {
+    @Test
+    @DisplayName("입력받은 Gamer들의 결과에 따라 최종 수익을 반환한다")
+    void initWithMap() {
+        BettingTable bettingTable = new BettingTable(Map.of("test", new BettingAmount("1000")));
+        Assertions.assertThat(bettingTable.getTotalProfit(Map.of("test", GamerResult.WIN)))
+                .isEqualTo(Map.of(
+                        GamerIdentifier.DEALER_IDENTIFIER, -1000,
+                        "test", 1000
+                ));
+    }


확인하고 싶은 내용은 게이머와 딜러의 상금이 반대로 간다이므로,
베팅에 참여하는 게이머 수를 늘리고, 딜러의 상금 결과가 얼마가 되는지 정확히 확인해보면 더 좋겠네요!

넵 여러 경우의 수에서도 잘 작동되는지 테스트하도록 추가해봤습니다!

Hyeon9mak · 2024-03-15T08:13:53Z

src/main/java/domain/constant/GamerIdentifier.java

+package domain.constant;
+
+public final class GamerIdentifier {
+    public static final String DEALER_IDENTIFIER = "dealer";
+}


딜러 표현 방식을 "dealer" 문자열이 아닌 웹 이미지로 변경하는 것도 비즈니스 규칙으로 보아야 할까요?
동료 개발자가 딜러 표현 방식을 변경하고자 할 때 domain 과 view 패키지 중 어떤 패키지를 먼저 열어보게 될지 상상해보면 어떨까요?

딜러와 플레이어의 dto를 GamerDto 하나로 관리하고 싶었는데, 여기서 문제는 플레이어만 이름을 갖고 있다는 것이었습니다. 그래서 딜러와 플레이어를 구분하기 위해 일종의 식별자로 사용하고자 넣었고 출력 도메인에서 해당 식별자를 통해 원하는 출력 양식으로 출력할 수 있도록 구현했습니다. GamerDto를 재사용하고 싶어서 이렇게 구현하게 됐는데, 차라리 딜러의 dto를 분리하는 편이 더 나을까요?

인과관계가 역전된 느낌인데요, dto 는 계층간 소통을 위해서 데이터를 나르는 짐꾼 객체,
언제든지 추가되고 언제든지 변경되고 언제든지 삭제될 수 있는 데이터 덩어리에 불과한데,
그런 객체를 위해서 전체적인 구조에 변화가 생겨나는 걸로 보여요.

지난 코멘트 설명했듯 코드 배치가 적당한가 아닌가를 판별할 수 있는 방법은 "나중에 코드를 열어볼 때 이 로직과 관련된 코드는 여기에 있겠지?" 라고 생각해요. 현재 GamerIdentifier 는 GamerDto 를 위해서만 존재하는 것 같은데, 동료 개발자가 딜러를 이미지로 표현하고자 할 때 GamerIdentifier 를 곧장 열어볼 수 있을지 상상해보면 도움이 될 것 같아요!

확실히 말씀을 듣고보니 주객이 전도된 느낌이 크네요. GamerDto를 딜러와 플레이어로 나눴고, 이름이 필요한지만 차이가 나므로 PlayerDto가 DealerDto를 상속하도록 했습니다.
그런데 리팩토링하면서 다시 살펴보니 BettingTable이 총 이익을 계산하는 데에도 DEALER_IDENTIFIER를 사용하고 있었습니다. 이 클래스에서는 처음 들어온 베팅액을 저장했다가 이후 결과를 받아서 총 결과를 반환하는데 딜러와 플레이어의 결과를 한 번에 반환하고 싶어서 사용하게 된 것 같습니다. 이 또한 딜러와 플레이어의 결과를 받는 메서드를 분리해서 처리했습니다!

Hyeon9mak · 2024-03-15T08:14:34Z

src/main/java/domain/constant/GamerResult.java

 public enum GamerResult {
-    WIN,
-    DRAW,
-    LOSE;
+    WIN(1),
+    DRAW(0),
+    LOSE(-1);

    private static final int BUST_THRESHOLD = 21;


처음 두 장의 카드 합이 21일 경우 블랙잭이 되면 베팅 금액의 1.5 배를 딜러에게 받는다. 딜러와 플레이어가 모두 동시에 블랙잭인 경우 플레이어는 베팅한 금액을 돌려받는다.

점수 대신 Hand의 정보를 받아 블랙잭인지 확인하고 블랙잭인 경우의 수도 고려해서 결과를 낼 수 있도록 수정했습니다.

Hyeon9mak

안녕하세요 알파카, 요구사항 잘 구현해주셨습니다 👍

domain 패키지에도, dto 패키지에도 각각 dto 들이 존재하는 것 같은데요,
어떤 기준으로 패키지를 분리해두신 것인지 설명해주시면 코드를 이해하는데 더 큰 도움이 될 것 같아요!

질문 주신 내용과 함께 간단한 코멘트 추가로 남겼으니 확인해주세요!
궁금하신 점 질문 코멘트 남겨주세요~ 👍

Hyeon9mak · 2024-03-18T01:08:48Z

README.md

+- [ ] 최종 수익을 출력한다
+    - [x] 이름과 수익 액수를 출력한다
+    - [x] 승리하면 베팅 금액만큼 수익을 거둔다
+    - [x] 패배하면 베팅 금액만큼 손해를 거둔다
+    - [ ] 블랙잭일 경우 베팅 금액의 1.5배의 수익을 거둔다
+    - [x] 무승부이면 최종 수익은 0이다
+        - [ ] 딜러와 플레이어가 둘 다 블랙잭이면 무승부이다


Hyeon9mak · 2024-03-18T01:10:40Z

src/test/java/domain/BettingTableTest.java

+        @Test
+        @DisplayName("플레이어가 여러 명인 경우")
+        void variousPlayersWinCase() {
+            BettingTable bettingTable = new BettingTable(Map.of(
+                    "test1", new BettingAmount(1000),
+                    "test2", new BettingAmount(2000),
+                    "test3", new BettingAmount(3000)));
+            Assertions.assertThat(bettingTable.getTotalProfit(Map.of(
+                            "test1", GamerResult.WIN,
+                            "test2", GamerResult.WIN,
+                            "test3", GamerResult.WIN)))
+                    .isEqualTo(Map.of(
+                            GamerIdentifier.DEALER_IDENTIFIER, -6000D,
+                            "test1", 1000D,
+                            "test2", 2000D,
+                            "test3", 3000D
+                    ));
+        }
+    }


Hyeon9mak · 2024-03-18T01:16:27Z

src/main/java/domain/constant/GamerIdentifier.java

+package domain.constant;
+
+public final class GamerIdentifier {
+    public static final String DEALER_IDENTIFIER = "dealer";
+}


인과관계가 역전된 느낌인데요, dto 는 계층간 소통을 위해서 데이터를 나르는 짐꾼 객체,
언제든지 추가되고 언제든지 변경되고 언제든지 삭제될 수 있는 데이터 덩어리에 불과한데,
그런 객체를 위해서 전체적인 구조에 변화가 생겨나는 걸로 보여요.

지난 코멘트 설명했듯 코드 배치가 적당한가 아닌가를 판별할 수 있는 방법은 "나중에 코드를 열어볼 때 이 로직과 관련된 코드는 여기에 있겠지?" 라고 생각해요. 현재 GamerIdentifier 는 GamerDto 를 위해서만 존재하는 것 같은데, 동료 개발자가 딜러를 이미지로 표현하고자 할 때 GamerIdentifier 를 곧장 열어볼 수 있을지 상상해보면 도움이 될 것 같아요!

Hyeon9mak · 2024-03-18T01:19:00Z

src/main/java/domain/BettingTable.java

+    public Map<String, Double> getTotalProfit(Map<String, GamerResult> totalResult) {
+        validateSize(totalResult);
+        Map<String, Double> totalProfit = new HashMap<>();
+        totalResult.forEach((key, value) ->
+                totalProfit.put(
+                        key,
+                        bettingAmounts.get(key).getAmount() * value.getProfitRatio()
+                ));
+        totalProfit.put(GamerIdentifier.DEALER_IDENTIFIER, getDealerProfit(totalProfit));
+        return totalProfit;


GamerResult 로부터 배율을 뽑아와서 직접 연산해줄 필요 없이, GamerResult 에게 메세지를 보내어 연산결과를 얻어낼 수도 있겠네요 😄

객체에 메세지를 보내라.

그리고 이 방식은 추후에도 getProfitRatio 를 통해 배율값이 여기저기 의존되어 배율을 변경하기 어려운(유지보수가 어려운) 상황을 방지하기도 좋을 것 같아요.
"블랙잭 우승시 배율을 4배로 늘리자!" 라는 요구사항이 전달될 때도 GameResult 의 배율값만 변경하면 유지보수를 끝낼 수 있겠어요!

Hyeon9mak · 2024-03-18T01:23:59Z

src/main/java/domain/HandStatus.java

+public class HandStatus {
+    private final int score;
+    private final int handCount;
+
+    public HandStatus(int score, int handCount) {
+        this.score = score;
+        this.handCount = handCount;
+    }
+
+    public int getScore() {
+        return score;
+    }
+
+    public int getHandCount() {
+        return handCount;
+    }
+}


record 로 표현할 수도 있겠네요!

record는 생각 못해봤는데 훨씬 낫겠네요!

slimsha2dy · 2024-03-18T04:05:14Z

dto라고 하면 도메인의 정보를 view로 넘겨준다는 역할에만 집중해서 생각하지 못했는데 HandStatus도 dto였네요. 일관성을 위해 해당 클래스를 dto 패키지로 이동했습니다.

Hyeon9mak

안녕하세요 알파카! 블랙잭 미션 마지막까지 고생 많으셨어요.

dto 에 관련된 코멘트를 여러차례 남겨드렸던 만큼,
마지막 체스 미션을 진행하시는 동안
도메인 객체와 vo, dto, 엔티티간의 차이를 명확하게 이해하고 다뤄보는 경험을 해보시면 좋을거 같아요.
레벨2에 영속화 객체(엔티티)를 만나기 시작하면서 큰 인지부조화가 찾아올텐데,
이를 헤쳐나가기 위해선 명확한 구분과 이해가 필요할거에요. 💪

벌써 레벨1 마지막 미션을 앞두고 있네요.
체스 미션도 재밌으니 마지막까지 화이팅 하시길 바랍니다!
고생 많으셨어요!! 👍

Hyeon9mak · 2024-03-18T15:28:31Z

README.md

+- [x] 최종 수익을 출력한다
+    - [x] 이름과 수익 액수를 출력한다
+    - [x] 승리하면 베팅 금액만큼 수익을 거둔다
+    - [x] 패배하면 베팅 금액만큼 손해를 거둔다
+    - [x] 블랙잭일 경우 베팅 금액의 1.5배의 수익을 거둔다
+    - [x] 무승부이면 최종 수익은 0이다
+        - [x] 딜러와 플레이어가 둘 다 블랙잭이면 무승부이다



slimsha2dy added 8 commits March 13, 2024 22:22

docs: 2단계 기능 요구사항 문서 반영

00070f1

feat: Gamer들의 베팅을 관리하는 BettingTable 기능 구현

6af3d3d

feat: 베팅 금액 클래스 BettingAmount 구현

8a9d380

feat: BettingTable이 매개변수 Map의 value로 BettingAmount를 받도록 수정

a0edb29

feat: 인자로 받은 플레이어의 베팅 금액을 입력받는 기능 구현

09e22d2

feat: 최종 수익을 출력하는 기능 구현

773033b

rename: BettingAmount를 생성할 때 정수로 변환하는 메서드의 이름 수정

f78f2a9

refactor: 딜러를 구분하는 문자열 상수를 상수 클래스에서 처리하도록 수정

1d8276a

Hyeon9mak requested changes Mar 15, 2024

View reviewed changes

slimsha2dy added 7 commits March 15, 2024 17:40

docs: 베팅 금액 조건에 블랙잭인 경우 업데이트

9995ebd

refactor: BettingAmount가 문자열이 아닌 정수로 생성되도록 수정

e32cefc

feat: 핸드의 정보를 포장한 HandStatus 클래스 구현

e6b157d

feat: 베팅액 결과를 double 형으로 반환하도록 수정

9a0d379

feat: BlackJackResult에서 블랙잭 결과를 반환하는 기능 구현

617ac6e

test: 베팅 결과 경우의 수 테스트 추가

57dbf0d

feat: 최종 수익 출력 포맷 변경

68589a7

slimsha2dy force-pushed the step2 branch from edafaa6 to 68589a7 Compare March 17, 2024 11:49

Hyeon9mak requested changes Mar 18, 2024

View reviewed changes

slimsha2dy added 2 commits March 18, 2024 12:45

docs: 문서 완료된 요구사항 업데이트

7b85fc9

refactor: HandStatus 클래스를 record로 수정

1202a03

slimsha2dy added 6 commits March 18, 2024 13:05

refactor: HandStatus 클래스를 dto 패키지로 이동

d63ac97

fix: HandStatus의 getter 사용 오류 수정

120aff6

refactor: GamerResult가 이익 배율이 아닌 이익액을 반환하도록 출력

bc3ed1a

refactor: GamerDto를 DealerDto와 PlayerDto로 분리

a226da5

refactor: Player의 이익과 Dealer의 이익을 반환하는 메서드 분리

262c992

refactor: DealerDto를 통해 출력하도록 수정

cfb4c33

Hyeon9mak approved these changes Mar 19, 2024

View reviewed changes

Hyeon9mak merged commit f56b239 into woowacourse:slimsha2dy Mar 19, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2단계 - 블랙잭 베팅] 알파카(최휘용) 미션 제출합니다. #714

[2단계 - 블랙잭 베팅] 알파카(최휘용) 미션 제출합니다. #714

slimsha2dy commented Mar 14, 2024

Hyeon9mak left a comment

Hyeon9mak Mar 15, 2024

slimsha2dy Mar 17, 2024 •

edited

Loading

Hyeon9mak Mar 15, 2024

slimsha2dy Mar 15, 2024

Hyeon9mak Mar 15, 2024

slimsha2dy Mar 17, 2024 •

edited

Loading

Hyeon9mak Mar 15, 2024

slimsha2dy Mar 17, 2024 •

edited

Loading

Hyeon9mak Mar 18, 2024

slimsha2dy Mar 18, 2024

Hyeon9mak Mar 15, 2024

slimsha2dy Mar 17, 2024

Hyeon9mak left a comment

Hyeon9mak Mar 18, 2024

Hyeon9mak Mar 18, 2024

Hyeon9mak Mar 18, 2024

Hyeon9mak Mar 18, 2024

Hyeon9mak Mar 18, 2024

slimsha2dy Mar 18, 2024

slimsha2dy commented Mar 18, 2024

Hyeon9mak left a comment

Hyeon9mak Mar 18, 2024

[2단계 - 블랙잭 베팅] 알파카(최휘용) 미션 제출합니다. #714

[2단계 - 블랙잭 베팅] 알파카(최휘용) 미션 제출합니다. #714

Conversation

slimsha2dy commented Mar 14, 2024

Hyeon9mak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slimsha2dy 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

Choose a reason for hiding this comment

slimsha2dy Mar 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slimsha2dy 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hyeon9mak 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

slimsha2dy commented Mar 18, 2024

Hyeon9mak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slimsha2dy Mar 17, 2024 •

edited

Loading

slimsha2dy Mar 17, 2024 •

edited

Loading

slimsha2dy Mar 17, 2024 •

edited

Loading