[2단계 - 블랙잭 베팅] 몰리(김지민) 미션 제출합니다. #728

jminkkk · 2024-03-15T08:42:33Z

안녕하세요 파랑🌊
2단계를 진행하면서, 도메인에 대한 이해가 잘되지 않아 어려움이 있었던 것 같아요😭
중간에 리드미의 요구사항 분석을 ~~많~이~~ 잘못했었었다는 것을 느끼고 갈아엎고 열심히 다시 설계를 했습니다..ㅎㅎ

플레이어와 딜러의 관계

파랑의 피드백을 받고 플레이어와 딜러의 관계에 대해 다시 생각해보았어요. 제가 생각해본 방법과 의견은 아래와 같았어요.

~~딜러가 플레이어를 상속~~
- 게임 안에서 딜러와 플레이어가 각각 다른 역할로 사용되고 있기 때문에 적절하지 않다.
딜러에 플레이어를 합성
- 마찬가지로 역할이 다르기 때문에 딜러가 플레이어를 가지고 있다는 게 어색한 것 같다.
딜러와 플레이어가 Playable 인터페이스를 구현
- 제가 생각하기에 인터페이스는 타입보다는 어떤 특징을 나타내게 할 때 좋을 것 같은데, 다른 역할이기 때문에 타입으로 정의를 하는 것이 조금 더 자연스러울 것 같다.
딜러와 플레이어가 Gamer 추상 클래스를 상속

딜러와 플레이어가 Gamer의 한 종류라고 볼 수 있기 때문에 가장 적절한 것 같다.

결론으로, Gamer 추상 클래스를 통해 게임에 참여하는 모든 게이머가 가져야할 공통적인 역할을 묶고, 딜러와 플레이어가 게이머를 상속받게 하여 다른 역할에 대한 부분은 오버라이딩을 하는 식으로 진행하였습니다.

2단계도 잘 부탁드립니다 파랑!

…리해 베팅 금액을 받는 기능 구현

- README.md 수정

… 경우임에 대한 테스트 작성

summerlunaa

몰리 안녕하세요~

2단계도 잘 구현해주셨네요! 리뷰 남겨드렸으니 확인부탁드립니다~

summerlunaa · 2024-03-15T11:28:28Z

src/main/java/blackjack/model/participant/HandStatus.java

+    BUST((score -> score > BLACKJACK_SCORE), (cardSize -> cardSize >= BLACKJACK_CARD_SIZE)),
+    BLACKJACK((score -> score == BLACKJACK_SCORE), (cardSize -> cardSize == BLACKJACK_CARD_SIZE)),
+    NOT_BLACKJACK_BUT_21((score -> score == BLACKJACK_SCORE), (cardSize -> cardSize > BLACKJACK_CARD_SIZE)),
+    UNDER_21((score -> score < BLACKJACK_SCORE), (cardSize -> cardSize >= BLACKJACK_CARD_SIZE));


HandStatus를 Enum을 통해 관리하도록 했네요. 좋은 방법인 것 같아요 👍

summerlunaa · 2024-03-15T11:31:10Z

src/main/java/blackjack/model/participant/Hand.java

+    public boolean isUnder21() {
+        return getHandStatus().equals(HandStatus.UNDER_21);
+    }


사실상 여기서 블랙잭 기준이 21에서 다른 숫자로 변할 가능성은 극히 낮지만.. 상수를 메서드명이나 변수명에 넣으면 어떤 단점이 있을까요?

블랙잭 게임에 대해 이해도가 낮은 경우 21이라는 숫자에 대해 이해하기가 어려울 것 같아요 🥲
값이 변경된다면, 관련 메서드나 변수를 다 수정해야 해서 유지보수에도 좋지 않을 것 같네요!
변경하겠습니다 ㅎㅎ

summerlunaa · 2024-03-15T11:35:00Z

src/main/java/blackjack/model/participant/Gamer.java

+import blackjack.model.deck.Card;
+import java.util.List;
+
+public abstract class Gamer {


결론으로, Gamer 추상 클래스를 통해 게임에 참여하는 모든 게이머가 가져야할 공통적인 역할을 묶고, 딜러와 플레이어가 게이머를 상속받게 하여 다른 역할에 대한 부분은 오버라이딩을 하는 식으로 진행하였습니다.

좋습니다 👍

summerlunaa · 2024-03-15T11:35:38Z

src/main/java/blackjack/model/participant/Player.java

-    @Override
-    public void receiveCard(final Card card) {
-        this.hand = hand.addCard(card);
-    }


중복 코드가 많이 줄었네요 💯

summerlunaa · 2024-03-15T11:42:03Z

src/main/java/blackjack/model/betting/ProfitRate.java

+    PLUS_150_PERCENT(1.5),
+    PLUS_100_PERCENT(1),
+    ZERO(0),
+    MINUS_100_PERCENT(-1)


여기도 숫자 값이 네이밍에 들어가있네요~

summerlunaa · 2024-03-15T11:52:07Z

src/main/java/blackjack/model/betting/MoneyStaff.java

+        for (Player player : playerGameResult.keySet()) {
+            Money profitMoney = calculateProfitMoney(player, playerGameResult.get(player));
+            playerProfitMoneys.put(player, profitMoney);
+        }


Suggested change

for (Player player : playerGameResult.keySet()) {

Money profitMoney = calculateProfitMoney(player, playerGameResult.get(player));

playerProfitMoneys.put(player, profitMoney);

}

for (Entry<Player, ResultCommand> entry : playerGameResult.entrySet()) {

Money profitMoney = calculateProfitMoney(entry.getKey(), entry.getValue());

playerProfitMoneys.put(entry.getKey(), profitMoney);

}

entry를 사용하면 map에서 get하는 로직을 없앨 수 있겠네욥.

좋은 코드 예시 너무 감사합니다!!! 🫡 👍

summerlunaa · 2024-03-15T12:38:09Z

src/main/java/blackjack/view/formatter/CardsFormatter.java

+public class CardsFormatter {
+    public static String format(final List<Card> cards) {


몰리는 클래스를 어떤 기준으로 분리하고 있나요~? 각 객체별 Formatter는 어떤 이유로 만들게 되었는지 궁금합니다!

format하는 부분 때문에 OutputView가 너무 지저분해지는 것 같아 분리를 하게 되었습니다..!
Dto나 Vo별 분리를 하게 되면 delimiter나 포맷 형식이 변경되는 경우, 해당 부분에 대한 Formatter만 수정하면 될 것 같아 편할수도 있을 것 같다고 생각하기도 했구요..!

분리하신 이유는 알겠습니다~ 문자열을 포맷팅하는 부분 때문에 OutputView가 지저분해진다는 점에도 공감하구요.
다만 클래스를 분리하기 전, 클래스를 분리하여 관리하는 것도 비용이 든다는 점을 항상 염두에 두면 좋을 것 같습니다.
블랙잭 게임 정도의 요구사항에도 Formatter가 5개나 생성되었는데요, 여기서 도메인과 View가 더 복잡해지면 수십개의 Formatter 클래스가 생성될 것 같아요. 이런 상황에서 몰리는 Formatter 클래스들을 어떻게 관리할 것인지 궁금합니다 :-)

여기서 도메인과 View가 더 복잡해지면 수십개의 Formatter 클래스가 생성될 것 같아요. 이런 상황에서 몰리는 Formatter 클래스들을 어떻게 관리할 것인지 궁금합니다 :-)

당장에 드는 생각은... 도메인과 View가 더 복잡해지면 Formatter를 없애고 OutputView 자체를 분리해볼수도 있겠다는 생각이 듭니다..!
블랙잭에서는, 게임 진행에 대한 출력, 결과에 대한 출력 등등 처럼요..!! ~~좋은 아이디어인지는 확신이 들지는 않지만요🥲~~

파랑 말처럼 클래스를 어떻게 관리해야 할지에 대해서도 고려를 해야할 것 같아요...! 상황이나 규모에 맞게 적절하게 분리를 하는 것이, 어렵게 느껴지지만 중요한 것 같습니다...!

summerlunaa · 2024-03-15T12:39:29Z

src/main/java/blackjack/BlackJackGame.java

@@ -21,29 +25,42 @@ public class BlackJackGame {



ConsoleReader가 util에 있네요. util에는 어떤 클래스가 들어가나요?

전역적으로 사용되는 파일이 들어가야 할 거 같아요 🥲
reader는 view 에서만 사용되기 때문에 view 패키지 안에 존재하는 것이 맞을 것 같네요!
감사합니다 파랑! 🌊

summerlunaa · 2024-03-15T12:45:57Z

src/main/java/blackjack/model/participant/Players.java

        validatePlayerSize(players);
        this.players = players;
    }

-    public static Participants of(final List<String> rawNames, final List<Hand> cards) {
+    public static Players of(final List<String> rawNames, final List<Hand> cards) {


Deck을 받으면 rawNames와 cards의 size가 같아야 한다는 관계에 신경쓰지 않고도 안전한 코드를 만들 수 있을 것 같네요!

편한 테스트를 위해 안전함을 ~~조금 걸리지만~~ 양보했었는데, 안전한 코드가 확실히 더 중요한 것 같아요.😂
좋은 피드백 감사합니다🔥

summerlunaa · 2024-03-15T12:48:05Z

src/main/java/blackjack/BlackJackGame.java

+        final Map<Player, Money> playerAndBettingMoney = moneyStaff.calculateProfitMoneys(playerResults);
+        final Money dealerProfit = moneyStaff.calculateDealerProfitAmount(
+                playerAndBettingMoney.values().stream().toList());
+        OutputView.printDealerProfit(dealer.getName(), dealerProfit);


dealer도 NameProfit을 사용할 수 있겠네요

넵 그렇게 하면 따로 printDealerProfit를 선언하지 않아도 될 것 같네요 감사합니다!!

jminkkk · 2024-03-17T06:55:48Z

좋은 피드백 감사합니다 파랑!🐬
이번 리뷰도 잘 부탁드립니다 🙏🙏

- import문 및 코드 정렬

summerlunaa

몰리 안녕하세요~

리뷰를 꼼꼼히 잘 반영해주셨네요 👍

지금도 충분히 좋지만 리뷰 조금 더 남겨드렸으니 확인부탁드릴게요!

summerlunaa · 2024-03-17T15:05:37Z

src/main/java/blackjack/model/participant/Players.java

        return IntStream.range(0, rawNames.size())
-                .mapToObj(index -> new Player(new Name(rawNames.get(index)), cards.get(index)))
-                .collect(Collectors.collectingAndThen(Collectors.toList(), Participants::new));
-    }
-
-    private static void validateNamesInitialCardsSize(final List<String> rawNames, final List<Hand> cards) {
-        if (rawNames.size() != cards.size()) {
-            throw new IllegalArgumentException("플레이어 인원과 초기 카드 목록의 사이즈가 다릅니다.");
-        }
+                .mapToObj(index -> new Player(new Name(rawNames.get(index)), deck.distributeInitialCard()))
+                .collect(Collectors.collectingAndThen(Collectors.toList(), Players::new));


List<Hand> cards가 사라졌으니 IntStream.range는 사용하지 않아도 되겠네요. 가능하면 리스트에 index로 접근하는 방식은 피하는 것이 좋습니다.

summerlunaa · 2024-03-17T15:10:05Z

src/main/java/blackjack/model/deck/Deck.java

Card 클래스에 대한 변경사항이 없어 여기 작성합니다.
Card의 종류는 총 몇가지 인가요? 만들어질 수 있는 Card의 수가 한정되어 있다면 반복적으로 불필요한 인스턴스 생성을 막기 위한 방법은 어떤 것이 있을까요?

카드의 종류가 고정되어 있기 때문에 미리 생성해두어도 좋을 것 같아요!
파랑 덕분에 캐싱에 대해 학습해볼 수 있었습니다 ㅎㅎ

추가로, 현재는 Deck에서 카드 목록을 생성하고 Shuffle한 후 Deck을 생성하여 반환하는데,
미리 카드 목록을 생성하는 것은 Cards에 두고 Deck은 Shuffle한 후 Deck 생성만 하게 두는 것이 좋을 것 같다는 생각이 들었습니다...!

좋은 피드백 감사합니다!

summerlunaa · 2024-03-17T15:13:40Z

src/main/java/blackjack/view/formatter/CardsFormatter.java

+public class CardsFormatter {
+    public static String format(final List<Card> cards) {


분리하신 이유는 알겠습니다~ 문자열을 포맷팅하는 부분 때문에 OutputView가 지저분해진다는 점에도 공감하구요.
다만 클래스를 분리하기 전, 클래스를 분리하여 관리하는 것도 비용이 든다는 점을 항상 염두에 두면 좋을 것 같습니다.
블랙잭 게임 정도의 요구사항에도 Formatter가 5개나 생성되었는데요, 여기서 도메인과 View가 더 복잡해지면 수십개의 Formatter 클래스가 생성될 것 같아요. 이런 상황에서 몰리는 Formatter 클래스들을 어떻게 관리할 것인지 궁금합니다 :-)

jminkkk

리뷰를 반영하면서 생각해보지 못했던 부분들에 대해 고민해볼 수 있었던 것 같아요!😁
이번에도 좋은 피드백 감사합니다 파랑🐬

jminkkk · 2024-03-17T16:47:29Z

src/main/java/blackjack/model/deck/Deck.java

카드의 종류가 고정되어 있기 때문에 미리 생성해두어도 좋을 것 같아요!
파랑 덕분에 캐싱에 대해 학습해볼 수 있었습니다 ㅎㅎ

추가로, 현재는 Deck에서 카드 목록을 생성하고 Shuffle한 후 Deck을 생성하여 반환하는데,
미리 카드 목록을 생성하는 것은 Cards에 두고 Deck은 Shuffle한 후 Deck 생성만 하게 두는 것이 좋을 것 같다는 생각이 들었습니다...!

좋은 피드백 감사합니다!

jminkkk · 2024-03-17T16:53:56Z

src/main/java/blackjack/view/formatter/CardsFormatter.java

+public class CardsFormatter {
+    public static String format(final List<Card> cards) {


여기서 도메인과 View가 더 복잡해지면 수십개의 Formatter 클래스가 생성될 것 같아요. 이런 상황에서 몰리는 Formatter 클래스들을 어떻게 관리할 것인지 궁금합니다 :-)

당장에 드는 생각은... 도메인과 View가 더 복잡해지면 Formatter를 없애고 OutputView 자체를 분리해볼수도 있겠다는 생각이 듭니다..!
블랙잭에서는, 게임 진행에 대한 출력, 결과에 대한 출력 등등 처럼요..!! ~~좋은 아이디어인지는 확신이 들지는 않지만요🥲~~

파랑 말처럼 클래스를 어떻게 관리해야 할지에 대해서도 고려를 해야할 것 같아요...! 상황이나 규모에 맞게 적절하게 분리를 하는 것이, 어렵게 느껴지지만 중요한 것 같습니다...!

summerlunaa

몰리 안녕하세요~

리뷰 반영도 빠르게 해주셨네요 💯 깔끔하게 잘 완성해주셨습니다ㅎㅎ

블랙잭 미션은 여기서 머지하겠습니다. 수고하셨습니다 👏👏 👍

jminkkk added 30 commits March 13, 2024 11:14

docs(README.md): 추가된 요구사항에 대한 명세 작성

937c772

refactor(Player): 필드의 접근제한자를 private로 수정

c33f8a9

feat(InputView): 금액 입력 시 숫자가 아닌 입력값인 경우 예외를 처리하는 로직 작성

acb508f

feat(InputView): 플레이어별 배팅 금액을 입력받는 기능 구현

0f479e8

feat(BettingAmount): 금액 입력 시 0 이하의 숫자인 경우 예외를 던지는 기능 구현

ded534e

feat(BettingPool): 플레이어가 만약 블랙잭으로 승리한 경우, 베팅 금액의 1.5배를 딜러에게 받는 기능 구현

390884d

feat(BettingPool): 플레이어가 만약 블랙잭으로 무승부인 경우, 베팅 금액을 돌려받는 기능 구현

119ca9f

feat(BettingPool): 딜러가 21을 초과하면 그 시점까지 남아 있던 플레이어들은 가지고 있는 패에 상관 없이 승…

eabc67f

…리해 베팅 금액을 받는 기능 구현

refactor(BettingBoard): BettingPool을 의미를 나타낼 수 있도록 이름 변경

19e4c6c

feat(BettingBoard): 21을 초과한 경우 플레이어의 배팅 금액은 모두 잃는 기능 구현

0276cf0

style(BettingAmountTest): 테스트명을 더 자세히 수정

a43adea

test(BettingAmount): 배팅 금액의 곱을 구하는 테스트 작성

8f60a47

test(BettingAmount): 배팅 금액의 전체를 잃는 테스트 작성

b71033b

refactor(betting/): 배팅 관련 패키지 생성 및 파일 이동

296da7e

refactor(BettingBoardTest): 공통 부분 @beforeeach로 제거

9656899

docs(README.md): 베팅 금액 조건을 다시 분석

f54664a

fix(BettingBoardTest): 변경된 명세에 맞춰 결과에 따른 플레이어 금액 수정

722fc6a

refactor(Money): BettingAmount를 Money로 변경

d28bd3a

refactor(Money, MoneyTest): money의 책임을 변경

b332dc0

test(PlayerFixture): player에 대한 테스트용 픽스처 이넘 생성

5e593f0

feat(BettingAccount): 계좌에 돈을 받는 기능 구현

5ea704a

feat(BettingAccount): 계좌에 돈을 빼는 기능 구현

251a10f

- README.md 수정

feat(BettingAccount): 특정 플레이어가 해당 배팅 계좌의 주인인지를 반환하는 기능 구현

3511b10

feat(BettingBoard): 배팅 목록에 해당 플레이어의 베팅 정보가 존재하지 않을 경우 예외를 던지는 기능 구현

5101980

hotfix(README.md, betting/): 객체 도출 다시, 배팅 관련 객체, 테스트 제거

5c7a475

feat(HandStatus): 카드 목록의 상태를 나타내는 HandStatus 생성 및 블랙잭인 경우 테스트

388d0d7

test(HandStatus): 카드 목록이 Bust인 경우는 총 스코어가 21을 초과한 경우임에 대한 테스트 작성

3a8e53a

test(HandStatus): 카드 목록이 NotBlackJackBut21인 경우는 총 스코어가 21, 갯수가 2개 이상인…

aa65ec9

… 경우임에 대한 테스트 작성

fix(HandStatus): 21 미만인 경우 조건 수정 및 테스트 작성

a358fe6

refactor(Hand, HandTest): 불필요한 상태 확인 메서드 제거

2d1d6d9

jminkkk added 6 commits March 15, 2024 15:55

refactor(formatter/): 포맷팅 클래스에 대한 테스트 작성

a4828c2

fix(OutputView): 포맷팅 형식과 다른 부분들 수정

8ba766e

test(Dealer): 딜러는 이름을 가지고 카드를 받을 수 있다는 누락된 테스트 추가

156b714

refactor(Players, Name): 이름을 통일성 있게 수정 및 상수 추출

d49d52c

refactor(test/fixture/): Fixture 클래스를 모아둔 패키지 생성

f59d0b0

refactor(Gamer): Playable 인터페이스를 추상 클래스로 변경 및 공통 부분 통일

1fe2c06

jminkkk changed the base branch from main to jminkkk March 15, 2024 08:42

summerlunaa requested changes Mar 15, 2024

View reviewed changes

jminkkk added 9 commits March 16, 2024 00:36

refactor(Hand, ProfitRate): 숫자값을 메서드에서 제거 및 추상적인 네이밍으로 변경

312de11

refactor(BettingRule): enum 비교 시 == 연산자로 비교

aa2e804

refactor(MoneyStaff): Map에서 foreach 순회 시 Entry로 순회하도록 수정

2ed0d82

refactor(Players): Players 생성 시 카드 목록이 아닌 Deck을 받도록 수정

b780a67

refactor(reader/): ConsoleReader를 util/에서 view/reader/로 이동

0cddc80

refactor(BlackJackGame, OutputView): 딜러 수익 출력 시 NameProfit를 사용하여 출력하도록

97eb24c

refactor(OutputView): 불필요한 메서드 제거

d6d92b9

refactor(NamesFormatter): 이름 목록 출력에 대한 Formatter 분리

46d7001

refactor(BettingRule): BettingRule을 싱글톤으로

4b632f6

refactor(ResultCommandDisplay): 요구사항 변경으로 불필요해진 클래스 삭제

08405a9

- import문 및 코드 정렬

summerlunaa requested changes Mar 17, 2024

View reviewed changes

jminkkk added 7 commits March 18, 2024 00:29

refactor(Players): Instream.range말고 일반 Stream으로 변경

26cdc61

fix(BettingRuleTest): BettingRule 싱글톤으로 변경에 따른 생성자 사용 부분 변경

dd1e387

refactor(Card): Card 목록을 static 블록을 통해 캐싱

686cbdb

refactor(Deck): Card 목록을 생성하는 책임을 버리고 받아서 셔플만 하도록 수정

bfd5083

test(Card): 생성된 카드 목록의 크기 확인 테스트 위치 변경

18a426d

test(Player): 카드를 추가로 받을 수 있는지에 대해 Hand별 테스트 추가

0081a9f

refactor(Card, test/): Card 생성 시 생성자가 아닌 from으로 찾기

af2808d

jminkkk commented Mar 17, 2024

View reviewed changes

summerlunaa approved these changes Mar 18, 2024

View reviewed changes

summerlunaa merged commit 3612a20 into woowacourse:jminkkk Mar 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2단계 - 블랙잭 베팅] 몰리(김지민) 미션 제출합니다. #728

[2단계 - 블랙잭 베팅] 몰리(김지민) 미션 제출합니다. #728

jminkkk commented Mar 15, 2024 •

edited

Loading

summerlunaa left a comment

summerlunaa Mar 15, 2024

summerlunaa Mar 15, 2024

jminkkk Mar 17, 2024

summerlunaa Mar 15, 2024

summerlunaa Mar 15, 2024

summerlunaa Mar 15, 2024

summerlunaa Mar 15, 2024

jminkkk Mar 15, 2024

summerlunaa Mar 15, 2024

jminkkk Mar 17, 2024

summerlunaa Mar 17, 2024

jminkkk Mar 17, 2024

summerlunaa Mar 15, 2024

jminkkk Mar 16, 2024

summerlunaa Mar 15, 2024

jminkkk Mar 16, 2024

summerlunaa Mar 15, 2024

jminkkk Mar 16, 2024

jminkkk commented Mar 17, 2024

summerlunaa left a comment

summerlunaa Mar 17, 2024

summerlunaa Mar 17, 2024

jminkkk Mar 17, 2024

summerlunaa Mar 17, 2024

jminkkk left a comment

jminkkk Mar 17, 2024

jminkkk Mar 17, 2024

summerlunaa left a comment

		public class CardsFormatter {
		public static String format(final List<Card> cards) {

[2단계 - 블랙잭 베팅] 몰리(김지민) 미션 제출합니다. #728

[2단계 - 블랙잭 베팅] 몰리(김지민) 미션 제출합니다. #728

Conversation

jminkkk commented Mar 15, 2024 • edited Loading

플레이어와 딜러의 관계

summerlunaa 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

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

jminkkk commented Mar 17, 2024

summerlunaa 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

jminkkk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

summerlunaa left a comment

Choose a reason for hiding this comment

jminkkk commented Mar 15, 2024 •

edited

Loading