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단계 - 블랙잭 베팅] 차니(이동찬) 미션 제출합니다 #887

Open
wants to merge 19 commits into
base: dongchannn
Choose a base branch
from

Conversation

DongchannN
Copy link

체크 리스트

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

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

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

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

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

Controller와 Domain사이에서 소통을 할 때 사용자로부터 카드를 더 받을 것인지 소통을 해야하기 때문에 getter의 사용이 있었습니다.

또한 배팅 금액 관련한 부분에서는 규칙이 필요한 도메인이라고 생각해서 Bet이라는 모델로 원시값 포장을 하였지만, 점수와 수익 관련해서는 원시값을 그대로 사용없다고 판단하여 원시값 포장을 하지 않았습니다.

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

안녕하세요 피케이, Dealer와 Player의 관계에 대해 많은 고민을 해보고 제 코드에 대한 확신이 부족하여 다양한 방법으로 시도하다 리뷰요청을 늦게 보내게 되었네요..

저는 이번 미션에서는 확장가능성과 추상화에 대해 고민 많이했습니다.

그와 관련해서 DealerPlayerHand라는 필드를 두어 조합을 사용해 중복 코드를 일부 제거했지만, 메서드 중복은 굳이 없앨 필요가 없다고 판단하여 상속은 사용하지 않았습니다.

Participant를 클래스나 추상클래스로 만들어 상속을 사용하는 경우 발생하는 문제점은 아래와 같았습니다.

  1. 딜러는 이름이 필요 없고, 승패를 계산할 필요가 없기 때문에 플레이어와 상태와 행동에서 차이점을 보입니다.
  2. 위 차이점 때문에 상속을 사용하고, Participant player = new Player()와 같이 사용한다면 player는 이름을 가져온다거나 승패를 계산하는 로직을 호출하지 못합니다.
  3. 이를 해결하고자 Dealer에게 private static final String name = "딜러"와 같이 이름을 부여하는 것은 부모 클래스가 하위 클래스의 설계를 강제하는 것이라 생각해 확장 가능성도 떨어지고 위험한 설계라고 판단했습니다.
  4. instanceof를 사용하는 것은 추상화를 깨뜨리는 행위라고 생각했기 때문에 경계했습니다.
  5. Player player = new Player()와 같이 사용한다면, 상속을 단지 코드 재사용만을 위해 사용했기 때문에 또 적절치 못하다고 판단하였습니다.

플레이어와 딜러는 다르기에 상속은 적절하지 못하다는 결론을 내려, 카드에 대한 공통 로직 구현만 관리하기 위해 인터페이스를 사용하였습니다.

이러한 기준 하에 코드를 작성했지만, 제 코드가 제 기준을 일관되게 잘 맞추었는지 모르겠습니다. 제 기준에 일관되지 않은 부분이 있는 지 중점으로 리뷰를 받고싶습니다!!

Copy link
Author

Choose a reason for hiding this comment

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

배팅을 관리하는 부분을 Player에서 실시하는 것이 맞을지, 배팅을 위한 도메인을 따로 만드는 것이 맞을지 고민을 하다가 아래와 같이 BettingPool로 따로 관리하였습니다.

Player에서 관리 시 지금 역할에 더해 배팅에 대한 수익을 판단하는 역할까지 생겨 책임이 막중해진다고 생각해, 배팅 관련 도메인을 빼 배팅에 대한 응집도를 챙기자는 생각이었어요.

이와 관련해서 피케이는 Player에 두는 것과 지금과 같은 방법 중 어떤 것이 더 나은 설계라고 판단하는지 궁금합니다!

Copy link
Member

Choose a reason for hiding this comment

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

하나의 도메인이 너무 많은 역할을 갖게되는 것은 좋지 않습니다.
코드가 길어져 유지보수나 확장성 측면에서도 좋지 않고요.

지금처럼 분리하는 것이 더 좋은 선택이라고 생각합니다. :)

}

return determineResultByScore(dealer);
}
Copy link
Author

Choose a reason for hiding this comment

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

피케이의 1단계 리뷰를 받고 아래 세가지 방법 중에서 고민을 했습니다.

  1. 승패에 대한 상태를 관리하는 enum에서 승패 판단.
  2. 블랙잭의 승패를 판단하는 도메인 생성
  3. Player에서 승패 판단

위 세가지 방법을 모두 작성해본 결과 확장 가능성 측면에서 2번 또는 3번이 우수하다고 생각했습니다.

하지만 플레이어가 점수를 계산하거나, 카드를 받을 지 여부를 판단하는 경우 등에서 블랙잭의 로직으로 점수를 도출한다는 점을 고려해 블랙잭 룰과 관련된 로직을 완전 담당하게 하는 것이 응집도를 더 높일 수 있다고 판단하여서 해당 모델로 로직을 옮겼습니다!

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

public class Cards {
public class Hand {
Copy link
Author

Choose a reason for hiding this comment

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

일급 컬렉션과 관련한 질문입니다. List<Card> 등과 같이 콜렉션을 사용할 때 일급 콜렉션을 사용하라는 것으로 배웠습니다.

이 때 아래의 경우 중 어떤 경우로 사용하라는 것인지 확실하지 않아 질문드려요.

코드를 작성해본 경험 상 2번 경우로 사용하는 것이 더 좋다고 판단하였는데 제 생각이 맞을까요?

// 1번
public class Cards {
  private final List<Card> cards;
}

public class Hand {
  private final Cards hand;
}

public class CardDeck {
  private final Cards deck;
}
// 2번
public class Hand {
  private final List<Card> cards;
}

public class CardDeck {
  private final Stack<Card> deck;
}

Copy link
Member

Choose a reason for hiding this comment

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

이 부분은 경우에 따라 선택이 달라집니다.
만약 Cards 내부에 공통화할 수 있는 메소드들(ex. addCards, removeCard 등)이 많고 이를 HandCardDeck에서 모두 사용한다면 Cards로 쓰는 것이 좋습니다.

그러나 HandCardDeck 내부에서 여러 개의 Card에 대해 공통적으로 사용하는 기능이 없다면 2번으로 구현하는 것 만으로 충분합니다. :)

Copy link
Author

Choose a reason for hiding this comment

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

정리하면 List<Card>를 들고 있는 일급 컬랙션들로 구현을 먼저 하되, 그 안에서 공통된 메서드가 있다면 그 때 따로 빼면 되겠네요!


public Dealer getDealer() {
return dealer;
}
Copy link
Author

Choose a reason for hiding this comment

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

기존에는 Dealer에 대한 getter가 존재하지 않았습니다.

모델 간의 로직에서 getter를 사용하지 않지만 Controller의 진입 메서드에서 getter를 사용하는 것이 가장 아름다운 코드가 나와서 도입하였는데, 이러한 경우 getter의 사용에 대해서 피케이는 어떻게 생각하시는지 궁금합니다!

Copy link
Member

Choose a reason for hiding this comment

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

getter 를 아예 사용하지 않을 수는 없습니다.
필요하다면 적절히 사용해야죠! 😀

@DongchannN DongchannN changed the title Step2 [2단계 - 블랙잭 재구현] 차니(이동찬) 미션 제출합니다 Mar 15, 2025
@DongchannN DongchannN changed the title [2단계 - 블랙잭 재구현] 차니(이동찬) 미션 제출합니다 [2단계 - 블랙잭 베팅] 차니(이동찬) 미션 제출합니다 Mar 15, 2025
Copy link
Member

@pkeugine pkeugine left a comment

Choose a reason for hiding this comment

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

차니 수고하셨습니다 👍👍
질문에 대한 답변과 코멘트 몇 가지 남겼으니 확인 부탁드립니다 :)


import domain.card.Card;

public interface CardOwner {
Copy link
Member

Choose a reason for hiding this comment

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

  1. 딜러는 이름이 필요 없고, 승패를 계산할 필요가 없기 때문에 플레이어와 상태와 행동에서 차이점을 보입니다.
  2. 위 차이점 때문에 상속을 사용하고, Participant player = new Player()와 같이 사용한다면 player는 이름을 가져온다거나 승패를 계산하는 로직을 호출하지 못합니다.

위 내용을 보면, 추상 클래스를 사용하지 못한 이유가 name 필드가 Player에만 필요하기 때문이라고 이해했습니다.

하지만, 추상 클래스를 사용하더라도 이를 상속하는 자식 클래스에서 추가 필드를 선언하여 활용할 수 있습니다.
CardOwner를 추상 클래스로 구현해도 Player에만 name 필드를 추가할 수 있어요!

return cardDeck.popCard();
@Override
public void receive(Card card) {
ownedHand.add(card);
Copy link
Member

Choose a reason for hiding this comment

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

메서드 중복은 굳이 없앨 필요가 없다고 판단하여 상속은 사용하지 않았습니다.

가능하다면 상속을 활용해서 메서드 중복 또한 없애주는 것이 좋습니다.


public Dealer getDealer() {
return dealer;
}
Copy link
Member

Choose a reason for hiding this comment

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

getter 를 아예 사용하지 않을 수는 없습니다.
필요하다면 적절히 사용해야죠! 😀

Copy link
Member

Choose a reason for hiding this comment

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

하나의 도메인이 너무 많은 역할을 갖게되는 것은 좋지 않습니다.
코드가 길어져 유지보수나 확장성 측면에서도 좋지 않고요.

지금처럼 분리하는 것이 더 좋은 선택이라고 생각합니다. :)

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

public class Cards {
public class Hand {
Copy link
Member

Choose a reason for hiding this comment

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

이 부분은 경우에 따라 선택이 달라집니다.
만약 Cards 내부에 공통화할 수 있는 메소드들(ex. addCards, removeCard 등)이 많고 이를 HandCardDeck에서 모두 사용한다면 Cards로 쓰는 것이 좋습니다.

그러나 HandCardDeck 내부에서 여러 개의 Card에 대해 공통적으로 사용하는 기능이 없다면 2번으로 구현하는 것 만으로 충분합니다. :)

Copy link
Member

Choose a reason for hiding this comment

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

JUnit 5에서는 public class 가 아니어도 테스트가 정상적으로 실행됩니다.

Copy link
Author

Choose a reason for hiding this comment

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

피케이의 위 코멘트를 보고 해당 내용에 대해서 찾아보았습니다! 덕분에 새로운 사실을 알게되었어요. 👍
테스트 파일 생성 시 인텔리제이의 단축키를 사용하면 package-private이 되지만 직접 클래스를 생성하면 위와 같이 public이 붙게 되네요.

Junit 5 : Test Classes and Methods

위 글에서는 특별한 기술적 이유가 아니라면, public을 붙이지 말라고 이해했습니다.

특별한 기술적인 이유가 저에게 와닿지 않아서 실제로 실무에서 어떤 경우에 public을 붙이는지 궁금합니다!

또한 쓰면 안되는 명확한 이유는 찾지 못했는데, 테스트 클래스를 public으로 모두 사용하다가 해당 테스트 클래스가 default이여야하는 이유가 있다면 제거하면 안될까 하는 생각도 듭니다. public을 사용해서 위험한 경우가 있는지가 궁금해요!

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