[오렌지] 블랙잭 미션 제출합니다. #1

YerinCho · 2020-07-01T20:04:59Z

상태패턴에 정신을 못차려서
수익부분 테스트코드 안 만듬. . .
목요일중에 만들겠습니다.. 토나올거같아...

이름 입력 구현 배팅금액 입력 구현

hongbin-dev

안녕하세요. 오렌지! 앨런입니다.
블랙잭 정말 열심히 짜주셨네요! 감탄입니다 👏👏
특히 상태패턴을 적용하신거에 되게 놀랐는데요.
오렌지코드를 잘 모르지만 변경되면 좋아보이는 점..? 작성해봤습니다.

감사합니다. ✌✌

hongbin-dev · 2020-07-02T06:02:46Z

src/main/java/domain/Game.java

+    public void drawFirst(Players players, Dealer dealer) {
+        List<Card> drawCards = new ArrayList<>();
+        for (int index = 0; index < players.size() * FIRST_DRAW_NUMBER; index++) {
+            drawCards.add(deck.deal());
+        }
+        players.firstDraw(drawCards);
+        dealer.drawFirst(deck.deal(), deck.deal());
+    }


Suggested change

public void drawFirst(Players players, Dealer dealer) {

List<Card> drawCards = new ArrayList<>();

for (int index = 0; index < players.size() * FIRST_DRAW_NUMBER; index++) {

drawCards.add(deck.deal());

}

players.firstDraw(drawCards);

dealer.drawFirst(deck.deal(), deck.deal());

}

public void drawFirst(Players players) {

List<Card> drawCards = new ArrayList<>();

for (int index = 0; index < players.size() * FIRST_DRAW_NUMBER; index++) {

drawCards.add(deck.deal());

}

players.firstDraw(drawCards);

}

Players에 딜러도 포함된다면 깔끔한 코드를 가져갈 수 있을 것 같아요.

hongbin-dev · 2020-07-02T06:03:34Z

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

+    private final Symbol symbol;
+
+    private final Type type;


별건 아니지만,, 둘이 사이가 안좋나요..?

hongbin-dev · 2020-07-02T06:05:30Z

src/main/java/domain/card/Hands.java

+public class Hands {
+    public static final int TEN = 10;
+    public static final int ELEVEN = 11;
+    public static final int BLACKJACK = 21;


21이 블랙잭이냐? 라고 했을 때 아닐수도 있다고 생각되는 네이밍이네요.

hongbin-dev · 2020-07-02T06:05:47Z

src/main/java/domain/card/Hands.java

+    public static final int TEN = 10;
+    public static final int ELEVEN = 11;
+    public static final int BLACKJACK = 21;
+    public static final int DEALER_BUST_NUMBER = 17;


public static final int DEALER_BUST_NUMBER = 17; 딜러의 책임 아닐까요?

17이 딜러가 버스트 하는 기준인가요? 제가 알고있는 바로는 딜러가 추가적인 패를 드로우할지 여부를 결정하는 기준입니다. 의미가 잘못 전달될 여지가 있으니 네이밍을 바꿔보는게 어떨까요?

hongbin-dev · 2020-07-02T06:08:44Z

src/main/java/domain/card/Hands.java

+    public boolean isDealerBust() {
+        return sum() >= DEALER_BUST_NUMBER;
+    }


딜러도 버스트는 21이 아닐까요..?

hongbin-dev · 2020-07-02T06:10:11Z

src/main/java/domain/money/EarningMoney.java

+    public static Map<String, Double> calculateMoney(Dealer dealer, Players players) {
+        Map<String, Double> earningMonies = new LinkedHashMap<>();
+        for (Player player : players.getPlayers()) {
+            earningMonies.put(player.getName(), calculate(dealer, player));
+        }
+        return earningMonies;
+    }


자료구조 선택이 좋네요..!
대신 이렇게 하시려면 Name부분에서 이름 중복체크도 해줘야할 것 같아요.

hongbin-dev · 2020-07-02T06:12:49Z

src/main/java/domain/user/Player.java

+    @Override
+    public boolean canDrawCard() {
+        return !state.hands().isBust();
+    }


state.isNotBust()를 뚫어놓으면 디미터법칙을 지킬 수 있을 것 같아요.

KS-KIM

안녕하세요 오렌지🍊, 어려운 미션인데도 불구하고 잘 구현해 주셨습니다.

1차적으로 프로덕션 코드 리뷰했으니 확인하시고 궁금한 점 있으시면 코멘트 달아주세요.

테스트 코드는 여력이 될 때 리뷰해드리겠습니다!

KS-KIM · 2020-07-02T06:25:25Z

src/main/java/domain/card/Deck.java

+    }
+
+    public Card deal() {
+        return cards.remove(0);


CardFactory를 보니 자료구조가 ArrayList네요.
0번째 인덱스 값을 지운다면 deal 메서드가 O(N)의 시간복잡도를 가지게 될 것 같은데, 아래와 같이 바꿔보는건 어떨까요?

Suggested change

return cards.remove(0);

return cards.remove(cards.size() - 1);

KS-KIM · 2020-07-02T06:29:23Z

src/main/java/domain/card/Hands.java

+
+import static domain.Game.FIRST_DRAW_NUMBER;
+
+public class Hands {


블랙잭에서 한 명의 플레이어가 가지는 패를 Hand라고 부릅니다. 일급 컬렉션이라서 복수형 네이밍을 선택하신 것 같은데, Hand가 이미 복수형의 의미를 내포하고 있으니 아래와 같이 바꾸는게 좋지 않을까요?

Suggested change

public class Hands {

public class Hand {

40 CARDS [countable]
a) the playing cards given to one person in a game

참고: https://www.ldoceonline.com/ko/dictionary/hand

KS-KIM · 2020-07-02T06:31:49Z

src/main/java/domain/card/Hands.java

+    public static final int TEN = 10;
+    public static final int ELEVEN = 11;
+    public static final int BLACKJACK = 21;
+    public static final int DEALER_BUST_NUMBER = 17;


17이 딜러가 버스트 하는 기준인가요? 제가 알고있는 바로는 딜러가 추가적인 패를 드로우할지 여부를 결정하는 기준입니다. 의미가 잘못 전달될 여지가 있으니 네이밍을 바꿔보는게 어떨까요?

KS-KIM · 2020-07-02T06:33:30Z

src/main/java/domain/card/Hands.java

+    }
+
+    private boolean isAceOne(int sum) {
+        return isAceExist() && sum > ELEVEN;


버스트 기준이 25로 바뀐다면 ELEVEN의 값은 바뀌지 않을까요? 보다 유연한 네이밍을 하는게 좋을 것 같아요.

KS-KIM · 2020-07-02T06:36:13Z

src/main/java/domain/card/Hands.java

+        if (isAceNotExist() || isAceOne(sum)) {
+            return sum;
+        }
+        return sum + TEN;


Ace를 1 또는 11이 아니라 1 또는 20으로 볼 수 있다면? TEN이라는 네이밍은 변경에 유연하지 못할 것 같아요. 이름을 다시 고민해보는게 어떨까요?

KS-KIM · 2020-07-02T06:56:14Z

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

+    public Dealer() {
+    }


빈 생성자가 필요한 특별한 이유가 있나요?

KS-KIM · 2020-07-02T06:57:55Z

src/main/java/domain/user/PlayerFactory.java

+import java.util.List;
+import java.util.Map;
+
+public class PlayerFactory {


정적 유틸리티 클래스라면 빈 private 생성자를 추가하여 인스턴스 생성을 방지해봐요!

KS-KIM · 2020-07-02T07:02:13Z

src/main/java/domain/Game.java

+        List<Card> drawCards = new ArrayList<>();
+        for (int index = 0; index < players.size() * FIRST_DRAW_NUMBER; index++) {
+            drawCards.add(deck.deal());
+        }
+        players.firstDraw(drawCards);
+        dealer.drawFirst(deck.deal(), deck.deal());


Game 클래스가 Deck에서 모든 카드를 꺼내 Players에 전달해줘야 할까요? Players와 Deck간의 협력 관계를 통해 풀어갈 수 있는 부분이라고 생각합니다.

KS-KIM · 2020-07-02T07:05:32Z

src/main/java/controller/Controller.java

+import java.util.Map;
+import java.util.stream.Collectors;
+
+public class Controller {


어떤 역할을 하는 컨트롤러인지 명시적인 네이밍을 주는게 좋을 것 같아요.

KS-KIM · 2020-07-02T07:14:08Z

src/main/java/domain/money/EarningMoney.java

+    private static void calculateWhenStay(Hands dealerHands, Player player) {
+        if (dealerHands.isBust() || player.getHands().isBiggerThan(dealerHands)) {
+            player.stay();
+        }
+        if (dealerHands.isBiggerThan(player.getHands())) {
+            player.bust();
+        }
+    }
+
+    private static void calculateWhenBlackjack(Hands dealerHands, Player player) {
+        if (dealerHands.isBlackjack()) {
+            player.stay();
+        }
+    }
+
+    private static void calculateWhenBust(Hands dealerHands, Player player) {
+        if (dealerHands.isBust()) {
+            player.stay();
+        }
+    }


이미 패를 draw하는 단계에서 딜러와 모든 플레이어는 Finished 상태로 진입했을텐데, 여기서 또 stay, bust를 호출해야 할 특별한 이유가 있을까요?

KS-KIM · 2020-07-02T14:08:59Z

src/main/java/domain/user/Players.java

+    private void validate(final List<Player> players) {
+        Objects.requireNonNull(players, "플레이어는 1명 이상이어야 합니다. 입력값 : null");
+        if (players.isEmpty()) {
+            throw new IllegalArgumentException("플레이어는 1명 이상이어야 합니다. 입력값 : empty");
+        }
+    }


플레이어가 9명 이상인 시점부터 카드 52장을 모두 소모하고 부족할 가능성이 있는데, 최대 8명까지로 제한하는게 어떨까요?

YerinCho added 13 commits July 1, 2020 15:52

docs: 요구사항 업데이트

3dfe0a1

docs: 요구사항 업데이트

88cdd0b

feat: 플레이어 생성

0595db4

이름 입력 구현 배팅금액 입력 구현

refactor: 불변 보장

cee048c

refactor: 불변 보장

5570d56

feat: 기본 카드 구현

719e1c7

feat: 게임 시작 시 2장의 카드 지급 구현

016bd97

feat: 상태 추가

f8c9d20

feat: 카드 점수 계산 구현

176562c

feat: 각 플레이어의 카드 합이 21이하인 경우 카드 더 받기 구현

6ef10b7

feat: 딜러의 경우 16 이하인 경우 카드 한 장 더 지급

c9b4270

feat: 딜러와 플레이어의 카드, 점수합 출력

457056c

feat: 수익 구현

79af801

hongbin-dev reviewed Jul 2, 2020

View reviewed changes

KS-KIM reviewed Jul 2, 2020

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[오렌지] 블랙잭 미션 제출합니다. #1

[오렌지] 블랙잭 미션 제출합니다. #1

YerinCho commented Jul 1, 2020

hongbin-dev left a comment

hongbin-dev Jul 2, 2020

hongbin-dev Jul 2, 2020

hongbin-dev Jul 2, 2020

hongbin-dev Jul 2, 2020 •

edited

Loading

KS-KIM Jul 2, 2020 •

edited

Loading

hongbin-dev Jul 2, 2020

hongbin-dev Jul 2, 2020

hongbin-dev Jul 2, 2020

KS-KIM left a comment

KS-KIM Jul 2, 2020

KS-KIM Jul 2, 2020 •

edited

Loading

KS-KIM Jul 2, 2020 •

edited

Loading

KS-KIM Jul 2, 2020

KS-KIM Jul 2, 2020

KS-KIM Jul 2, 2020

KS-KIM Jul 2, 2020

KS-KIM Jul 2, 2020

KS-KIM Jul 2, 2020

KS-KIM Jul 2, 2020

KS-KIM Jul 2, 2020

	return cards.remove(0);
	return cards.remove(cards.size() - 1);


		import static domain.Game.FIRST_DRAW_NUMBER;

		public class Hands {

[오렌지] 블랙잭 미션 제출합니다. #1

Are you sure you want to change the base?

[오렌지] 블랙잭 미션 제출합니다. #1

Conversation

YerinCho commented Jul 1, 2020

hongbin-dev 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

hongbin-dev Jul 2, 2020 • edited Loading

Choose a reason for hiding this comment

KS-KIM Jul 2, 2020 • 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

KS-KIM left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KS-KIM Jul 2, 2020 • edited Loading

Choose a reason for hiding this comment

KS-KIM Jul 2, 2020 • 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

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

hongbin-dev Jul 2, 2020 •

edited

Loading

KS-KIM Jul 2, 2020 •

edited

Loading

KS-KIM Jul 2, 2020 •

edited

Loading

KS-KIM Jul 2, 2020 •

edited

Loading