[블랙잭 게임 미션] 김병준 1단계 미션 제출합니다. #4
Conversation
| String[] playerNameList = playerNameStr.split(","); | ||
| validateDuplicatedPlayerName(playerNameList); |
There was a problem hiding this comment.
","로 나눠서 입력 받는 것은 입력 요구사항이니 inputView가 "," 까지는 잘라준 List을 반환해주는 것은 어떨까요??
| public int converter(CardRank cardRank) { | ||
| if ((cardRank == CardRank.JACK) || (cardRank == CardRank.QUEEN) || (cardRank == CardRank.KING)) { | ||
| return 10; | ||
| } | ||
| if (cardRank != CardRank.ACE) { | ||
| return Integer.parseInt(cardRank.getValue()); | ||
| } | ||
| return 0; | ||
| } |
|
|
||
| public void distributeOneCard(CardDeck cardDeck, Participant participant) { | ||
| Card card = cardDeck.getCardRandomly(); | ||
| participant.getCardList().addCard(card); |
There was a problem hiding this comment.
get으로 가져와서 직접 넣는 것은 participant의 card 구현체를 드러내는 것이기 때문에
participant.giveCard(card) 이렇게 만드는 것이 좋아보입니다!!
| private final CardSuit cardSuit; | ||
| private final CardRank cardRank; |
| Random random = new Random(); | ||
| int randomIndex = random.nextInt(this.cardDeck.size()); | ||
| Card randomCard = this.cardDeck.get(randomIndex); | ||
|
|
||
| this.cardDeck.remove(randomIndex); | ||
|
|
||
| return randomCard; |
There was a problem hiding this comment.
getCardRanomly를 호출할 때 마다 Random을 생성하지 말고
CardDeck을 생성할 때 섞어두면 어떨까요?? 섞는 방법은
Collections 의 메서드들을 한번 찾아보면 좋을 것 같습니다!!
| public class Participant { | ||
| private String name; | ||
| private CardList cardList; |
There was a problem hiding this comment.
상속을 사용하셨네요!!
상속을 사용하신 이유를 알려주실 수 있을까요??
There was a problem hiding this comment.
Player랑 Dealer를 하나의 List로 묶어서 사용할 일이 많아서 Participant로 자동 형변환을 위해 사용했습니다 !
|
|
||
| import java.util.List; | ||
|
|
||
| public class CalculateScore { |
There was a problem hiding this comment.
CaculateScore가 하는 행위를 만들어주신 다른 객체에 포함시키면 좋을 것 같아요!!
그 판단을 "자신의 필드에 대한 책임을 객체 스스로 책임진다"는 원칙을 기준으로 찾아보시면 좋을 것 같습니다.
| } | ||
| return sum; | ||
| } | ||
| } |
| private int addOneOrEleven(int sum) { | ||
| sum += 1; | ||
| if (sum + 11 <= Constant.TARGET_SCORE) { | ||
| sum += 10; | ||
| } | ||
| return sum; | ||
| } |
There was a problem hiding this comment.
요 메서드 명을 calculateAceValue로 가는 것은 어떨까요??
|
|
||
| import java.util.List; | ||
|
|
||
| public class CardDistributor { |
There was a problem hiding this comment.
카드를 나눠주는 것도 객체로 만들어 주셨네요!!
생각해보면 블랙잭이 생성되면 항상 딜러랑 유저는 2장의 카드를 받아야합니다!!
그래서 blackJack이란 객체를 만들어서 balckJack을 생성할 때 딜러랑, 유저를 생성자로 넘기고 카드를 배분하는 로직까지 생성자에 포함시키면 어떨까요??
public BackJack(List<Participant> participants, Dealer dealer) {
/// 딜러, 유저에게 2장씩 배분
}왜 이런식으로 제안드리는지 한번 이유를 생각해보시고 댓글로 남겨주세요!!
There was a problem hiding this comment.
BlackJack이라는 객체를 만들어서 게임 시작의 경계를 분명히 하려는 의도가 아니신가 싶습니다.
BlackJack 객체보다 CardDistributor라는 객체는 그 의미에서 게임 시작의 경계가 불분명한 것 같습니다.
궁금한 점이 있습니다!
그러면 CardDistributor 객체는 없애는 게 좋을까요??
그리고 플레이어 각자에게 한 장씩 더 받을 지 여부에 대해 묻고 만약 y라고 답하면 카드 한 장을 더 배분하는 로직은 controller에서 구현되는게 맞을까요 아니면 BlackJack 객체에 Player와 응답(y 또는 n)을 매개변수로 받아서 이를 처리하는 메소드를 하나 구현하는 게 좋을까요??
답변 주시면 감사드리겠습니다.
There was a problem hiding this comment.
BlackJack이라는 객체를 만들어서 게임 시작의 경계를 분명히 하려는 의도가 아니신가 싶습니다.
시작의 경계가 BlackJack 이라면 만족해야할 필수조건을 의미한다면 맞습니다!!
그러면 CardDistributor 객체는 없애는 게 좋을까요??
카드를 나눠주는 것을 객체로 나눠서 만드는 것은 해당 책임을 지는 객체에게 위임하겠다는 의미입니다!!
즉, 카드 분배 로직을 추상화하는 것이고 Distributor를 아는 객체는 카드 분배 방식의 실직적 구현에서 관심을 끈다는 것입니다.
추상화는 항상 답이 되진 못합니다. 추상적으로 알기 때문에 약한 의존으로 변화에는 유연하지만 그 코드 자체로 이해하기가 힘들기 때문입니다.
그러면 지금 상황에서 비교할 만한 것이
장점
- BlackJack이 카드를 나눠준다 -> 코드가 이해하기 쉽다. 이해를 높히지만 변화가 발생할 경우 개선해야한다.
- 카드 분배 로직을 CardDistributor에게 위임한다. -> 초반 카드 분배, 카드 분배 룰 같은 것은 변경되더라도 수용가능한 구조이다.
단점
- BlackJack이 카드를 나눠준다 -> 카드 분배 로직에 변화에 따른 코드 변경 필요
- CardDistributor 로 추상화한다 -> 잘못된 확장 고려일 수 있음. 나중에 해당 방향으로 확장이 안될 수도 있고 CardDistributor 하나 추상화하는 것으로 변경이 안 끝날 수 있다. 로직 이해가 어려워진다.
지금 단계에서는 BlackJack에서 CardDistributor를 분리하여 추상화하는 것은 오버프로그래밍이지 않을까라는 생각입니다 ㅎㅎ
| if (score > Constant.TARGET_SCORE) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
TARGET_SCORE를 Checker에서 관리하는 것이 어떨까요??
| if (participant instanceof Player) { | ||
| askForAdditionCard(participant); | ||
| } |
There was a problem hiding this comment.
instanceof 는 객체지향 설계에서 캡슐화를 깨뜨립니다. 참고 글
만약 객체의 상세 타입을 instanceof로 처리해야 한다면
- 해당 객체의 참조를 처음부터 상세 타입으로 가져야하진 않을까?
- instanceof보다는 isDealer와 같은 메서드로 묻는 방식은 어떨까?
로 고민해보시면 좋을 것 같습니다!!
src/main/java/domain/BlackJack.java
Outdated
| public boolean isDealerAboveThreshold(Dealer dealer) { | ||
| int score = ScoreCalculator.calculateScore(dealer); | ||
| if (score <= DEALER_THRESHOLD) { | ||
| distributeCard(dealer); | ||
| return true; | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
메서드는 딜러가 임계점을 넘겼냐고 묻고 있는데
점수가 더 낮을 경우 카드를 주고 참을 반환하고 아닐 경우 false를 반환하는 군요!!
다음 메서드를 두 가지 측면에서 수정해보시면 좋을 것 같습니다,
- 하는 행위와 메서드 네이밍이 다른 것
- 메서드의 부수 효과 제거
| public class ScoreCalculator { | ||
|
|
There was a problem hiding this comment.
해당 클래스는 유틸리티 클래스이군요!!
객체지향적 설계에서는 유틸리티 클래스는 지양하는 것이 좋습니다! 참고 글
점수 계산이라는 행위를 분배할만한 객체를 찾아서 할당해보시면 좋을 것 같습니다!
There was a problem hiding this comment.
CardList에서 점수 계산을 하는 걸로 코드를 개선해보겠습니다 !
| public class CardList { | ||
|
|
There was a problem hiding this comment.
해당 클래스가 담당하는 역할이 없군요!!
CardList가 담당할 수 있는 역할이 없을까요??
There was a problem hiding this comment.
점수 계산을 카드 리스트에서 하면 될 것 같습니다 !
| } | ||
|
|
||
|
|
||
| } |
There was a problem hiding this comment.
요기 마지막 줄 개행 부탁드립니다!!
만약 IntelliJ 사용하시면 다음 글을 참고해주세요!
| public class NameChecker { | ||
|
|
There was a problem hiding this comment.
NameCheck를 두는 것보단 Name에 해당 검증을 두는 것이 어떨까요??
그랬을 경우 차이는 무엇일까요??
There was a problem hiding this comment.
['딜러' 라는 이름 사용 불가] 검증과 [이름 빈칸 불가] 검증은 Name에 들어가는 게 적절할 것 같습니다.
그런데 [이름 중복]에 대한 검증은 Name에 들어가면 사용할 방법이 없습니다.
그래서 NameChecker를 만들었던 건데 지금 생각해보니 이 객체의 역할이 2개 이상으로 만들어 진 것 같습니다.
차라리 PlayerList라는 객체를 만들고 거기서 [이름 중복]에 대한 검증을 하는 것이 좋을 것 같습니다!
| public void gameStart() { | ||
| List<Participant> participants = new ArrayList<Participant>(); | ||
|
|
There was a problem hiding this comment.
gameStart라는 메서드가 길~어 졌는데
적절히 메서드를 나누어준다면
- 잘못된 데이터 접근을 막을 수 있다.
- 메서드 명을 통해 의미있는 행위의 경계를 잡을 수 있다.
의 장점을 취할 수 있습니다!!
| public class DealerWinChecker { | ||
|
|
||
| private int dealerScore; | ||
| private int winNum; | ||
| private int loseNum; | ||
| private int pushNum; |
There was a problem hiding this comment.
딜러가 이겼는지 판단을 하는 것을 객체로 분리해줬군요!!
결과 계산을 딜러에게 위임하는 방식과 한번 비교해보시고 결과를 공유해주세요!!
| public class Constant { | ||
|
|
||
| public static final int TARGET_SCORE = 21; | ||
| public static final String YES = "y"; | ||
| public static final String NO = "n"; | ||
|
|
||
| } |
There was a problem hiding this comment.
해당 값들은 하나의 클래스에서 밖에 사용되지 않으니 사용되는 클래스에서 관리해주는 것이 어떨까요??
전역적으로 여러번 사용되는 상수만 이렇게 빼는 방향으로 가시면 좋을 것 같습니다!!
안녕하세요, 1단계 미션 제출합니다.
아직 최종 승패 처리 로직과 입력 기능, 출력 기능이 미완성인 상태입니다.
일단 pr을 한 후에 지속적으로 코드를 업데이트 하겠습니다.
감사합니다.