[라빈] 블랙잭 2단계 미션 제출합니다. #65
Conversation
src/main/java/view/InputView.java
Outdated
| playerBettingMoney.add(bettingMoney); | ||
| } | ||
|
|
||
| Map<RequestPlayerNameDTO, RequestPlayerBettingMoneyDTO> playerInformation = new LinkedHashMap<>(); |
There was a problem hiding this comment.
RequestPlayerNameDTO와 RequestPlayerBettingMoneyDTO로 각각 만들어야할 이유가 있을까요
dto를 합치면 더 깔끔하게 코드가 나올것 같은데..
| private static void runBlackJackPerUser(CardDeck cardDeck, User user) { | ||
| if (user.isBlackJack()) { | ||
| private static void runBlackJackPerPlayer(CardDeck cardDeck, Player player) { | ||
| if (player.isBlackJack()) { |
There was a problem hiding this comment.
블랙잭 여부는 while 실행될 때 걸러져야하지 않을까요?
컨트롤러에서 게임 규칙을 거르면 안됩니다
| @@ -8,6 +8,7 @@ | |||
| public class CardCalculator { | |||
There was a problem hiding this comment.
부모 클래스를 User로 변경했으면 메서드명도 그에 맞게 바꿔야하지 않을가요?
| @@ -8,6 +8,7 @@ | |||
| public class CardCalculator { | |||
| private static final int BLACK_JACK = 21; | |||
There was a problem hiding this comment.
블랙잭 조건은 카드 합이 21일 때일까요?
카드 2장이면서 카드 합이 21인 경우 아닌가요?
| playerCardsSum += SUM_CONTAIN_ACE; | ||
| } | ||
| return playerCardsSum; | ||
| } | ||
|
|
||
| private static boolean isPlayerCardsSumWithAceStrategyUnderBlackJack(int playerCardsSum) { |
| private static int calculateCards(List<Card> cards) { | ||
| return (int) cards.stream() | ||
| .mapToLong(Card::getCardNumber) | ||
| .sum(); | ||
| } | ||
|
|
||
| public static boolean determineWinner(Cards userCards, Cards dealerCards) { | ||
| if (userCards == null || dealerCards == null) { | ||
| public static boolean isPlayerCardsSumOverDealerCardsSum(Cards playerCards, Cards dealerCards) { |
| } | ||
| } | ||
|
|
||
| private static void runDealerBlackJack(CardDeck cardDeck, Dealer dealer) { | ||
| Cards dealerCards = dealer.getCard(); | ||
| if (dealerCards.isCardsSumUnderSixteen()) { | ||
| int dealerHitCount = 0; |
There was a problem hiding this comment.
피드백을 보고 다시 생각해보니까 필요가 없는 변수였네요! 로직을 항상 더 나은 방법을 다시 한번 생각해 봐야겠습니다. 감사합니다!
| private ProfitCalculator() { | ||
| } | ||
|
|
||
| public static Double getProfit(Player player, Dealer dealer) { |
There was a problem hiding this comment.
게임 조건이 늘어날 수록 if 절이 복잡하게 늘어나겠네요
ProfitCalculator가 필요한게 맞을까요?
getter를 안쓰려면 어떻게 해야할까요?
| @@ -0,0 +1,5 @@ | |||
| package domain.profit; | |||
|
|
|||
| public interface ProfitStrategy { | |||
There was a problem hiding this comment.
| public interface ProfitStrategy { | |
| public interface ProfitStrategy { | |
| double getProfit(int bettingMoney); | |
| boolean condition(Player player, Dealer dealer); | |
| } |
이렇게 바꿔보면 어떨까요?
| private ProfitFactory() { | ||
| } | ||
|
|
||
| public static ProfitStrategy create(Player player, Dealer dealer) { |
There was a problem hiding this comment.
새로운 ProfitStrategy이 생길 때마다 if가 계속 늘어나겠네요
객체지향적으로 바꿔볼까요?
There was a problem hiding this comment.
private static final List<ProfitStrategy> strategies
= Arrays.asList(new BothBlackJack(), new PlayerLoose(), new PlayerWin(), new PlayerBlackJack());ProfitStrategy에 남긴 코멘트를 참고하고 팩토리에 static으로 위 리스트를 추가해보면 어떨까요?
src/main/java/view/OutputView.java
Outdated
| @@ -13,6 +13,8 @@ | |||
| private static final String DELIMITER = ", "; | |||
| private static final int DEALER_INDEX = 0; | |||
| private static final int START_USER_INDEX = 1; | |||
| private static final int DEALER_NOT_HIT = 2; | |||
| private static final double DEALER_PROFIT_SUM_RATE = -1; | |||
구구 안녕하세요!
가장 먼저 머지하면서 달아주셨던 피드백에 대한 수정부터 하고 기능 구현 했습니다.
페어랑 다시 만나서 이야기를 해봤는데 현재 게임에 참여하는 클래스의 구조가 Player 를 User 와 Dealer 가 상속받았는데 User 라는 이름이 Player 보다 더 포괄적인 개념으로 느껴져서 User 와 Player 의 클래스 이름을 바꾸었습니다.
이번에 코드의 중복을 제거하기 위해 많이 노력해봤습니다. 게임 규칙에 관련된 상수(딜러의 카드를 더 받는 기준인 16, 블랙잭의 기준인 21 등) 을 한 클래스에서만 관리하기 위해 CardCarculator 로 옮기고 이 클래스의 메서드들을 활용해 다른 클래스의 책임을 구현했습니다.
DTO 의 테스트 코드는 getter 를 테스트 하는 테스트 코드였는데 getter 는 굳이 테스트 할 필요가 없다고 느껴져서 DTO 관련 테스트 코드는 모두 삭제하였습니다.
게임 규칙을 좀 더 찾아봤는데 딜러가 16이하의 카드를 갖고있을 때 16을 초과할 때 까지 계속 카드를 받는 것이라고 해서 기능을 수정했습니다.
감사합니다!