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

[1단계 - 블랙잭 게임 실행] 포포(이민우) 미션 제출합니다. #816

Merged
merged 39 commits into from
Mar 12, 2025

Conversation

Minuring
Copy link

@Minuring Minuring commented Mar 7, 2025

체크 리스트

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

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

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

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

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

들여쓰기, 인스턴스 변수 3개와 같은 원칙을 지키지 못했습니다.

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

  1. 플레이어가 카드를 더 받을지 입력, 더 받은 결과 카드 리스트 출력을 도메인 로직 도중에 수행하기 위해 인터페이스를 이용하고 컨트롤러에서 각각 사용자입력, 출력하는 구현체를 넣어주었습니다. 잘 사용해보지 못한 방식을 새로 사용해 보았는데, 리뷰어가 보기에는 괜찮은 지 궁금합니다.

  2. 실제 블랙잭 게임을 생각하면 딜러가 플레이어 리스트와 카드덱을 가지고 있고, 모든 게임의 진행자이면서, 참가자이기도 합니다.
    이를 코드로 표현할 때 사실에 기반하여 표현하면 딜러는 막대한 역할을 가지게 된다고 생각했습니다.
    동시에 한편으로는 딜러의 역할이 맞기도 하지 않냐는 생각도 들었습니다.
    (딜러의 역할이니까 역할이 많다고 하더라도 응집도 높은 코드라고 해석할 수 있을 것 같습니다)
    리뷰어라면 이를 어떻게 구현할 것 같나요?


피드백을 반영하면서 어려웠던 부분

리뷰로 남겨두었습니다!

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.

포포 안녕하세요~
블랙잭 리뷰어 제로입니다.
이번 미션 잘 구현해주셨는데요 👏
몇 가지 코멘트 남겼으니 확인 부탁드려요!


image

딜러가 한장만 더 받는 것 같아요.
받았을 때에도 16이하라면 다시 카드를 받아야하지 않을까요?


  1. 인터페이스와 추상클래스 둘 다 사용한 것으로 확인됩니다. 어떤 기준으로 나누고 개발을 진행하였을까요?
  • 참고하면 좋을 자료도 공유드립니다.
  1. 실제 블랙잭 게임을 생각하면 딜러가 플레이어 리스트와 카드덱을 가지고 있고, 모든 게임의 진행자이면서, 참가자이기도 합니다.
    이를 코드로 표현할 때 사실에 기반하여 표현하면 딜러는 막대한 역할을 가지게 된다고 생각했습니다.
    동시에 한편으로는 딜러의 역할이 맞기도 하지 않냐는 생각도 들었습니다.
    (딜러의 역할이니까 역할이 많다고 하더라도 응집도 높은 코드라고 해석할 수 있을 것 같습니다)
    리뷰어라면 이를 어떻게 구현할 것 같나요?

딜러의 역할을 어디까지로 결정할지에 따라 다른 것 같은데요,

  • 딜러를 단순 참가자로 볼 지
  • 딜러가 게임 진행자로 볼 지
    두 가지 관점에 따라 다를 것 같아요. 저는 후자 쪽에 맞추어 개발할 것 같은데 정답은 없다고 생각합니다.

Comment on lines +7 to +20
문양이 스페이드, 다이아몬드, 하트, 클로버
- [x] 카드의 숫자 계산은 이렇게 한다.
- [x] Ace는 1 또는 11로 계산할 수 있다
- [x] King, Queen, Jack은 각각 10으로 계산한다.
- [x] 나머지 숫자는 그대로

- [x] 게임을 시작하면 플레이어는 두 장의 카드를 지급 받는다.
- [x] 두 장의 카드 숫자를 합쳐 21을 초과하지 않으면서 21에 가깝게 만들면 이긴다.
- [x] 플레이어와 딜러의 카드 합이 같으면 무승부이다.
- [x] 21을 넘지 않을 경우 원한다면 얼마든지 카드를 계속 뽑을 수 있다.
- [x] 딜러는 처음에 받은 2장의 합계가
- [x] 16점 이하 -> 반드시 1장의 카드를 추가로 받고
- [x] 17점 이상 -> 추가로 받을 수 없다.
- [x] 게임을 완료한 후 각 플레이어별로 승패를 출력한다.
Copy link

Choose a reason for hiding this comment

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

요구사항을 잘 작성해주셨네요 👏
에러케이스에 대해서도 작성할 수 있을까요?

import domain.card.Card;
import java.util.List;

public record FinalResultDTO(
Copy link

Choose a reason for hiding this comment

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

dto가 어떤 목적으로 사용하는 지 몰라서 명확하게 네이밍에서 request 또는 response라는 네이밍으로 개선해볼 수 있을까요?

Comment on lines 34 to 35
SetUpCardsDTO setUpCardsDTO = createSetUpCardsDTO(dealer, players);
outputView.printSetUpCardDeck(setUpCardsDTO);
Copy link

Choose a reason for hiding this comment

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

DTO를 만들고 이를 출력에 사용하도록 구현해주셨네요
DTO이 이번 프로젝트에서 필요하다고 느끼셨는지 궁금합니다!
그리고 어떤 장점이 있었을까요?

Comment on lines 67 to 83
private class TakeMoreCardViewSelector implements TakeMoreCardSelector {

@Override
public boolean isYes(String name) {
return inputView.getYesOrNo(name);
}

@Override
public void takenResult(String name, List<Card> cards) {
outputView.printTakenMoreCards(name, cards);
}

@Override
public void dealerTakenResult() {
outputView.printDealerTake();
}
}
Copy link

Choose a reason for hiding this comment

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

내부 클래스로 정의한 이유가 있을까요?
BlackjackController와 결합이 강해질 것 같아요

public SetUpCardsDTO createSetUpCardsDTO(Dealer dealer, List<Player> players) {
Card dealerOpenCard = dealer.getOpenCard();

Map<String, List<Card>> cards = new LinkedHashMap<>();
Copy link

Choose a reason for hiding this comment

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

LinkedHashMap으로 정의한 이유가 궁금합니다~

Copy link
Author

Choose a reason for hiding this comment

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

플레이어의 순서를 유지한 채 뷰로 넘겨주고 싶었습니다!


@Override
public boolean canTakeMoreCard() {
return (calculateScore() <= 16);
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 17 to 32
private static void validateLength(String name) {
if (name.length() > 10) {
throw new IllegalArgumentException("플레이어의 이름은 10자를 넘을 수 없습니다.");
}
}

private static void validateNotBlank(String name) {
if (name == null || name.isBlank()) {
throw new IllegalArgumentException("플레이어의 이름은 비어있을 수 없습니다.");
}
}

@Override
public boolean canTakeMoreCard() {
return (calculateScore() <= 21);
}
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 12 to 14
public Card getOpenCard() {
return cardDeck.getCards().getFirst();
}
Copy link

Choose a reason for hiding this comment

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

get, get을 사용하는 구조인데요, 줄여봅시다!


public class InputView {

private final Scanner scanner = new Scanner(System.in);
Copy link

Choose a reason for hiding this comment

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

외부에서 주입받으면 어떨까요?

  • DI의 장점은 무엇일까요?

Comment on lines 16 to 30
private static final Map<Rank, String> RANK_NAMES = Map.ofEntries(
Map.entry(Rank.ACE, "A"),
Map.entry(Rank.TWO, "2"),
Map.entry(Rank.THREE, "3"),
Map.entry(Rank.FOUR, "4"),
Map.entry(Rank.FIVE, "5"),
Map.entry(Rank.SIX, "6"),
Map.entry(Rank.SEVEN, "7"),
Map.entry(Rank.EIGHT, "8"),
Map.entry(Rank.NINE, "9"),
Map.entry(Rank.TEN, "10"),
Map.entry(Rank.JACK, "J"),
Map.entry(Rank.QUEEN, "Q"),
Map.entry(Rank.KING, "K")
);
Copy link

Choose a reason for hiding this comment

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

기존 enum을 활용할 순 없을까요?
만약 Rank에 새로운 값이 추가된다면, Rank 뿐 아니라 OutputView 수정도 필요할 것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

Gambler를 인터페이스로 만들면 추상 메서드인 canTakeMoreCard() 외에 완전히 똑같은 메서드들을 Dealer, Player에서 구현해야 했습니다.

인터페이스가 추상 클래스보다 우선시되는 이유를 읽어 보았는데, 만약 Gambler가 인터페이스였다면 인터페이스로써의 장점은 누리는 대신 중복 코드는 어쩔 수 없는건가요?

물론 중복 코드를 제거하기 위해 추상 골격 클래스를 이용하라는 방법도 있었습니다.
하지만 추상 클래스로만 사용하는것과 차이를 모르겠습니다. 추상 클래스가 있을 때 인터페이스가 또 존재함으로써 얻는 게 무엇인 지 공감이 되지 않습니다.
예를들어 블랙잭이 아닌 PokerGambler와 같은 확장 가능성을 생각하는 건가요?

Copy link

Choose a reason for hiding this comment

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

인터페이스가 추상 클래스보다 우선시되는 이유를 읽어 보았는데, 만약 Gambler가 인터페이스였다면 인터페이스로써의 장점은 누리는 대신 중복 코드는 어쩔 수 없는건가요?

맞습니다. 인터페이스는 함수를 정의만 할 뿐이기에 중복 코드가 발생할 수도 있습니다. 항상 인터페이스를 사용하는 것이 정답이라는 의미는 아니에요 :)

물론 중복 코드를 제거하기 위해 추상 골격 클래스를 이용하라는 방법도 있었습니다.
하지만 추상 클래스로만 사용하는것과 차이를 모르겠습니다. 추상 클래스가 있을 때 인터페이스가 또 존재함으로써 얻는 게 무엇인 지 공감이 되지 않습니다.
예를들어 블랙잭이 아닌 PokerGambler와 같은 확장 가능성을 생각하는 건가요?

추상 골격 클래스라는 개념을 어떤 문맥에서 접하셨는지 궁금합니다.
'추상클래스 + 인터페이스'를 말씀하신 것으로 이해하고 말씀드리면 말씀하신 예시가 맞다고 생각해요.
확장가능성이 있다고 생각되지만 오버 엔지니어링이 될 수도 있다고 생각해서 적절하게 사용하는게 중요할 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

추상 클래스 + 인터페이스 모두 사용하는 것을 말했습니다!
감사합니다 덕분에 이해하는 데 많은 도움이 되었어요 ㅎㅎ

Copy link
Author

@Minuring Minuring left a comment

Choose a reason for hiding this comment

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

피드백을 반영하면서 어려웠던 부분

객체가 필요로 하는 의존성을 생성자로 주입받도록 변경했습니다.

(대부분의 시간을 쏟은 부분입니다.)

의존성을 생성자로 주입받았을 때 제가 느낀 장점은,

A. 확장이 쉬워진다.
- 필요로 하는 객체의 종류가 확장되어도 영향을 받지 않습니다.
- 필요로 하는 객체의 생성 과정이 변경되어도 영향을 받지 않습니다.
- 즉 결합이 느슨해집니다.

B. 테스트하기 쉬워진다.
- 필요로 하는 객체의 행동을 테스트할 때 쉽게 제어할 수 있습니다.

하지만 오히려 단점이라고 생각한 부분도 있었습니다. 그 부분(Gambler)

단점이라고 생각한 이유는,

A. 참여자를 만들 때, 아무런 복잡한 생성 과정이 없는 CardHand를 강제로 만들어야하기 때문에,
참여자를 만드는 모든 코드에서 단순히 new CardHand()를 반복해야 했습니다.

B. 참가자에게는 카드를 지급하는 public 메서드가 있습니다.
따라서 생성자에서 주입을 하지 않더라도 해당 메서드를 이용해
테스트 시 참가자의 카드를 제어하는 데 무리가 없습니다.

(+ 최상위에서는 내부의 모든 구체적인 객체를 알아야 합니다. 
하지만 **모든 구현체의 조립**이라는 역할의 객체를 두면 합당할 것 같습니다.)

단점이라고 생각한 이유 A는 다음과 같이 해결(?)할 수도 있을 것 같습니다.

static Player ofDefaultHand(String name) {
  return new Player(name, new CardHand());
}

하지만 여전히 CardHand의 변경에 예민하게 영향을 받을 것 같습니다.

그래서 궁금한 것

  1. 위의 단점보다 장점이 더 가치있기 때문에 의존성을 주입받도록 하는 것인가요? 혹은 제가 느끼지는 않았지만 다른 큰 이유가 있는건가요?

  2. 객체를 설계할 때 대부분의 경우에서 의존성 주입을 받도록 하면 작은 단위의 도메인에서는 어떨 때 의존성을 직접 생성하나요?

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.

포포 안녕하세요~
다만 제가 질문드린 코멘트에서 답변이 빠져있어서 조금 아쉬웠어요ㅠ (포포의 생각을 알 수 없었네요)
다음엔 좀 더 신경써서 확인해주세요 :)
1단계는 이만 머지하겠습니다 :)
2단계도 화이팅이에요 🔥


  1. 위의 단점보다 장점이 더 가치있기 때문에 의존성을 주입받도록 하는 것인가요? 혹은 제가 느끼지는 않았지만 다른 큰 이유가 있는건가요?

이야기 주신 단점예시가 의존성 주입이 꼭 필요한가?에 대해서 작성해주신 것 같아요. 다만, 의존성을 주입받음으로써 발생하는 단점이라기보다 객체 조립을 분리하지 않았을 때 발생하는 불편함같이 느껴졌어요. 작성 주신 예시도 팩토리 패턴같은 것으로 보여지는데요, 단점을 완화하는 방법에 동의합니다

  1. 객체를 설계할 때 대부분의 경우에서 의존성 주입을 받도록 하면 작은 단위의 도메인에서는 어떨 때 의존성을 직접 생성하나요?

정말 단순한 객체이거나 내부적으로 독립적인 책임을 가질 때에는 직접생성하는 것도 적절하다고 생각합니다.
참고하면 좋을 자료도 공유드릴게요 :)

@asebn1 asebn1 merged commit 09a0775 into woowacourse:minuring Mar 12, 2025
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