-
Notifications
You must be signed in to change notification settings - Fork 467
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[2단계 - 블랙잭 베팅] 머랭 미션 제출합니다. #882
base: cookie-meringue
Are you sure you want to change the base?
[2단계 - 블랙잭 베팅] 머랭 미션 제출합니다. #882
Conversation
- 초기화 시 랜덤으로 섞어 주입.
- 딜러는 수익의 개념을 가지지 않는 것이 더 적절하다고 판단했기 때문.
step1 을 pull 받지 않아 충돌이 생겼습니다! bfab425 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 머랭 👋
구현 잘 해주셨습니다.
몇 가지 코멘트 남겼으니 확인해주세요.
|
||
private final JudgementStrategy judgementStrategy; | ||
private final Hand hand; | ||
private final CardDeck cardDeck; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
딜러가 덱을 갖고 있게 변경한 이유가 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이전 리뷰에서 영감을 받았습니다.
딜러가 카드 덱을 다루며 플레어들에게 카드를 나눠 주고, 각 플레이어들은 딜러와 겨루도록 설계했습니다.
|
||
@Override | ||
public boolean isDraw(final Dealer dealer, final Player player) { | ||
if (player.isBust() || dealer.isBust() || player.isBlackjack() || dealer.isBlackjack()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
플레이어와 딜러가 모두 블랙잭이라면 비겨야합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
네 알겠습니다! 제가 요구사항을 잘 분석하지 못한 것 같네요 수정하겠습니다
@Override | ||
public boolean isDealerWin(final Dealer dealer, final Player player) { | ||
if (player.isBlackjack() || dealer.isBust()) { | ||
return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이렇게 승/패/무가 엮여 있는 경우에 각 로직을 따로 짜면 버그가 발생할 가능성이 높습니다. 지금도 계속 요구사항을 만족하지 못하는 현상이 발생하고 있구요.
로직도 각각 다양한 케이스를 모두 처리하려면 복잡해지고 경우의 수도 너무 많아지고요.
이렇게 한 결과가 다른 결과에 영향을 미치는 경우는 그냥 메서드가 좀 길어지더라도 하나로 처리하면 좋을 것 같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정해보겠습니다
int bettingMoney = player.getBettingMoney(); | ||
if (player.isBlackjack()) { | ||
return Math.round(bettingMoney * BLACKJACK_REWARD_RATE); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Player에게 get을 해서 로직을 처리하고 있네요.
메세지를 던져보는 건 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그렇게 되면 플레이어가 너무 많은 것을 알게 되지 않을까요? 어떻게 생각하시나요?
private static final int DRAWABLE_POINT = 21; | ||
|
||
private final String name; | ||
private final Betting betting; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
플레이어가 베팅을 가져도 되는가? 에 대해 어떻게 생각하시는 지 궁금합니다.
가져야할 이유와 가져야하지 말아야할 근거들을 설명해주시면 답변 달기 편할 것 같네요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
제가 설명이 부족했네요 ㅎㅎ 죄송합니다.
플레이어의 베팅을 관리하는 방법은 크게 두 가지가 있다고 생각했습니다.
1. 외부에서 Map<Player, Betting> 으로 관리
플레이어는 자신이 베팅해야 한다는 사실을 몰라도 됩니다. 외부에서 관리하기 때문입니다.
결국, 플레이어가 책임져야 할 것이 줄어듭니다.
2. 플레이어의 필드로 Betting 을 관리
플레이어는 자신이 베팅을 한다는 것과, 돈을 잃거나 얻을 수 있다는 사실을 인지해야 하고, 외부에 메서드를 제공해야 합니다.
결국, 플레이어가 책임져야 할 것이 늘어납니다.
제가 생각한 두 가지 방법 중, 저는 2번을 선택했습니다.
우선, 게임의 특성 상 플레이어가 자신의 베팅을 관리하는 것이 개념적으로 당연하다고 느껴졌습니다.
플레이어가 책임지는 것이 늘어나긴 하지만, 플레이어가 책임지는 것이 적절하다는 판단을 한 것입니다.
하지만 제출하기 전, 플레이어가 자신의 돈을 얻고 잃는다는 메서드를 외부에 제공하는 것이 올바른 책임인가? 에 대한 의문이 들었습니다.
계속 생각한 결과, 플레이어가 하는 역할이 너무 많다는 결론이 나왔습니다.
플레이어가 베팅을 관리하는 것이 플레이어의 책임이 될 수 있는가? 에 대한 범블비의 생각이 궁금합니다!
import blackjack.model.blackjack_player.dealer.Dealer; | ||
import blackjack.model.blackjack_player.player.Player; | ||
|
||
public interface JudgementStrategy { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위 코멘트들 반영하면서 이걸 인터페이스로 분리해야할지도 고민해보면 좋을 것 같네요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Judgement 를 인터페이스로 분리해 전략 패턴을 사용한 이유는, 딜러의 책임이 너무 많다고 느꼈기 때문입니다.
단순 승,패, 보상 판단은 Judgement 가 해도 되지 않을까? 하는 생각에서 Judgement 를 만들게 되었습니다.
Judgement 클래스를 만들까? 하는 생각도 들었지만, 위의 제 논리대로라면, Judgement 는 필드 없이 함수만 제공하는 클래스가 될 것 같았습니다.
return; | ||
} | ||
if (judgementStrategy.isDealerWin(this, player)) { | ||
player.loseMoney(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
가변 객체와 불변 객체를 사용했을 때의 차이점과 장단점도 비교해보면 좋을 것 같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
betting.lose()
를 호출하면 새로운 Betting
을 반환하도록 하는 방식을 사용해 불변 객체를 설계하려고 했습니다.
그러나, 위 방식대로 하면, Betting
의 balance
는 final 이 되겠지만, Player
의 betting
은 final
이 될 수 없었습니다.
또한, player
는 loseMoney()
메서드로만 자신의 betting
의 상태를 변경할 수 있습니다.
이 구조 상, betting
의 balance
를 불변으로 만들어 얻을 수 있는 이점이 와닿지 않았습니다!
|
||
public final class Player { | ||
|
||
private static final int DRAWABLE_POINT = 21; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hand에도 21이 있고, 여기도 21이 있네요. 어떤 식으로 중복을 제거할 수 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
중복을 제거하면 안된다고 생각해 따로 만들었습니다.
블랙잭 포인트와 무관하게 플레이어는 자신이 언제 카드를 더 뽑을 수 있는 지 판별할 수 있어야 한다고 생각했습니다.
지금은 블랙잭 포인트와 같은 21 이지만, 딜러가 17을 가지고 있는 것 처럼, 플레이어의 DRAWABLE_POINT 도 언제든 변할 수 있다고 생각했습니다.
하지만, 한편으로는 너무 확장성을 고려한 것이 아닌가? 하는 생각이 듭니다. 우선 블랙잭 포인트와 같은 값이기 때문에 참조해서 사용하면 좋을 것 같긴 하네요.
어떻게 생각하시나요??
allBlackJackPlayers.add(dealer); | ||
blackJackGame.dealInitialCards(allBlackJackPlayers); | ||
private void drawMorePlayerCards(final Dealer dealer, final Player player) { | ||
while (player.canReceiveMoreCard() && inputView.readUserDrawMoreCard(player)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
지금은 Predicate 만으로 판단이 가능하지만, 추후에 매개변수가 계속해서 늘어나게 되고, 요구사항이 변하면 확장에 어려움이 있을 것이라 생각했습니다.
이에 대한 생각이 궁금합니다!
잘 시도해보셨네요.
"플레이어가 카드를 받을 수 있는 상태이고, 더 받길 원한다면" 이라는 요구사항을 판단하기 위해 더 많은 정보가 필요해지는 경우를 말씀하시는 거겠죠?
잘 생각해주신 것 같아요. 요구사항이 추가될 수록 점점 시그니처가 알아보기 어려운 형태로 변할 거에요. (물론 Predicate 정도만 해도 충분히 읽기 어렵다고 생각하긴 합니다)
대신 함수를 넘기면 위 요구사항을 컨트롤러가 아닌 도메인에 정의할 수 있다는 장점은 있겠죠.
구현하는 방법은 여러가지고 각각 방법은 트레이드 오프를 갖고 있습니다.
우테코 미션을 진행하면서는 어떤 방법이 맞다를 결정하기 보다는 최대한 열린 마음으로 다양한 방법을 시도해보고, 각 방법들이 갖는 특징들을 정리하는 습관을 들여보면 배우는 게 많을 것 같아요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
감사합니다! 덕분에 새로운 방법을 시도해볼 수 있었습니다
체크 리스트
test
를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?
1~5점 중에서 선택해주세요.
선택한 점수의 이유를 적어주세요.
각 객체의 책임을 최대한 분리하기 위해 노력했습니다.
기존의 추상 클래스 상속 구조에서 조합 구조로 변경해 딜러와 플레이어가 추상 클래스의 default 로직에 종속되지 않도록 구현해보았습니다.
또한, Service 역할을 하는 Game 을 제거하고, 딜러와 플레이어가 메시지를 주고받도록 변경했습니다.
디미터 법칙을 지키기 위해 플레이어만 사용하는 Betting 클래스를 만들고, 플레이어 외부로 노출하지 않았습니다.
어떤 부분에 집중하여 리뷰해야 할까요?
1. Betting 클래스
플레이어의 베팅을 관리하기 위해
Betting
클래스를 만들어 플레이어의 필드로 관리하도록 변경했습니다.플레이어가 베팅을 가져도 되는가? 에 대해 어떻게 생각하시는 지 궁금합니다.
2. 함수형 인터페이스를 통한 while 문 제거
이전 리뷰 대화에서 함수형 인터페이스를 통해 계속 카드를 뽑는 방법에 대해 논의한 적이 있습니다.
말씀하신 대로,
Player
를 받아Boolean
을 반환하는Predicate
의 역할을 하는 함수형 인터페이스를 사용해서 구현하려 시도했습니다.그러나, 확장성이 좋지 않을 것 같다는 생각이 들었습니다.
지금은
Predicate<Player>
만으로 판단이 가능하지만, 추후에 매개변수가 계속해서 늘어나게 되고, 요구사항이 변하면 확장에 어려움이 있을 것이라 생각했습니다.이에 대한 생각이 궁금합니다!
3. MVC 에서의 Controller
이번에 말이 많이 나왔던 주제입니다.
크루들이 Controller 가 왜 필요한지에 대한 의문을 많이 던졌습니다. (View -Model 연결을 제외하고)
Controller 가 필요한 이유에 대한 제 생각은 다음과 같습니다.