[2단계 - 블랙잭 베팅] 모루(최동준) 미션 제출합니다. #879
Conversation
There was a problem hiding this comment.
step1에서 달아주신 코멘트에 대한 답변을 들고 왔습니다!
오 LinkedHashMap을 사용해주셨군요!
HashMap을 사용하려 하니, 순서를 보장하지 않아, 처음에 플레이어 입장에서 입력한 순서대로 결과가 나오지 않더라구요! 그래서 LinkedHashMap을 사용했습니다!
이걸 MAP으로 view에서 관리하도록 해주신 이유가 궁금해요!
Enum에서 View를 위한 부분을 최대한 빼고 싶었습니다.
그렇게 하려면, 각 Denomination에 대한 출력값을 따로 저장해주어야 하는데, 이전 처럼 if문에서 하는 것보다는 View에 Map으로 관리하는게 낫다고 생각했습니다.
이전에 if문에서는 하나하나의 If문을 거쳐서 그 값을 찾았어야 했는데, Map으로 관리하면 .get()으로 할 수 있기 때문에, 더욱 효율적일 것이라고 생각했습니다. 새로운 값을 추가할 때에도, If문을 추가하는 것보다 Map에 추가하는 것이 더욱 편리하다고 생각했습니다!
|
|
||
| private boolean drawAdditionalCard(final Participant participant) { | ||
| participant.addCard(deck.draw()); | ||
| if (participant.getClass().equals(Player.class)) { |
There was a problem hiding this comment.
상속을 사용했는데, 이렇게 getClass()로 해당 클래스인지 물어보는 것에 문제가 있을까요?
요구 사항에 의해 Player와 Dealer의 출력 내용이 달라서 작성한 코드입니다.
There was a problem hiding this comment.
추상 메서드로 isPlayer()를 만들어 각각의 구현체에서 구현하면 된다는 것을 깨달았습니다..
| public int getHandTotal() { | ||
| return hand.getTotal(); | ||
| } | ||
| public boolean isBlackJack() { return hand.isBlackJack(); } |
There was a problem hiding this comment.
Participant가 추상 클래스이지만, 추상 메서드가 하나도 없는데, 추상 클래스를 메서드 재사용만을 위해 사용하는 것 같아 제대로 사용하지 못하는 것 같습니다.
우선 추상 메서드가 없는 추상 클래스가 잘못된 설계는 아니라고 생각해요. 다만 코드의 의도와 역할을 좀 더 명확히 하기 위해 아래와 같은 질문을 남겨볼게요ㅎㅎ
- Participant가 꼭 추상 클래스여야하는 이유가 무엇인가?
- Dealer와 Player가 반드시 구현해야할 동작이 있나?
- shouldDrawCard와 같이 서로 다른 정책적인 부분을 추상메서드화할 순 없을까?
There was a problem hiding this comment.
현재의 블랙잭에서는 딜러와 플레이어의 동작이 크게 다르지 않습니다. 차이점은
- 딜러는 이름이 없다
- 추가 카드를 받을 때 결정하는 방식
그래서, 기본적으로 블랙잭 게임하는 큰 동작은 Participant에서 구현하여 Dealer와 Player에서 상속 받는 것이 낫다고 생각했습니다. 추상 클래스를 이용한 상속을 통해 중복 코드를 제거하고 공통되는 블랙잭 게임에서의 동작을 쉽게 관리할 수 있을 것이라 생각했습니다.만약 인터페이스를 사용한다면, 각각의 클래스에서 어차피 같은 코드를 작성해야 해서 중복 코드가 생긴다고 생각했습니다.
또한, 상속을 사용하면, 부모 클래스에서 수정으로 인한 자식 클래스에게 영향이 크게 미칠 수 있고, 캡슐화가 깨질 수 있지만, Participant는 Player와 Dealer의 공통된 동작을 정의하는 것이므로, 블랙잭의 룰이 크게 변경되는 것이 아니라면, 수정할 일이 없어 안정적일 것이라고 생각했습니다.
추상 메서드를 통해 구현해야하는 동작은 Dealer와 Player가 비슷하지만, 다른 동작이 있을 것 같은데, shouldDrawCard와 처음에 2장씩 받은 카드를 Dealer는 1장, Player는 2장을 공개하는 부분이 있을 것 같습니다.
| if (participant.isMaxScore()) return; | ||
|
|
||
| boolean isAlive = participant.resolveBust(); | ||
| while (isAlive && shouldDrawMore.get()) { |
There was a problem hiding this comment.
shouldDrawMore 관련해서 남겨주신 코멘트를 바탕으로 생각해봤는데요, 다양한 패턴들을 도입해볼 수도 있겠지만 간단하게는
public abstract class Participant {
// 기존 코드...
// 템플릿 메서드
public final void playTurn() {
if (isMaxScore()) return;
boolean isAlive = resolveBust();
while (isAlive && shouldDrawMore()) {
addCard(drawCard());
isAlive = resolveBust();
}
////
}
// 훅 메서드 - 하위 클래스에서 구현
protected abstract boolean shouldDrawMore();
// 카드를 뽑는 메서드는 외부에서 주입받거나 상위 클래스에서 구현
protected abstract Card drawCard();
}뭔가 이런식으로 shouldDrawMore만 빼볼수도 있을 것 같다는 생각을 했어요! 전략패턴 같은것도 쓸 수 있을 것 같은데 불필요하게 복잡해지는 느낌이긴 해서.. 혹시 확인해보시고 적합한 걸 적용해보셔도 좋을 것 같네요!
There was a problem hiding this comment.
찾아본 결과로, 전략 패턴은 인터페이스를 사용해야 하므로, 지금 당장은 shouldDrawMore만을 위해서 패턴이 필요한 것이니 불필요하게 복잡해지는 느낌이 저도 듭니다. 추가적인 Dealer와 Player의 차이점이 생기는 것이 아니라면, 템플릿 메서드 패턴이 적합한 것 같습니다.
There was a problem hiding this comment.
템플릿 메서드 패턴을 학습한 후에, 적용해보려 했으나 하지 못했습니다,,
조앤이 말씀하신대로, 템플릿을 playTurn() 메서드로 하고, shouldDrawMore을 추상 메서드로 만들어 하려 하였습니다.
하지만, playTurn() 메서드에서 플레이어는 카드 한장을 받을 때마다 출력을 해야 합니다. 그러면 도메인이 view를 직접 사용하게 되는 상황에 되어버립니다. playTurn()을 도메인으로 옮길 수 있는 방법을 생각하지 못해 적용하지 못했습니다.
하지만, shouldDrawMore()을 긴 고민 끝에 추상 메서드로 만들어 딜러와 플레이어에서 구현했습니다.
문제가 되었던 플레이어가 inputView를 통해 입력받고 결정하는 것은 while문에서 처리하도록 하였습니다.
| import java.util.Map; | ||
| import java.util.Map.Entry; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| import static domain.participant.Dealer.THRESHOLD; | ||
|
|
||
| public class OutputView { |
There was a problem hiding this comment.
로직이 조금 헷갈리는데 버스트가 되어 턴을 종료합니다가 두번 나오는 것 같아요?
There was a problem hiding this comment.
b가 카드를 받으면서 버스트 출력이 나오고, 딜러가 카드를 받으면서 버스트 출력이 나온 것 같습니다.
딜러의 경우에는 출력이 되지 않도록 수정하겠습니다.
| import java.util.Map; | ||
| import java.util.function.Supplier; | ||
|
|
||
| public class BlackJackController { |
There was a problem hiding this comment.
BlackJack 도메인이 사라지고 Controller만 남겨주신 이유가 궁금해요!
Controller는 단순히 view와 domain을 연결하는 역할만 하고, 지금 구현된 대부분의 로직을 BlackJack으로 옮겨보는게 좋을 것 같아서요.
실제 게임을 생각해보더라도 BlackJack 안에 딜러와 카드덱이 있는게 직관적인 것 같은데, 어떻게 생각해요?
There was a problem hiding this comment.
원래 BlackJack에 흐름이 있어서 절차지향적이라고 생각했습니다. Domain은 객체지향적이어야 하지만, Controller는 절차지향적이어도 된다고 생각해서 일단 이름을 변경했습니다. BlackJack 객체와 Controller를 따로 가지는 것은 저도 필요성을 느껴서 수정해보겠습니다.
There was a problem hiding this comment.
분리하긴 하였으나, 아직 깔끔하지 않은 것 같습니다. Controller에 View를 사용하지 않는 부분을 최대한 찾아 분리하였습니다. BlackJack 도메인이 흐름을 갖지 않도록 하면서, BlackJack 객체가 어디까지 담당해도 될지 헷갈렸습니다. 그래서 분리한게 많지 않습니다..
- BlackJack 객체가 어디까지 담당해도 될지 헷갈린 부분
- 참가자가 더 드로우해도 될지를 참가자가 BlackJack을 통해서 컨트롤러에 전달해야 할지, 직접 전달해도 될지
| private static final double BLACKJACK_BONUS = 1.5; | ||
| private static final int WIN_BONUS = 2; | ||
|
|
||
| private final double amount; |
There was a problem hiding this comment.
double이 금액 연산에 정확하다고 잘못 알고 있었습니다. '원'은 소수점이 없고, 1000원 단위로 입력하도록 했기 때문에 그냥 int로 해도 될 것 같습니다. 감사합니다.
| import java.util.LinkedHashMap; | ||
| import java.util.Map; | ||
|
|
||
| public class BettingPool { |
| class BetMoneyTest { | ||
|
|
||
| @Test | ||
| @DisplayName("금액을 받아 BetMoney를 생성할 수 있다.") |
| } | ||
|
|
||
| @Test | ||
| @DisplayName("Hand에 카드가 3장이면서, 총 합이 21이면 블랙잭이 아니다.") |
| } | ||
|
|
||
| @Test | ||
| @DisplayName("딜러와 플레이어의 총합이 21로 같은데, 딜러만 블랙잭이면 플레이어 패배") |
| dealer.addCard(new Card(Denomination.TEN, Suit.CLUB)); | ||
| dealer.addCard(new Card(Denomination.ACE, Suit.DIAMOND)); |
There was a problem hiding this comment.
아 이런것들이 테스트에서 계속 반복되는데, 어떤 카드 팩터리가 있어서 원하는 대로 조건을 넣으면 해당 카드들을 리스트로 뽑아주는게 있어도 편하겠네요
체크 리스트
test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?
1~5점 중에서 선택해주세요.
선택한 점수의 이유를 적어주세요.
getter와 Card에서 인스턴스 변수 3개를 쓰게 되어서 아쉬움이 남습니다.
어떤 부분에 집중하여 리뷰해야 할까요?
Participant가 추상 클래스이지만, 추상 메서드가 하나도 없는데, 추상 클래스를 메서드 재사용만을 위해 사용하는 것 같아 제대로 사용하지 못하는 것 같습니다.
컨트롤러의 playTurn() 메서드의 인자인 shouldDrawMore를 Participant에서 추상 메서드로 선언하여 Player와 Dealer에서 오버라이딩하려고 하였습니다. 그런데, Player는 추가로 드로우 할지 결정하는 것에서 InputView에서 받은 입력이 필요하지만, Dealer는 본인이 가진 상태만으로 판단이 가능해 인자가 필요하지 않습니다. 그래서 추상 메서드로 만들지 못했습니다. 이것을 추상 메서드로 만들 수 있는 방법이 있을까요?
그래서, 현재는 Player와 Dealer의 일관성 유지를 위해, 도메인에서 처리하는 것이 아닌, Controller에서 처리하도록 하였습니다.
플레이어들의 수익을 BettingPool에서 모두 관리하도록 하였습니다.
Player 각자가 베팅액을 관리하고, 각자 수익을 내는 것보다, BettingPool에 플레이어가 베팅을 하고, 수익을 플레이어에게 알려주는 것이 더 실제와 비슷하고, 플레이어 수익을 통해, 딜러의 수익을 계산하기에 더 편하다고 생각했습니다.