[1단계 - 블랙잭 게임 실행] 배키(백은희) 미션 제출합니다. #610
Conversation
… 도메인 로직을 콜백을 이용하여 분리
her0807
left a comment
her0807
left a comment
There was a problem hiding this comment.
안녕하세요 배키 🚣
도메인이 참 복잡하죠? ㅎㅎ 그래도 충분히 잘 구현해주셨습니다.
남겨주신 코멘트들에 추가 답변 달았어요~
한번만 더 리뷰 가시죠!! 파이팅💪🏻
| private String cardToString(Card card) { | ||
| String cardNumber = cardNumberToString(card.getNumber()); | ||
| String cardShape = cardShapeToString(card.getShape()); | ||
| return cardNumber + cardShape; | ||
| } |
There was a problem hiding this comment.
조금 더 쉽게 말씀드리자면,
현재 View 에서 직접적으로 도메인을 알고 있어요.
이렇게 되면 출력하는 로직에서 userCards 에 다른 메서드들을 호출하여 행위를 할 수도 있겠죠?
그렇기 때문에 계층별 분리를 위해서 Dto 를 사용해보는 것을 의도하고 드렸던 코멘트입니다!
| private String cardNumberToString(CardNumber cardNumber) { | ||
| if (cardNumber == CardNumber.ACE) { | ||
| return "A"; | ||
| } | ||
| if (cardNumber == CardNumber.QUEEN) { | ||
| return "Q"; | ||
| } | ||
| if (cardNumber == CardNumber.KING) { | ||
| return "K"; | ||
| } | ||
| if (cardNumber == CardNumber.JACK) { | ||
| return "J"; | ||
| } | ||
| return String.valueOf(cardNumber.minimumNumber()); | ||
| } |
There was a problem hiding this comment.
음 ~ 배키가 생각하기에 view 에 영역에서 알아야할 내용이라고 판단하면 이렇게 진행하는게 맞죠!
위 코멘트를 남긴 이유는 ACE 는 A, QUEEN 는 Q 라고 명칭하는 것은 실제 카드세계에서 쓰이는 규칙이기 때문에 그 규칙을 담당하는 객체가 알고 있어야한다고 생각했어요. 그런데 배키는 이 부분을 뷰에서 표현하기 나름이고, 변경 가능성이 있다고 판단한거니 그렇게 진행해도 될것 같아요~
| @@ -0,0 +1,16 @@ | |||
| package model.player; | |||
|
|
|||
| public class Dealer extends Player { | |||
| private BlackJack createBlackJack() { | ||
| List<String> names = inputView.askParticipantNames(); | ||
| List<Participant> players = new ArrayList<>(names.stream() | ||
| .map(Participant::new).toList()); | ||
| return new BlackJack(new Participants(players), new Dealer()); | ||
| } |
There was a problem hiding this comment.
배키가 변경하신 내용처럼 참가자들을 생성하는 로직을 분리하기를 기대했어요~
잘하셨습니다 😀
| } | ||
| } | ||
|
|
||
| private void checkDealerMoreCards(BlackJack blackJack) { |
There was a problem hiding this comment.
check 라고 표현되어 있어서 검증만 하는 것이 맞다고 생각했었어요.
decideParticipantPlay 로 변경되면서 현재 구조도 좋아보입니다!
src/main/java/model/card/Cards.java
Outdated
|
|
||
| public static List<Card> selectRandomCards(int size) { | ||
| if (cards == null) { | ||
| Cards.createCards(); |
There was a problem hiding this comment.
자 조금 더 고민해보아요!
덱에서 createCards 를 할 책임이 있을까요? 🫢
카드를 생성해서 덱으로 가지고 있는건 딜러가 주체일 것 같아요.
카드를 한장씩 주고 말고를 행위하는 것도 딜러이기 때문에
딜러를 생성하는 곳 (블랙잭 시작) 하는 곳에서
딜러 생성 전에 덱을 생성하고 딜러에게 주입해주는 형태로 변경해보면 어떨까요?
public List<Card> createCards() {
List<Card> cards = new ArrayList<>();
for (CardShape cardShape : CardShape.values()) {
createSameShapeCards(cardShape, cards);
}
return cards;
}| public int calculateScore() { | ||
| int sum = cards.stream().mapToInt(Card::minimumNumber).sum(); | ||
|
|
||
| List<Integer> differenceNumbers = mapDifferenceNumber(); | ||
| return differenceNumbers.stream() | ||
| .reduce(sum, this::changeToBestNumber); | ||
| } |
There was a problem hiding this comment.
지금 현재 로직은 1을 더한 뒤에 값이 바뀌면 10을 더해주는 방식으로 진행됩니다.
리뷰어님께서 말씀해 주신 내용은 ACE 값을 더하는 즉시 1과 11로 계산해야 한다는 걸까요?
만약 List 값에서 Ace에 해당하는 값이 중간에 있으면 Ace를 제외한 총 합계를 알지 못해서 바로 ACE가 11인지 1인지 판단하는 것이 어렵다는 생각이 듭니다., 방법이 있을까요?
There was a problem hiding this comment.
당연히 즉시 계산을 할 수는 없죠! 총합이 계산되고 나야 11로 할지, 1로 할지 결정할 수 있으니까요~
다만 여기에서 총합에 대한 변경이 일어나는데 그것이 드러나지 않는 것 같아서 1로 할지 11로 할지 계산 주체는 여기 로직에서 드러나야하지 않을까? 라는 생각이 들었어요!
지금도 크게 나쁘지는 않아서 유지해도 괜찮다고 생각합니다
리뷰어님 말씀을 듣고 딜러를 생성하는 곳에서 딜러 생성 전에 카드를 생성한 뒤, 밑의 코드와 같이 딜러에게 카드를 주입하였습니다! // CardDeck을 관리하는 Dealer 객체
public class Dealer extends User {
private static final int NUMBER_THRESHOLD = 16;
private static final String DEALER_NAME = "딜러";
private final CardDeck cardDeck;
public Dealer(CardDeck cardDeck, Supplier<List<Card>> supplier) {
super(DEALER_NAME, supplier.get());
this.cardDeck = cardDeck;
}
...
}다만 아쉬운 점은 딜러를 생성할 때 아래 첫번째 코드처럼 진행하면 테스트를 실행할 때 딜러의 초기 값을 설정하지 못하는 단점이 발생합니다. public Dealer(CardDeck cardDeck) {
super(DEALER_NAME, cardDeck.selectRandomCards(CardSize.TWO));
this.cardDeck = cardDeck;
}이를 해결하고자 아래와 같이 구현했습니다. public class BlackJack {
private BlackJack createBlackJack(List<String> names) {
....
Dealer dealer = new Dealer(cardDeck, () -> cardDeck.selectRandomCards(CardSize.TWO));
return new BlackJack(participants, dealer);
}
}
// CardDeck을 관리하는 Dealer 객체
public class Dealer extends User {
private static final int NUMBER_THRESHOLD = 16;
private static final String DEALER_NAME = "딜러";
private final CardDeck cardDeck;
public Dealer(CardDeck cardDeck, Supplier<List<Card>> supplier) {
super(DEALER_NAME, supplier.get());
this.cardDeck = cardDeck;
}
...
}그러나 이렇게 하면 제가 느끼기엔 외부에서 카드를 뽑는?느낌이 듭니다..🥲 더 좋은 방법이 있을까요? |
cardDeck.selectRandomCards(CardSize.TWO) 이부분에서 랜덤으로 카드가 나오기 때문이죠? 그러면
|
her0807
left a comment
her0807
left a comment
There was a problem hiding this comment.
안녕하세요 배키~
구현해주신 내용 확인했습니다. 크고작은 변경사항들 고민하고 반영해주어서
구조가 훨씬 나아진 것 같아요 👍🏻
2단계로 가시죠!!! ㅎㅎ
2 participants
안녕하세요! 수달 🤗
이번 미션 잘 부탁드립니다~ 미션을 진행하면서 궁금한 점이 몇 가지 있어서 여기에 남겨둡니다.
감사합니다 :)
카드를 어떻게 관리하는가?
현재 저희는 52가지의 카드를 Card객체로 생성한 뒤, Cards객체에 담았습니다.
이 방식으로 카드를 관리하면 관리가 쉽고 카드를 제작할 때 참가자와 결합도가 줄어드는 장점이 있습니다.
또 다른 방식을 생각했는데요..! Card객체를 생성하지 않고 참가자가 카드를 뽑을 때 랜덤으로 카드를 생성한 뒤,
참가자의 카드를 모두 비교하고 동일한 카드가 존재한다면 다시 랜덤으로 생성한 카드를 참가자에게 건네는 것입니다.
이에 대한 장점은 사용하지 않는 카드가 생성되지 않는다는 점입니다.
Scanner를 선언하는 방식
저번 미션에서는 아래와 같이 Scanner를 사용했습니다.
private static final Scanner = new Scanner(System.in);그러나 Scanner가 static이라는 점이 조금 걸리는 것 같습니다.. 이 방식이 괜찮을지 궁금합니다!