Skip to content
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단계 - 블랙잭 베팅] 포포(이민우) 미션 제출합니다. #880

Open
wants to merge 8 commits into
base: minuring
Choose a base branch
from

Conversation

Minuring
Copy link

@Minuring Minuring commented Mar 14, 2025

체크 리스트

  • 미션의 필수 요구사항을 모두 구현했나요?
  • Gradle test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?
  • 애플리케이션이 정상적으로 실행되나요?
  • 프롤로그에 셀프 체크를 작성했나요?

객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?

1~5점 중에서 선택해주세요.

  • 1 (전혀 충족하지 못함)
  • 2
  • 3 (보통)
  • 4
  • 5 (완벽하게 충족)

선택한 점수의 이유를 적어주세요.

switch문을 사용했습니다.
메서드 10줄을 지키지 못한 지점이 있습니다.
들여쓰기가 2단계인 지점이 있습니다.

이전 리뷰에 대한 답변

이전 리뷰에서 하지 못한 답변입니다.
조급함에 정신없이 하다보니 정성껏 질문해 주셨는데 답장을 하지 않았어요.. 죄송합니다 😭

DTO가 필요하다고 생각한 이유

현재 Dealer, Player는 `takeCards()`라는 메서드가 제공되고 이로 인해 `가변 객체`입니다.
그래서 뷰에서 이들을 직접 참조하지 못하는 게 가장 좋다고 생각했습니다.
그래서 컨트롤러에서 출력을 위한 DTO를 만들어 참여자의 이름, 카드와 같은 불변 데이터만 전달하려고 했습니다.

+ 추가적으로 데이터 전달이라는 목적을 생각해 record로 선언하였습니다!
  • 참가자를 불변 객체로 설계하고 DTO를 사용하지 않는 것은 어떻게 생각하시나요?
    (참가자를 가변 객체로 설계하고 출력 시 DTO를 사용하는 것이 훨씬 편할 것 같다고 생각해서 지금은 DTO를 사용했습니다.)

어떤 부분에 집중하여 리뷰해야 할까요?

1. Gamblers 클래스의 역할의 크기가 적절한 지 궁금합니다.

크게 카드를 나눠주는 역할과 결과를 집계하는 역할을 하고있습니다.
배팅에 대한 집계가 2단계에 추가되면서 플레이어의 배팅금 정보까지 저장하고 있습니다.
현재 Gamblers가 너무 많은 일을 하는 것 처럼 보이는 지 궁금합니다!

2. 도메인을 생성하는 책임은 누가?

현재 코드에서 Player를 들고있는 Gamblers가 있을 때 Player를 생성하는 위치가 ControllerGamblers 중 어디가 적절할까요?

일급 컬렉션인 도메인이나 Manager같은 상위 도메인에서 작은 단위의 도메인 객체들을 사용하는 상황이 항상 발생합니다.
현재 코드에서 예를들면 Gamblers에서 Player를 사용하는 상황을 말합니다. (출석부출석일시를 사용하는 그런 상황)

  1. 단순하고 불변하거나 값처럼 사용되면 상관없지만 도메인이 복잡하거나 가변적인 객체이면, 컨트롤러에서 도메인을 제어할 가능성이 있을 것이라고 생각합니다.
// Controller
String name = inputView.readName();
Player player = new Player(name); // takeCards()라는 수정메서드가 열린 Player에 대한 참조를 가짐!
Gamblers gamblers = new Gamblers(List.of(player));
  1. 반면 입력값을 상위 도메인에 넘겨주어, 생성하려는 도메인을 상위에서 생성하도록 위임하면 사용자 입력을 가공하는 작업을 도메인이 하는 것이 마음에 들지 않았습니다.
    무엇보다 플레이어의 생성 과정이 바뀌면 Gamblers도 영향을 받게 됩니다.
// Controller
String name = inputView.readName();
// Player에 대한 참조가 없으니 마음대로 제어할 수 없음. 하지만 입력 가공 (Player 생성)을 Gamblers 도메인이 해야함
Gamblers gamblers = Gamblers.createByPlayerNames(List.of(name));

제로의 도메인을 생성하는 위치에 대한 기준이 있는 지 궁금합니다!!

<질문 요약>

  1. 참가자를 불변 객체로 설계하고 DTO를 사용하지 않는 것은 어떻게 생각하시나요?
  2. 현재 Gamblers가 너무 많은 일을 하는 것 처럼 보이나요?
  3. 도메인을 생성하는 위치에 대한 기준이 있으신 지 궁금합니다!

Copy link

@asebn1 asebn1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

포포 안녕하세요
2단계도 빠르게 구현해주셨네요!
질문 주신 내용에 대한 답과 몇 가지 코멘트 남겼습니다
확인해주세요 :)

Comment on lines +21 to +38
### 승패
- [x] 처음 두 장의 카드 합이 21이면 블랙잭이다.
- [x] 딜러가 블랙잭이 되었을 때
- [x] 블랙잭인 플레이어는 Push, 무승부가 된다.
- [x] 블랙잭이 아닌 플레이어는 즉시 패배한다.
- [x] 플레이어와 딜러의 카드 합이 같으면 무승부이다.
- [x] 플레이어가 버스트되면 딜러의 버스트 여부와 상관없이 패배한다.
- [x] 플레이어가 버스트되지 않고 딜러가 버스트되면 승리한다.

### 배팅
- [x] 플레이어는 원하는 금액을 배팅한다.
- [x] 배팅금은 최소 1000원에서 최대 100만원이다.
- [x] 1000원 단위로만 배팅할 수 있다.
- [x] 플레이어가 블랙잭이 되었을 때
- [x] 딜러도 블랙잭(Push)이면 배팅금을 딜러에게 돌려받는다.
- [x] 딜러가 블랙잭이 아니면 배팅금의 1.5배를 딜러에게 받는다.
- [x] 플레이어가 승리하면 배팅 금액을 받는다.
- [x] 플레이어가 패배하면 배팅 금액을 잃는다.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요구사항을 상세히 작성해 주셨네요 👏

Comment on lines +14 to +15
final CardPack cardPack = new CardPack(Card.allCards());
cardPack.shuffle();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
final CardPack cardPack = new CardPack(Card.allCards());
cardPack.shuffle();
final CardPack cardPack = CardPack.ofShuffled();

셔플된 카드를 바로 받으면 어떨까요?

@@ -27,48 +29,48 @@ public BlackjackController(InputView inputView, OutputView outputView) {
this.outputView = outputView;
}

public void play() {
public void play(CardPack cardPack) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

play가 모든 흐름을 직접 제어하고있네요.
메서드를 분리하거나 비즈니스 로직을 위임해볼까요?

gamblers.distributeSetUpCards(cardPack);
outputView.printSetUpCardDeck(dealer, players);
List<GamblerDto> gamblerDtos = toKeyList(players).stream().map(GamblerDto::from).toList();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

객체지향 생활체조 원칙을 참고하여 개선해봅시다 :)

Comment on lines +19 to +22
public boolean isBlackJack() {
return cards.size() == BLACK_JACK_CARD_COUNT
&& calculateScore() == Winning.BLACK_JACK;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Winning.BLACK_JACKCardHand에서만 사용중인데 CardHand에 상수를 옮겨볼까요?

Comment on lines +21 to +28
@Override
public final boolean equals(Object o) {
if (!(o instanceof GamblingMoney betting)) {
return false;
}

return amount == betting.amount;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instanceof를 사용하고 있네요
개선해볼까요?

Comment on lines +35 to +38
@Override
public String toString() {
return amount + "";
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이렇게 toString을 적용한 이유가 궁금합니다 ㅎㅎ

public enum Winning {
WIN("승"),
DRAW("무"),
LOSE("패"),
;
LOSE("패");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
LOSE("패");
LOSE("패")
;

여기도 형식을 맞추어줍시다!

Comment on lines +64 to +81
@Test
void 딜러가_블랙잭인지_알_수_있다() {
//given
dealer.takeCards(Card.SPADE_A, Card.HEART_10);

//when
boolean blackJack = dealer.isBlackJack();

//then
assertThat(blackJack).isTrue();
}

@Test
void 점수가_21점을_넘으면_버스트이다() {
dealer.takeCards(Card.SPADE_10, Card.HEART_10, Card.SPADE_2);
assertThat(dealer.isBust()).isTrue();
}

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

꼼꼼한 테스트케이스 💯

import java.util.Map;
import java.util.Map.Entry;
import java.util.function.Function;

public class Gamblers {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gamblers는 participant가 아니라 game에 있는 이유가 궁금합니다 :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Gamblers 클래스의 역할의 크기가 적절한 지 궁금합니다.
    크게 카드를 나눠주는 역할과 결과를 집계하는 역할을 하고있습니다.
    배팅에 대한 집계가 2단계에 추가되면서 플레이어의 배팅금 정보까지 저장하고 있습니다.
    현재 Gamblers가 너무 많은 일을 하는 것 처럼 보이는 지 궁금합니다!

지금 작은 미션이어서 보기 어렵다고 느껴지진 않았는데요, 말씀처럼 과도한 역할 수행으로 볼 수 있을 것 같아요.
카드배분/결과집계 이 두 가지 역할로 분리해서 클래스 분리를 고민해볼 것 같아요 :)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 도메인을 생성하는 책임은 누가?
    현재 코드에서 Player를 들고있는 Gamblers가 있을 때 Player를 생성하는 위치가 Controller와 Gamblers 중 어디가 적절할까요?

일반적으로 비즈니스 로직을 담당하는 서비스 계층을 우선으로 고민할 것 같고, 지금 과정에서는 독립적으로 생성될 필요가 있는지 여부를 체크할 것 같아요. 독립적으로 생성될 필요가 있다면 컨트롤러에서 생성 후 주입하고 강하게 결합됐다면 Gamblers에서 진행할 것 같아요. 이번 케이스에선 Player가 가변 객체이고 핵심 도메인 객체여서 Controller에서 생성 후 주입하는 방식을 선택할 것 같아요.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 참가자를 불변 객체로 설계하고 DTO를 사용하지 않는 것은 어떻게 생각하시나요?

참가자를 불변 객체로 만들기는 어려울 것 같다는 생각이 들어요. 입/출력용 DTO를 사용하는 건 유지보수에 유리하다고 생각합니다 :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants