[2단계 - 블랙잭 (배팅)] 도기(김동호) 미션 제출합니다. #523
Conversation
- 52장의 트럼프 카드를 가진다. - 가지고 있는 카드 한 장을 준다.
- Set에서 Stack으로 변경
There was a problem hiding this comment.
안녕하세요 도기!
코드가 많이 보기 좋아졌네요 ㅎ 리팩토링 후 뿌듯하셨을 것 같습니다
테스트 단위의 고민
현재는 단순히 호출을 위한 코드지만 나중에 세부 구현이 달라질 수 있습니다
복붙을 하더라도 테스트 코드가 필요하다고 생각해요.
디미터 법칙 지켜보기
getHand()와 getCards()를 분리하기 어려운 상황일까요~? 반드시 지켜야 하는 규칙은 아니므로 적당히 융퉁성을 발휘하시는 것도 방법입니다 ㅎ
리뷰 반영 및 기능 수정 후 다시 요청주세요 :>
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| public final class Bank { |
There was a problem hiding this comment.
블랙잭 게임 돈이 은행이라는 추상은 게임 결과물 금액 계산이라는 역할이 한번에 안 와닿긴 하는거 같아요 ㅎ 그치만 그대로 두셔도 됩니당
| } | ||
|
|
||
| public Money subtract(final Money money) { | ||
| // 최종 수익을 구할 때 마이너스가 나올 수 있기에, 음수 검증 생략 |
There was a problem hiding this comment.
해당 객체는 음수 validation을 애초에 안 하는 클래스니 해당 주석은 없어도 될거같아요 ㅎㅎ
Money가 음수 일 수 없다라는 생각이 드신다면 클래스명이 Money 대신 음수일 수 있는 금액으로 다른 추상(클래스명 변경)을 하는 것도 좋을 것 같습니다.
|
|
||
| private Money evaluate(final Player player, final Result result, final Money findMoney) { | ||
| if (isBlackJackWin(player, result)) { | ||
| return new Money((int) (findMoney.getMoney() * BLACKJACK_WIN_RATE)); |
There was a problem hiding this comment.
money#multiply(Money money)
하나 만들어주셔도 좋겠네요 ㅎㅎ
| return result == WIN && player.isBlackjack(); | ||
| } | ||
|
|
||
| public Money totalMoney() { |
| return this.hand.isBlackjack(); | ||
| } | ||
|
|
||
| public Result compareTo(Hand anotherHand) { |
There was a problem hiding this comment.
compareTo는 자주 쓰이는 Comparable의 구현 메서드기도 해서 다른 명칭을 쓰시는 게 좋을것 같아요 ㅎ Player가 Comparable을 상속할 경우 모두 바꿔야 합니다
| private static final int DECK_SIZE = 52; | ||
|
|
||
| private final List<Card> deck = new ArrayList<>(DECK_SIZE); | ||
| private final Stack<Card> deck = new Stack<>(); |
| } | ||
| } | ||
|
|
||
| private boolean ask(final Dealer dealer, final Player player) { |
There was a problem hiding this comment.
다시보니 ask는 너무 축약되어있습니다 ㅎ 어떤 행위를 물어보는건지 추가해주세요
| } | ||
|
|
||
| private void betMoney(final BlackjackGame blackjackGame) { | ||
| Bank bank = blackjackGame.getDealer().getBank(); |
There was a problem hiding this comment.
blackjack game을 만드셨다면 getDealer() 나 getPlayers()를 해서 로직을 전개하지 말고 BlackjackGame의 메서드를 통해 전달해주세요 ㅎ
출력용으로 객체로 나오는 건 상관 없지만 bank에 돈을 추가하는 등의 행위는 게임쪽에 있는게 맞다고 생각합니다.
| @@ -2,9 +2,9 @@ | |||
|
|
|||
| public enum Suit { | |||
| HEART("하트"), | |||
| SPADE("스페이드"), | |||
| DIAMOND("다이아몬드"), | |||
There was a problem hiding this comment.
엄밀히 말하면 "다이아몬드" 등은 콘솔 뷰에 의존적인 출력 로직입니다.
현재 미션에서 뷰 패키지가 도메인 패키지를 참조하는건 상관없다고 생각하지만,
도메인이 특정 뷰에 의존적인 코드를 가지는 건 다른 이야기입니다.
There was a problem hiding this comment.
저도 이 부분에 대해 도메인이 뷰를 위해 필드를 갖는 것이 찝찝했습니다..
그래서 따로 converter나 OutputView에서 값을 convert 해줄까도 했지만, Suit는 4가지만 있지만 Denomination을 모두 convert 해줄 생각하니, enum이 상태를 갖고 있는 것이 더 맞다고 생각되어 그냥 두었습니다.
이 부분은 다른 크루들도 많은 고민을 안고 있는 문제이지만, 동시에 어떻게 해야 좋을지 잘 모르겠는 문제인 것 같아요.
그런데, 콘솔 뷰에 의존적인 출력 로직 이라는 웨지의 말이 와닿지는 않아요.
이 부분이 어떤 점에서 콘솔 뷰에 의존적인 출력 로직인지 여쭤봐도 될까요??
There was a problem hiding this comment.
이 부분에 대해 곰곰이 생각해보았습니다.
우선, 뷰의 요구사항이 변경되었을 때 도메인이 변경된다는 문제점을 안고 있다는 것이 가장 큰 문제가 된다고 생각해요.
하지만 현재 프로그램 규모와 단일 콘솔 프로그램 환경임을 고려했을 때, 위 문제를 해결하기 위해 복잡한 코드를 추가할 만큼 직관적으로 해결할 방법을 포기하고 싶지 않았습니다.
제가 잘못 알고 있거나 잘못 생각하는 부분이 있을까요?
There was a problem hiding this comment.
앗 죄송합니다 ㅠ 코멘트를 못 보고 있었네요.
MVC 패턴이 해결하고자 하는 과제는 도메인 모듈화입니다. 차후 어떤 뷰가 들어오더라도 동일한 비즈니스 로직을 활용하려는 것이 목적이므로 MVC를 적용하기로 결심한 순간부터 특정 뷰에 의존적인 코드가 생기는 것은 안티패턴으로 볼 수 있습니다.
물론 하나의 코드로 인해 패턴이 무너지지는 않지만, 원칙에 벗어난 코드가 쌓여갈수록 전체 아키텍처가 빠르게 무너지는 모습을 현업 코드에서 자주 보게 됩니다.. (ㅠㅠ)
현재 Suit의 name 필드(ex. "다이아몬드")는 현재는 OutputView에 전달하기 위한 코드로 보이는데요, OutputView가 현재는 콘솔이므로 name필드는 잘 동작합니다.
하지만 OutputView가 웹 콘솔 이었다고 가정하면 해당 name은 이미지 에셋의 경로로 수정됬어야 할겁니다. (아마도 이미지 url이 됐겠네요)
해당 로직의 분리는 복잡하게가 아니더라도 OutputView 클래스 내에 다음과 같은 간단한 메서드 추가로도 해결될 거 같습니다. (요구사항으로 switch는 못 쓰게 했었던가용? ㅎㅎ)
private static String printSuit(Suit suit) {
switch (suit) {
case HEART:
return "하트";
case SPADE:
return "스페이드";
case DIAMOND:
return "다이아몬드";
case CLOVER:
return "클로버";
default:
throw new RuntimeException("정의되지 않은 모양입니다"); // 아니면 ""
}
}궁금한 점이 생기면 DM으로 주세요!! 코멘트를 놓치는 일이 생겼으니 DM으로 받겠습니다 ㅠ.ㅠ
|
안녕하세요 웨지 🥔 월요일은 무사히 잘 보내셨나요?? 항상 감사합니다 🙇♂️ |
안녕하세요 웨지! 🥔
저번에 DM에서 이야기 나눴지만, 이번에는 정말 요구사항 분석부터 시작하여 정말 싹 지우고 시작해보았습니다.
확실히 도메인 지식이 늘어난 상태에서 새롭게 요구사항 분석을 한 것도 많은 도움이 되었고, 이전과는 다르게 객체들의 역할이 분명하게 느껴졌습니다.
그럼 잘 부탁드립니다 🙇♂️
노력한 점
최대한 객체의 역할을 잘 나누려고 노력했습니다. (거의 이틀동안 온종일을 설계하는 데에 고민했어요 🥺)
잘게 나눈 것을 바탕으로 기능 하나하나 꼼꼼하게 README에 하나씩 적어가고 체크해가며 테스트도 하고 기능 구현도 하려고 노력했어요.
궁금했던 점
테스트 단위의 고민
Dealer의settingCards(Players players)를 구현하기 위해players.distributeHands()를 구현했습니다. 또,players.distributeHands()를 구현하기 위해player.receiveHand(Card card)를 구현했습니다. 그러다보니 추상화 계층 하위에 있는 메서드를 모두 테스트해야하나? 라는 고민이 생겼습니다.모든 메서드를 테스트한다면, 전반적인 프로그램의 안정성이 훨씬 올라갈 것 같아요. 하지만 추상화 계층 상위에 있는 메서드 하나만을 테스트했을 때의 효율성을 생각하게 됩니다. 어떤 방법이 더 효율적이고 적절한 방법일까요?
디미터의 법칙
Participant의getHand()를 기존에는return hand.getHand()같은 방식으로 하여 디미터의 법칙을 해결했습니다. 하지만 Controller나 OutputView에서Hand타입의 인스턴스가 필요할 때가 있어, 항상List<Card>타입으로 줄 수 없는 상황입니다. 이럴 때는 어떻게 해결할 수 있나요?