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단계 - 블랙잭 베팅] 프리(김현욱) 미션 제출합니다 #884

Open
wants to merge 17 commits into
base: kimdevv
Choose a base branch
from

Conversation

kimdevv
Copy link

@kimdevv kimdevv commented Mar 14, 2025

체크 리스트

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

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

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

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

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

1단계에서는 나름 객체지향 생활체조 요구사항을 잘 지켰다고 생각하였습니다.
그러나 2단계를 진행하고 리팩토링을 하면서 부족한 부분이 굉장히 많았다는 것을 깨닫게 되었습니다.
객체지향의 고수가 되기 위해서는 아직까지 갈 길이 멀다고 생각합니다.

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

안녕하세요, 매트! 1단계가 머지되고도 무려 4일이 지나서야 2단계 PR을 요청드리게 되었습니다. 리팩토링을 하던 도중 MVC 패턴에 대해 의문이 많이 생겨서 주변 크루들과 열심히 토론을 하니 시간이 너무 빠르게 지나갔던 것 같습니다...!

이번 PR에서 가장 문의드리고 싶은 부분 역시 이 MVC 패턴입니다. 기본적으로는 Model과 View가 프로그램을 구성하고 있어야 하고, 이 둘은 서로를 알지 못하는 상태여야 합니다. 그러나 사용자는 View를 통해서 입력과 출력을 받고, 그렇게 입출력된 내용을 Model이 사용하게 하기 위해서 그 중간에 Controller가 있는 것이라고 생각합니다. 즉, 가장 이상적인 컨트롤러는 뷰와 모델을 연결해 주기만 하고, 그 외의 부가적인 기능은 가지고 있지 않아야 한다고 생각했습니다. 물론 모델 하나만 참조해서는 비즈니스 로직을 전개할 수 없기 때문에 컨트롤러가 모델 대신 Service라고 하는 일련의 비즈니스 모델을 호출하는 방식도 가능합니다. 가장 간단한 예로, Spring 프레임워크에서 사용되는 컨트롤러가 이러한 양상을 보이고 있습니다.

제가 기존에 작성했던 코드(1단계) 링크는 다음과 같습니다. https://github.com/woowacourse/java-blackjack/pull/815/files
이 코드에서의 컨트롤러는 위의 내용과 다르게 굉장히 많은 일을 하고 있습니다. 입력을 받아서 객체를 생성하는 일부터, 게임의 턴을 진행하는 일, 버스트인지 확인한 후 버스트라면 관련 로직을 수행하는 일, 최종 결과를 출력하는 일 등 컨트롤러에게 굉장히 많은 책임을 부여하고 있다는 것을 알게 되었습니다. 그래서 저는 '만약 지금처럼 콘솔 프로그램이 아니라, 여기서 더 확장하여 웹으로 입출력을 받는 프로그램으로 확장된다면?'을 고려하게 되었습니다. 그럴 경우에는 View 뿐만 아니라 Controller도 바뀌게 될 텐데, 그러면 웹 전용 컨트롤러를 만들면서 관련된 로직을 모두 다시 짜야 하는 문제가 생겼습니다. 그래서 저는 컨트롤러에서는 Participant, Dealer, Deck 등 기본적인 객체를 생성하는 역할만 하고, 그것들을 조합하여 일련의 비즈니스 흐름을 진행시키는 클래스를 새로 만들어서 컨트롤러에 담겨 있는 다양한 역할을 최소한으로 분산하고자 했습니다(Bean이나 Repository 등으로 객체들이 관리되는 Spring과는 달리 저희는 쌩 자바 MVC이므로, 완벽하게 Spring Controller처럼은 만들 수 없고 객체를 생성하는 등의 최소한의 역할은 어쩔 수 없이 컨트롤러에서 담당해야 한다고 생각합니다). 그렇게 되면 컨트롤러가 웹 컨트롤러로 변경되어도 비즈니스 로직은 해당 클래스가 모두 관리하므로 아마 확장성이 증가할 거라고 생각했습니다.

이러한 일련의 비즈니스 흐름을 담당하는 객체가 바로 BlackJackRule입니다. 이 BlackJackRule 클래스는 실제 생활에서 상품 안내서와 같은 역할을 수행합니다. Participant나 Dealer과 같은 객체들을 갖고 있는 컨트롤러는 BlackJackRule에게 **나 이런 객체들 갖고 있는데, 모든 플레이어에게 카드를 2장씩 나눠주는 기능을 하려면 어떻게 해야 돼?**라고 물어보면 BlackJackRule은 **내가 initializeBlackJack()라는 메서드를 갖고 있으니, 너는 이 메서드를 실행시키는 데 필요한 객체들만 전달해 줘.**라고 주고받는 느낌...? 그렇기 때문에 BlackJackRule은 모든 메서드가 static으로 선언되어 있습니다. 상태를 가지지 않고, 프로그램 전역에서 충분히 사용될 수 있으며, 무엇보다도 현실에서의 상품 안내서는 진짜 상품을 가지고 소개하는 것이 아니기 때문입니다. 필요한 객체들만 주어진다면 안내서에 나와있는 대로 뚝딱뚝딱 결과를 도출할 수 있도록 하기 위함이었습니다. 이 부분이 기존에 Service 레이어와의 차별화된 부분이라고도 볼 수 있는 것 같은데, Service와 달리 BlackJackRule은 필드를 가지지 않고, 필요한 객체들만 주어지면 그에 해당하는 비즈니스 로직을 처리하는 부분이 독립적이라는 것입니다.(이 부분이 틀렸을 수도 있는데 만약 틀렸다면 조금 더 공부해 보겠습니다!)

정리를 해보자면,

  1. 이상적인 컨트롤러의 역할은 View와 Model(Service)에서의 연결만 담당하는 것이다. 그러나 기존의 컨트롤러는 너무 많은 역할을 가지고 있었다.
  2. 그렇다면 컨트롤러가 단순히 View와 Model의 연결만을 담당하게 하려면 어떻게 해야 할까?
  3. Spring 프레임워크의 Controller와는 다르게 현재의 Controller는 완벽히 이상적일 수 없다. 그렇다면 현재 Controller가 가지고 있는 일련의 비즈니스적인 부분을 최대한 다른 클래스에 위임한다면 어떨까? -> BlackJackRule
  4. Controller에서는 블랙잭에 필요한 객체들을 생성한 후, BlackJackRule을 호출하여 그 객체들을 가지고 이루어지는 비즈니스 로직을 실행시켜 주는 최소한의 역할만을 하게 한다면 Controller의 역할은 많이 줄어들 것이고 굉장히 가벼워질 것이다.

대충 이런 흐름으로 생각해 봤습니다...! 새벽이라서 머리가 잘 안 돌아가서 글이 제대로 안 써지네요...ㅋㅋ큐ㅠㅠ 이런 관점에서 한 번 리뷰해 주시면 정말 감사하겠습니다!

Comment on lines +90 to +98
/*public void outputFinalResult(final Map<Participant, ParticipantResult> participantResults, final Map<ParticipantResult, Integer> participantResultCounts) {
System.out.println("## 최종 승패");
CustomStringBuilder customStringBuilder = new CustomStringBuilder();
Map<ParticipantResult, Integer> winLoseResult = new HashMap<>(Map.of(ParticipantResult.WIN, 0, ParticipantResult.LOSE, 0));
customStringBuilder.appendLine(String.format("딜러: %d승 %d패", participantResultCounts.get(LOSE), participantResultCounts.get(WIN)));
for (Map.Entry<Participant, ParticipantResult> resultEntry : participantResults.entrySet()) {
customStringBuilder.appendLine(String.format("%s: %s", resultEntry.getKey().getName(), resultEntry.getValue().getDescription()));
}
customStringBuilder.print();
}*/
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

Choose a reason for hiding this comment

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

어디서도 사용하지 않는다는 것이 확인되면 지우는 편 입니다. 남아 있게 되면 오히려 필요한 로직을 찾는데 방해를 하는 경우도 생기더라구요.

Copy link

@hyeonic hyeonic left a comment

Choose a reason for hiding this comment

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

안녕하세요 프리~ 이야기 나눠보면 좋을 것 같은 부분 질문 주셔서 간단히 코멘트 남겼습니다. 확인해주시고 질문이 모호하거나 이해가 안된다면 언제든 DM 남겨주세요 ㅎㅎ

Comment on lines +90 to +98
/*public void outputFinalResult(final Map<Participant, ParticipantResult> participantResults, final Map<ParticipantResult, Integer> participantResultCounts) {
System.out.println("## 최종 승패");
CustomStringBuilder customStringBuilder = new CustomStringBuilder();
Map<ParticipantResult, Integer> winLoseResult = new HashMap<>(Map.of(ParticipantResult.WIN, 0, ParticipantResult.LOSE, 0));
customStringBuilder.appendLine(String.format("딜러: %d승 %d패", participantResultCounts.get(LOSE), participantResultCounts.get(WIN)));
for (Map.Entry<Participant, ParticipantResult> resultEntry : participantResults.entrySet()) {
customStringBuilder.appendLine(String.format("%s: %s", resultEntry.getKey().getName(), resultEntry.getValue().getDescription()));
}
customStringBuilder.print();
}*/
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 +3 to +6
import blackjack.model.game.BettedMoney;
import blackjack.model.player.Participant;
import blackjack.model.player.Participants;
import blackjack.model.player.PlayerName;
Copy link

Choose a reason for hiding this comment

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

불필요한 import는 제거해주세요!

import blackjack.model.game.BlackJackGame;
import blackjack.model.game.DeckInitializer;
import blackjack.model.BlackJackRule;
import blackjack.model.game.*;
Copy link

Choose a reason for hiding this comment

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

와일드카드로 import를 사용할 경우 어떤 단점을 가질 수 있을까요?

Comment on lines +3 to +26
public class BettedMoney {

public static final int MINIMUM_AVAILABLE_MONEY = 0;
public static final int MAXIMUM_AVAILABLE_MONEY = 1_000_000_000;
private final int money;

public BettedMoney(final int money) {
validate(money);
this.money = money;
}

private void validate(final int money) {
if (money < MINIMUM_AVAILABLE_MONEY) {
throw new IllegalArgumentException("0원 이상 베팅 가능합니다.");
}
if (money > MAXIMUM_AVAILABLE_MONEY) {
throw new IllegalArgumentException("최대 10억 원까지 베팅 가능합니다.");
}
}

public int getMoney() {
return money;
}
}
Copy link

Choose a reason for hiding this comment

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

극한의 상황이지만 모든 참가자가 999_999_999를 베팅한다면 경우에 따라 아래와 같은 결과가 나올 수도 있을 것 같아요.

image

돈과 관련된 연산을 진행할 때는 어떤 타입을 고려해볼 수 있을까요?

Comment on lines +14 to +57
public class BlackJackRule {

public static Deck generateDeck() {
return new DeckInitializer().generateDeck();
}

public static void initializeBlackJack(final Deck deck, final Dealer dealer, final Participants participants) {
participants.getParticipants()
.forEach(participant -> giveTwoCards(participant, deck));
giveTwoCards(dealer, deck);
}

private static void giveTwoCards(final Player player, final Deck deck) {
player.putCard(deck.drawCard());
player.putCard(deck.drawCard());
}

public static void giveCard(final Player player, final Deck deck) {
player.putCard(deck.drawCard());
}

public static void giveCardToCurrentTurnParticipant(final Participant participant, final boolean isParticipantWantCard, final Deck deck) {
if (isParticipantWantCard) {
giveCard(participant, deck);
}
}

public static boolean isDealerDrawable(final Dealer dealer) {
return dealer.isCardDrawable();
}

public static Map<Participant, Integer> calculateWinningMoney(final Dealer dealer, final Participants participants) {
Map<Participant, ParticipantResult> participantResults = ParticipantResult.calculateParticipantResults(dealer, participants);
return MoneyDistributor.calculateWinningMoney(dealer, participantResults);
}

public static int calculateDealerWinningMoney(final Map<Participant, Integer> winningMoney) {
return MoneyDistributor.calculateDealerMoney(winningMoney);
}

public static boolean isPlayerBust(final Participant participant) {
return participant.isBust();
}
}
Copy link

Choose a reason for hiding this comment

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

프리가 PR 설명란에 적어주신 것 처럼 많은 고뇌가 느껴지는 것 같아요. 우선 제 생각을 남기기 전에 간단히 질문 먼저 드릴게요.

지금 프리는 콘솔 환경의 Controller를 구성할 때 웹 개발에 사용하는 레이어드 아키텍처와 Controller에 대한 고려가 같이 하기 때문에 더욱 어렵게 느끼시는 것 같아요. 현재 미션에서 웹 환경에 대한 특성을 다루긴 이르지만 현재 개발하고 있는 콘솔 환경은 웹 환경은 어떠한 차이가 있다고 생각하시나요? 지속적으로 서버가 떠있는 상태에서 사용자와 상호작용하나요? 혹은 게임을 진행하고 끝나면 그대로 종료되나요?

import static blackjack.model.game.ParticipantResult.LOSE;
import static blackjack.model.game.ParticipantResult.WIN;

public class MoneyDistributor {
Copy link

Choose a reason for hiding this comment

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

컨벤션 한번 확인해주세요!

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