[1단계 - 블랙잭 게임 실행] 로빈(임수빈) 미션 제출합니다. #1
Conversation
| public class YesOrNoInputView { | ||
| public static Boolean getYNAsBoolean() { | ||
| String input = Console.getInputFromConsole(); | ||
| if (input.equals("y")) { | ||
| return true; | ||
| } | ||
| if (input.equals("n")) { | ||
| return false; | ||
| } | ||
| throw new IllegalArgumentException("입력 형식이 올바르지 않습니다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
input을 관리하는 InputView를 분리 하셨군요.
혹시 이렇게 나누었을때 장점은 무엇인가요?
There was a problem hiding this comment.
input을 관리하는 InputView를 분리 하셨군요. 혹시 이렇게 나누었을때 장점은 무엇인가요?
책임이 작게 분리되어 적절한 객체로 분리되었을 때의 장점과 동일합니다.
| private final Gamer dealer; | ||
| private final List<Gamer> players; |
There was a problem hiding this comment.
controller의 필드에 두 도메인이 있는 객체를 선언한 이유가 궁금합니다.
이렇게 했을때 장점이 무엇인가요?
There was a problem hiding this comment.
controller의 필드에 두 도메인이 있는 객체를 선언한 이유가 궁금합니다. 이렇게 했을때 장점이 무엇인가요?
장점이 있다기 보다는 딱히 별로 중요한 부분이라 생각하지 않았습니다. 어차피 컨트롤러 내부 private 메서드 이곳 저곳에서 도메인을 사용하기 때문에, 필드로 두든, 메서드를 통해 넘겨주든 실질적인 차이가 있다고 생각하지 않습니다.
There was a problem hiding this comment.
Deck은 단순히 요구사항 때문에 인자로 안넣으신건지 궁금합니다.
네. 맞습니다.
| String namesOutput = players.stream().map(Gamer::getRawName).collect(Collectors.joining(", ")); | ||
| OutputView.print("딜러와 %s에게 2장을 나누었습니다.".formatted(namesOutput)); |
src/main/java/Main.java
Outdated
| import controller.BlackjackController; | ||
| import domain.blackjack.Gamer; | ||
| import domain.blackjack.HoldingCards; | ||
| import domain.card.Deck; | ||
| import java.util.List; | ||
| import view.NameInputView; | ||
| import view.OutputView; |
There was a problem hiding this comment.
패키지 구조를
src/main/java 아래
- controller
- domain
- view
등이 바로 있는데 이런경우에 이렇게
import 중간에 java.util.List 가 끼게 됩니다.
하나의 패키지로 더 묶으면 좋을 것 같습니다.
| public static void print(GamerDTO gamerDTO) { | ||
| String outputWithoutSummationCardPoint = generateOutputWithoutSummationCardPoint(gamerDTO); | ||
| String summationCardPointOutput = "결과: %d".formatted(gamerDTO.getSummationCardPoint()); | ||
| System.out.printf("%s - %s\n", outputWithoutSummationCardPoint, summationCardPointOutput); | ||
| } |
There was a problem hiding this comment.
view 의 모든 로직이 static인데 이랬을때 어떤 장점이 있나요?
view가 사용되는 곳은 controller 밖에 없어 컨트롤러에 선언해서 사용한다면 객체 생성의 메모리적인 부담은 많이 없을것 같은데 이유가 궁금합니다!
| } | ||
| if (input.equals("n")) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
input.equals("y") 로 비교하게 된다면 input 에 null 이 오게 된다면 NPE가 발생하게 됩니다!
"y".equals(input) 과 같이 null 이 아닌 값에 equals을 붙여주는 것이 좋을 거 같아요
| static CardPoint calculate(Card card) { | ||
| CardName cardName = card.name(); | ||
| int cardNumber = cardName.getCardNumber(); | ||
| if (cardNumber > TEN.getCardNumber()) { | ||
| return new CardPoint(TEN.getCardNumber()); | ||
| } | ||
| return new CardPoint(cardNumber); | ||
| } |
There was a problem hiding this comment.
블랙잭에서 j,q,k 는 10으로 계산한다! 라는 룰이 있어 enum 값 세팅할 때 10으로 해두어도 되지 않았을까요?
이렇게 설계한 로빈의 생각이 궁금합니다!
There was a problem hiding this comment.
블랙잭에서 j,q,k 는 10으로 계산한다! 라는 룰이 있어 enum 값 세팅할 때 10으로 해두어도 되지 않았을까요? 이렇게 설계한 로빈의 생각이 궁금합니다!
저는 블랙잭의 규칙은 트럼프 카드 고유의 것이라 생각하지 않았습니다. 블랙잭은 트럼프 카드를 사용하는 여러 게임 중 하나기 때문에 블랙잭의 규칙이 트럼프 카드를 추상화한 클래스 내부에 있으면 안된다고 생각했습니다.
| try { | ||
| dealerDraw(deck); | ||
| OutputView.print("딜러는 16이하라 한장의 카드를 더 받았습니다.\n"); | ||
| } catch (IllegalStateException ignored) { | ||
|
|
||
| } |
There was a problem hiding this comment.
예외를 무시하지 말고 어떤 예외가 발생하였는지 메시지를 던져보는건 어떤가요?
There was a problem hiding this comment.
예외를 무시하지 말고 어떤 예외가 발생하였는지 메시지를 던져보는건 어떤가요?
이 부분은 애초에 예외를 여기서 처리하는게 아니라 도메인 내부에서 잡아야 할 것 같아서 리팩토링중입니다.. ㅠㅠ
| BufferedReader bufferedReader = new BufferedReader(new InputStreamReader(System.in)); | ||
| String input; | ||
| try { | ||
| input = bufferedReader.readLine(); | ||
| } catch (IOException e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| return input; | ||
| } |
There was a problem hiding this comment.
try {
return bufferedReader.readLine();
} catch () {
thr
}이렇게 바로 리턴해주는건 어떤가요?
| System.out.printf("%s: %s\n", gamerName, gameResult); | ||
| } | ||
|
|
||
| public static void print(DealerGameResultDTO dealerGameResultDTO) { | ||
| Map<GameResult, Integer> dealerGameResultCounts = dealerGameResultDTO.getDealerGameResultCounts(); | ||
| String dealersGameResultOutput = dealerGameResultCounts.entrySet() | ||
| .stream() | ||
| .map(mapEntry -> mapEntry.getValue() + mapToString(mapEntry.getKey())) | ||
| .collect(Collectors.joining(" ")); | ||
| System.out.printf("딜러: %s\n", dealersGameResultOutput); |
There was a problem hiding this comment.
"/n" 을 사용하면 운영체제 마다 다른 줄바꿈이 됩니다!
printf()를 사용하면 "%n" 을 사용하면 해결됩니다!
| String cardsOutput = cards.stream() | ||
| .map(card -> mapToString(card.cardType()) + mapToString(card.name())) | ||
| .collect(Collectors.joining(", ")); | ||
| return "%s: %s".formatted(nameOutput, cardsOutput); |
There was a problem hiding this comment.
4스페이드 형태로 출력해야 하는데 스페이드4로 출력하고 있어요!
| 5. 모든 플레이어가 카드를 더 받을 수 없는 경우, 딜러의 턴으로 넘어간다. | ||
| 6. 딜러의 턴에서 딜러는 딜러의 점수가 16점 이하인 경우 카드를 더 받는다. 17점 이상인 경우 카드를 더 받지 않는다. | ||
| 7. 딜러의 턴이 끝나면, 최종 점수 계산 및 게임의 승패를 가린다. | ||
| 8. 최종 점수가 21점에 가장 가까우면서 21점을 넘기지 않는 사람이 승리한다. 동점인 플레이어(딜러 포함)이 나온 경우, 무승부로 판단한다. |
There was a problem hiding this comment.
모든 플레이어가 버스트가 된 경우 딜러는 카드를 뽑지 않아도 승리하기 때문에 딜러가 승이라고 생각합니다!
|
|
||
| @Override | ||
| boolean canDraw() { | ||
| return !player.getSummationCardPoint().isDeadPoint(); |
There was a problem hiding this comment.
!player.getSummationCardPoint().isDeadPoint();
대신
!player.isDeadPoint();
이렇게 get을 하지 않고 하는건 어떨까요?
There was a problem hiding this comment.
!player.getSummationCardPoint().isDeadPoint();
대신
!player.isDeadPoint(); 이렇게 get을 하지 않고 하는건 어떨까요?
그러게요.. 그냥 되면 넘어가서 이렇게 된 것 같습니다. 애초에 Gamer 내부에 이미 메서드가 있습니다.. ㅋㅋ
| @@ -0,0 +1,27 @@ | |||
| package domain.card; | |||
|
|
|||
| public enum CardName { | |||
There was a problem hiding this comment.
enum 이름을 CardName 이라고 지었는데 카드 네임이면 4스페이드 처럼 숫자모양 전부 가지고 있을 거 같은 이름이라고 생각하는데 로빈의 의견은 어떤가요?
src/main/java/view/OutputView.java
Outdated
| public static void print(String output) { | ||
| System.out.println(output); | ||
| } | ||
| } |
There was a problem hiding this comment.
저는 출력할 메시지마다 따로 메서드를 만들어서 출력문구는 해당 메서드안에서 사용하였는데, 그렇게 사용했을 때의 장점은 출력 메시지는 view 패키지 안에만 있어 분리가 잘되었다는 장점이 있었습니다!
로빈의 의견은 어떤가요?
There was a problem hiding this comment.
저는 출력할 메시지마다 따로 메서드를 만들어서 출력문구는 해당 메서드안에서 사용하였는데, 그렇게 사용했을 때의 장점은 출력 메시지는 view 패키지 안에만 있어 분리가 잘되었다는 장점이 있었습니다!
로빈의 의견은 어떤가요?
저는 구현 당시 출력해야 할 출력 문구를 예제 그대로 복사해서 컨트롤러에서 출력해야 할 시점에 System.out.println 으로 일단 적어두고 했습니다. 그러면 제가 어디서 뭘 출력해야 하는지 알아보기 쉬워서요.
말씀하신 것 처럼 출력은 View에서 담당해야 하니까, View로 옮기는게 더 적절한 것 같아요.
There was a problem hiding this comment.
이부분은 매번 미션 할때마다 다르게 했네요. ㅋㅋ 별로 중요한 부분은 아니라고 생각했어요.
src/main/java/Main.java
Outdated
| public class Main { | ||
| public static void main(String[] args) { | ||
| Gamer dealer = new Gamer("딜러", HoldingCards.of()); | ||
| OutputView.print("게임에 참여할 사람의 이름을 입력하세요.(쉼표 기준으로 분리)"); | ||
| List<Gamer> players = NameInputView.getNames().stream() | ||
| .map(name -> new Gamer(name, HoldingCards.of())) | ||
| .toList(); | ||
| BlackjackController blackjackController = new BlackjackController(dealer, players); | ||
| blackjackController.startBlackjackGame(Deck.fullDeck()); | ||
| } | ||
| } |
| private void dealerDraw(Deck deck) { | ||
| dealer.draw(deck, new DealerRandomCardDrawStrategy(dealer)); | ||
| } | ||
|
|
||
| private void playerDraw(Deck deck, Gamer player) { | ||
| player.draw(deck, new PlayerRandomCardDrawStrategy(player)); | ||
| } |
There was a problem hiding this comment.
엇.. draw가 호출될 때마다 전략 인스턴스가 생성되네요. 괜찮을까요?
| private void dealerDraw(Deck deck) { | ||
| dealer.draw(deck, new DealerRandomCardDrawStrategy(dealer)); | ||
| } | ||
|
|
||
| private void playerDraw(Deck deck, Gamer player) { | ||
| player.draw(deck, new PlayerRandomCardDrawStrategy(player)); | ||
| } |
There was a problem hiding this comment.
전략 인스턴스를 실질적으로 사용하는 곳이 Deck.class 인 것 같아요.
depth가 꽤 깊어지는 것 같은데, 전략 생성에 대한 책임을 controller가 맡는 것에 대한 근거가 있을까요?
고민하신 내용이 궁금해요 💭 로빈의 생각을 공유해주시면 감사하겠습니다~
| private static void printDealer(Gamer dealer) { | ||
| List<Card> rawHoldingCards = new ArrayList<>(dealer.getRawHoldingCards()); | ||
| rawHoldingCards.remove(0); | ||
| GamerDTO gamerDTO = new GamerDTO(dealer.getRawName(), rawHoldingCards, | ||
| dealer.getRawSummationCardPoint()); | ||
| GamerOutputView.printWithoutSummationCardPoint(gamerDTO); | ||
| } |
There was a problem hiding this comment.
단 한 번 실행되긴 하지만, remove의 성능에 대해 생각해 보시면 좋을 것 같아요!
| GamerDTO gamerDTO = new GamerDTO(dealer.getRawName(), rawHoldingCards, | ||
| dealer.getRawSummationCardPoint()); |
There was a problem hiding this comment.
이 부분 개행에 대한 로빈의 생각이 궁금해요 💭
참고로 저.. 이런 개행 좋아해요 😀
| throw new IllegalStateException("카드를 더이상 뽑을 수 없습니다."); | ||
| } | ||
|
|
||
| abstract boolean canDraw(); |
There was a problem hiding this comment.
전략 대신에 Gamer나 Dealer에게 물어보는 것은 어떤가요?
| @@ -0,0 +1,16 @@ | |||
| package domain.blackjack; | |||
|
|
|||
| public class DealerRandomCardDrawStrategy extends AbstractRandomCardDrawStrategy { | |||
There was a problem hiding this comment.
개인적인 의견이니 참고만 해주세요!
추상화 depth가 깊다고 생각해요. 꼭 필요한 추상화였을까 🤔 싶어요!
물론 저도 동일한 미션을 진행한 크루로서 도메인 지식이 있는 상태이기에 읽는 데에 무리는 없었지만,
각 클래스가 담당한 역할이 많이 않아서 추상화를 덜 해도 괜찮을 것 같단 생각이 들었습니다~
| package domain.blackjack; | ||
|
|
||
| public class DealerRandomCardDrawStrategy extends AbstractRandomCardDrawStrategy { | ||
| private final Gamer dealer; |
There was a problem hiding this comment.
의존성에 대한 의문인데요
현재 코드는 전략이 Gamer를 알고 있고, Gamer도 전략을 알고 있는 것 같아요.
서로에게 의존하고 있는데, 결국 클래스 간의 결합도가 다소 높다는 생각이 드네요 🙂
| public static HoldingCards of(Card... cards) { | ||
| return new HoldingCards(new ArrayList<>(List.of(cards))); | ||
| } |
There was a problem hiding this comment.
성능적인 측면에서 비즈니스 코드에서 가변 인자는 조심할 필요가 있을 것 같아요 😉
src/main/java/domain/card/Deck.java
Outdated
| public static Deck fullDeck() { | ||
| List<Card> cards = Arrays.stream(CardType.values()) | ||
| .map(Deck::makeSameTypeCards) | ||
| .flatMap(Collection::stream) | ||
| .toList(); | ||
| return new Deck(cards); | ||
| } | ||
|
|
||
| private static List<Card> makeSameTypeCards(CardType cardType) { | ||
| return Arrays.stream(CardName.values()) | ||
| .map(cardName -> new Card(cardName, cardType)) | ||
| .toList(); | ||
| } |
There was a problem hiding this comment.
다른 크루들에게도 드린 질문인데요!
모양과 숫자로 조합할 수 있는 모든 경우로 덱을 채우는 걸로 보여요.
이 로직에서 중첩 반복문을 사용하는 것에 대해 어떻게 생각하시나요~?
저는 중첩 반복문이 훨씬 명확하다고 생각했고, 사용했어요!
indent < 2라는 요구 사항이 없다면, 이에 대한 로빈의 생각이 궁금해요 💭
상속이 아니라 조합을 이용함
안녕하세요! 이번 미션은 이전의 미션들과는 다르게 요구사항에 선택의 여지가 조금 더 많았던 것 같습니다. 아래 주요 요구사항을 짧게 요약했습니다. 자세한 요구 사항은 README 를 참고해 주세요!
해석의 여지가 있는 주요 요구 사항
딜러의 드로우
딜러는 최초로 지급 받은 카드의 점수 합이 16 이하인 경우 딱 한번 추가 드로우를 합니다.
Ace 카드 처리
Ace 카드는 카드의 소유자에게 유리한 방향으로 해석합니다.
딜러의 경우 Ace를 1로 해석한 경우 추가 드로우가 발생하지만, 11로 해석할 경우 추가 드로우가 발생하지 않는 경우 추가 드로우를 수행하지 않습니다.
봐주셨으면 하는 부분
Card를 Enum화 하는 것이 더 나은가?
요약 : Card를 Enum 으로 만드는 것이 더 나을까요?
카드는 총 52개의 경우만 나올 수 있습니다. 따라서, Enum 으로 정의하는 것이 가능합니다. 하지만, 52개나 되는 Enum 값을 적는 것이 많다고 생각해, 카드의 종류(하트, 클로버, 스페이드, 다이아몬드) 와 카드 이름(Ace, 2 ~ 10, Jack, Queen, King) 을 각각 Enum으로 만든 뒤 이를 이용해 카드 객체를 생성하도록 했습니다.
그런데, 이렇게 하니까 테스트 코드에서 적절한 카드를 생성할 때 마다 조금 더러워지는 부분이 있었습니다.
DTO도 필드의 개수가 3개가 되면 안되나?
요약 : DTO도 필드의 개수가 3개 이상이면 부적절한 것인가요?
이번 미션의 프로그래밍 요구 사항에 인스턴스 필드의 개수가 3개 이하가 되도록 하라는 요구사항이 있었습니다.
이 요구사항의 의도가 클래스에 너무 많은 책임을 주지 말고, 클래스 내부에서도 메서드가 공유하는 상태가 최대한 적게 하여 유지보수성을 높이라는 것으로 해석했습니다.
하지만 DTO는 내부에서 별도의 로직을 같지 않고, 단순히 데이터를 감싸서 전달하는 것이기 때문에, 필드들이 의미적으로 높은 연관이 있다면 3개 이상의 필드를 사용하는 것이 요구사항의 의도를 해치지 않는다고 판단했습니다.