늦어서 죄송합니다 박스터 입니다 #15

drunkenhw · 2023-03-12T04:33:08Z

No description provided.

be-student

박스터 좋은데요?
이상한 점 찾아내느라 힘들었어욧

be-student · 2023-03-12T07:29:21Z

src/main/java/blackjack/constants/ErrorCode.java

+public enum ErrorCode {
+    DUPLICATE_NAME,
+    RESERVED_NAME,
+    NOT_EXIST_MESSAGE,
+    BLANK_NAME,
+    EMPTY_CARD,
+    INVALID_COMMAND,
+    INVALID_MONEY_UNIT,
+    INVALID_MONEY_BOUND,
+    ;


신기한데요?
패키지는 어디에 위치할지 고민해보시면 좋을 것 같아요

패키지 나누기 힘들어요 팁 주세요

어떤 문제를 해결하고 있는지에 대해서 적으래요

그게 무슨 말인데요

에러코드가 메시지를 가지는것에 대해서는 어떻게 생각하시나요?

@Choi-JJunho 일관성 있게 view 에서 메세지를 다 관리하고 싶었습니다.

be-student · 2023-03-12T07:58:59Z

src/main/java/blackjack/controller/BlackjackController.java

+    private BlackjackGame createBlackjackGame() {
+        Participants participants = createParticipants();
+        return new BlackjackGame(participants);
+    }


BlackjackGame(List<String> playerNames) 같은 형태는 어떤가요?
바깥에서 생성할 이유가 없어보여요

맞는 말 입니다

be-student · 2023-03-12T07:59:30Z

src/main/java/blackjack/controller/BlackjackController.java

+    private Participants createParticipants() {
+        try {
+            List<String> names = inputView.readNames();
+            return Participants.from(names);
+        } catch (CustomException e) {
+            outputView.printError(e.getErrorCode());
+            return createParticipants();
+        }
+    }


재귀보다는 while 어떤가요?

그거 쓸 줄 몰라요.. 알려주세요

be-student · 2023-03-12T08:05:00Z

src/main/java/blackjack/view/message/CardMessageConvertor.java

+    public static String convert(Card card) {
+        return CardNumberMessage.from(card.getNumber()).getMessage() +
+                CardSuitMessage.from(card.getSuit()).getMessage();
+    }
+}


이 클래스는 CardNumberMessage 에 들어가면 어떨까요?

be-student · 2023-03-12T08:06:57Z

src/main/java/blackjack/view/message/CardNumberMessage.java

+    ACE(CardNumber.ACE, "A"),
+    TWO(CardNumber.TWO, "2"),
+    THREE(CardNumber.THREE, "3"),
+    FOUR(CardNumber.FOUR, "4"),
+    FIVE(CardNumber.FIVE, "5"),
+    SIX(CardNumber.SIX, "6"),
+    SEVEN(CardNumber.SEVEN, "7"),
+    EIGHT(CardNumber.EIGHT, "8"),
+    NINE(CardNumber.NINE, "9"),
+    TEN(CardNumber.TEN, "10"),
+    JACK(CardNumber.JACK, "J"),
+    QUEEN(CardNumber.QUEEN, "Q"),
+    KING(CardNumber.KING, "K");


view 가 domain 을 필드로 가지고 있다면, 도메인에 너무 세부사항에 의존하게 되는 것 같아요
domain 을 필드에서 빼보는건 어떤가요?

view가 도메인을 의존하면 안되는 이유가 있을까요? view 주제에

be-student · 2023-03-12T08:19:41Z

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

+    public Card(CardSuit cardSuit, CardNumber cardNumber) {
+        this.cardSuit = cardSuit;
+        this.cardNumber = cardNumber;
+    }


new 로 생성할 수 있는데, 캐싱을 하는 의미가 없어보여요

죄송합니다

ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ
왜 의미가 없죠?

생성자 막아놓고 정적 팩터리로 이미 만들어놓은거 반환해주면 될라나요?

그죠 그러면 되죠

캐싱 좋은데요?

be-student · 2023-03-12T08:21:10Z

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

+    public List<Card> toList() {
+        return List.copyOf(cards);
+    }


모든 게 다 방어적 복사네요
꼼꼼한사람...

be-student · 2023-03-12T08:22:31Z

src/main/java/blackjack/domain/user/Participant.java

+import java.util.List;
+
+public abstract class Participant {
+    protected static final String DEALER_NAME = "딜러";


protected 변수는 안좋은 것 같아요
웨인지는 궁금하면 월요일날....
말로하기 길어요

글로 적기 긴거 아닌가요? 근데 리뷰어가 이렇게 하라던데 궁금합니다 눈우 박사

말랑이 이거 괜찮데요

be-student · 2023-03-12T08:22:53Z

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

+    public String getValue() {
+        return value;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        Name name = (Name) o;
+        return Objects.equals(value, name.value);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(value);
+    }


getter 는 equalsHashcode 밑에!

나는 위에 놓고 싶던데

be-student · 2023-03-12T08:24:03Z

src/main/java/blackjack/domain/game/BettingResult.java

+    public Map<Player, Money> getBettingResults() {
+        return Map.copyOf(bettingResults);
+    }


이거 순서 보장 안되는거 아시죠?
혹시나해서 말씀드려요><@

ㅋㅋ 어쩐지 출력이 지멋대로 되더라고요 ㅋㅋ

LinkedHashMap이던가 그런거 순서 되는것도 있으요

gitchannn

나머지는 고기먹고 와서 남기겟슴다

from. 깃짱코딩

gitchannn · 2023-03-12T08:51:48Z

src/main/java/blackjack/view/message/ErrorCodeMessage.java

+    DUPLICATE_NAME(ErrorCode.DUPLICATE_NAME, "중복된 이름은 사용할 수 없습니다."),
+    RESERVED_NAME(ErrorCode.RESERVED_NAME, "이름은 딜러일 수 없습니다"),
+    NOT_EXIST_MESSAGE(ErrorCode.NOT_EXIST_MESSAGE, "해당 메세지가 없습니다."),
+    BLANK_NAME(ErrorCode.BLANK_NAME, "이름은 공백일 수 없습니다."),
+    EMPTY_CARD(ErrorCode.EMPTY_CARD, "뽑을 수 있는 카드가 없습니다."),
+    INVALID_COMMAND(ErrorCode.INVALID_COMMAND, "Y 또는 N을 입력해주세요."),
+    INVALID_MONEY_UNIT(ErrorCode.INVALID_MONEY_UNIT, "금액은 1000원 단위로 입력 가능합니다."),
+    INVALID_MONEY_BOUND(ErrorCode.INVALID_MONEY_BOUND, "금액은 10,000,000원 이하만 입력 가능합니다."),
+    ;


ㅇㅈ요 저도 ErrorCode에 필드로 관리하거나 하는게 더 좋을 것 같다고 생각함다

gitchannn · 2023-03-12T08:52:53Z

src/main/java/blackjack/view/message/GameResultMessage.java

@@ -0,0 +1,42 @@
+package blackjack.view.message;


메세지 패키지는 무엇을 하는 아이인가여??
승, 패, 무는 view의 역할인 것 같기도 한데
뷰에서 하기 너무 복잡해지는 것들을 모아놓는 보조 뷰...?

gitchannn · 2023-03-12T08:53:38Z

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

+    public Card(CardSuit cardSuit, CardNumber cardNumber) {
+        this.cardSuit = cardSuit;
+        this.cardNumber = cardNumber;
+    }


ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ
왜 의미가 없죠?

gitchannn · 2023-03-12T08:54:11Z

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

+    public Card(CardSuit cardSuit, CardNumber cardNumber) {
+        this.cardSuit = cardSuit;
+        this.cardNumber = cardNumber;
+    }


생성자 막아놓고 정적 팩터리로 이미 만들어놓은거 반환해주면 될라나요?

Choi-JJunho

박스터 안녕하세요~
전체적으로 좋은 리뷰가 달려있어 궁금한 내용 위주로 코멘트 남겼습니다~

Choi-JJunho · 2023-03-12T16:31:16Z

src/main/java/blackjack/constants/ErrorCode.java

+public enum ErrorCode {
+    DUPLICATE_NAME,
+    RESERVED_NAME,
+    NOT_EXIST_MESSAGE,
+    BLANK_NAME,
+    EMPTY_CARD,
+    INVALID_COMMAND,
+    INVALID_MONEY_UNIT,
+    INVALID_MONEY_BOUND,
+    ;


에러코드가 메시지를 가지는것에 대해서는 어떻게 생각하시나요?

Choi-JJunho · 2023-03-12T16:31:43Z

src/main/java/blackjack/controller/BlackjackController.java

+        BlackjackGame blackjackGame = createBlackjackGame();
+        start(blackjackGame);
+        printResult(blackjackGame);


너무 간결해서 좋은데요?

Choi-JJunho · 2023-03-12T16:32:21Z

src/main/java/blackjack/controller/GameCommand.java

+        if (PLAY.command.equalsIgnoreCase(command)) {
+            return PLAY;
+        }
+        if (STOP.command.equalsIgnoreCase(command)) {


equalsIgnoreCase 좋은데요?

Choi-JJunho · 2023-03-12T16:33:34Z

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

+    public Card(CardSuit cardSuit, CardNumber cardNumber) {
+        this.cardSuit = cardSuit;
+        this.cardNumber = cardNumber;
+    }


캐싱 좋은데요?

Choi-JJunho · 2023-03-12T16:34:34Z

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

+    }
+
+    public boolean isAce() {
+        return this.cardNumber.isAce();


this를 붙이는 기준이 있을까요?
아래에는 없네요?

페어탓 해명하세요

Choi-JJunho · 2023-03-12T16:41:32Z

src/main/java/blackjack/domain/game/BettingTable.java

+        Map<Player, Money> bettingResults = bets.keySet().stream()
+                .collect(Collectors.toMap(
+                        player -> player,
+                        player -> bets.get(player).multiply(blackjackResult.get(player).getRatio()),


.이 너무 많네요..
디미터의 법칙 위배아닐까요?
변수로 추출해주면서 어떤 값인지 표현하면 읽기 편할것 같아요

Choi-JJunho · 2023-03-12T16:41:55Z

src/main/java/blackjack/domain/game/BettingTable.java

+                .collect(Collectors.toMap(
+                        player -> player,
+                        player -> bets.get(player).multiply(blackjackResult.get(player).getRatio()),
+                        (o1, o2) -> o1,


o1, o2? 무슨의미죠?
근데 저도 이렇게 썼던거 같네요
죄송합니다

Choi-JJunho · 2023-03-12T16:49:46Z

src/main/java/blackjack/domain/game/BlackjackResult.java

+        if (score.equals(dealerScore)) {
+            return GameResult.TIE;
+        }
+        if (score.isGreaterOrEqualsTo(dealerScore)) {


좋은데요?

Choi-JJunho · 2023-03-12T16:50:02Z

src/main/java/blackjack/domain/game/BlackjackResult.java

+    }
+
+    public int getTieCount() {
+        return Collections.frequency(blackjackResult.values(), GameResult.TIE);


Collections.frequency 좋은데요?

Choi-JJunho · 2023-03-12T16:53:34Z

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

+    }
+
+    public List<Card> getFirstCard() {
+        return List.of(getAllCards().get(0));


한장만 가져오는데 List일 이유가 있나요?

drunkenhw and others added 23 commits March 10, 2023 11:24

[1단계 - 블랙잭 게임 실행] 박스터 (한우석) 미션 제출합니다. (woowacourse#407)

1258521

refactor: 패키지 이동

3e207f0

refactor: 메소드명 변경

7c771a0

refactor: score 계산하는 로직 수정

3e398eb

feat: 무승부를 만드는 기능 추가

47c9b2e

refactor: 챰가자들의 bust 유무 메소드 추가

15a6617

refactor: 무승부를 구하는 로직 수정

6247163

refactor: 패키지명 변경

0c8b90e

feat: 첫번째 카드 오픈 기능 추가

5c4dcab

feat: Score 값 객체 추가

52ef885

docs: 그래프 추가

a1491d3

refactor: 메소드 위치 수정

5d78e14

refactor: 결과를 반환하는 로직 위치 이동

ee3b48b

refactor: 변수명 변경

d923b0b

docs: 기능 목록 추가

d4d08a8

feat: BettingMoney 생성

380de2e

feat: BettingMoney 검증 추가

ebf4816

feat: BettingTable 생성

4d62340

feat: 베팅 금액을 입력 받는 기능 추가

ae75ab7

feat: 블랙잭 여부를 판단하는 기능 추가

039cfc5

feat: 수익률을 계산하는 기능 추가

6243221

feat: 딜러의 수익금을 계산하는 기능 추가

c69b6b3

feat: 결과를 출력하는 기능 추가

e1e25c3

drunkenhw changed the title ~~늦어서 죄송합니다~~ 늦어서 죄송합니다 박스터 입니다 Mar 12, 2023

be-student reviewed Mar 12, 2023

View reviewed changes

gitchannn reviewed Mar 12, 2023

View reviewed changes

Choi-JJunho reviewed Mar 12, 2023

View reviewed changes

drunkenhw added 3 commits March 13, 2023 02:17

refactor: 오버라이딩 어노테이션 추가

7b053be

refactor: Money 상속 제거

e3f91e3

refactor: 사용하지 않는 메소드 제거

5629a10

drunkenhw added 6 commits March 13, 2023 20:35

refactor: bettingTable을 불변으로 변경

2224ee6

refactor: score의 검증 추가

a277ee8

refactor: bettingTable을 불변으로 변경

54c89db

refactor: bettingTable for loop로 변경

c6e78a8

refactor: 컬렉션 복사 방법 변경

c02da94

feat: 예외 처리 추가

ec5e66a

늦어서 죄송합니다 박스터 입니다 #15

Are you sure you want to change the base?

늦어서 죄송합니다 박스터 입니다 #15

Conversation

drunkenhw commented Mar 12, 2023

be-student 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

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

gitchannn left a comment • 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

Choi-JJunho 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

gitchannn left a comment •

edited

Loading