[1단계 - 블랙잭] 모다(이채영) 미션 제출합니다. by Chaeyoung714 · Pull Request #780 · woowacourse/java-blackjack

Chaeyoung714 · 2025-03-07T06:57:08Z

안녕하세요 라라! 리뷰를 부탁하게 된 모다라고 합니다. 😊
리뷰 해주셔서 감사합니다 ㅎㅎ 아래 어떤 점을 중점으로 리뷰해주셨으면 좋겠는지 작성했으니, 아낌없는 지적과 리뷰 부탁드립니다!!! 🙇‍♀️🙇‍♀️🙇‍♀️

체크 리스트

미션의 필수 요구사항을 모두 구현했나요?
Gradle test를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?
애플리케이션이 정상적으로 실행되나요?
프롤로그에 셀프 체크를 작성했나요?
- [셀프체크 링크](https://prolog.techcourse.co.kr/studylogs/4017)

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

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

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

한 메서드에 오직 한 단계의 들여쓰기만 한다 -> 4점
else 표현을 사용하지 않는다 -> 5점
모든 원시 값과 문자열을 포장한다 -> 3점
한 줄에 점을 하나만 사용한다 -> 5점

뷰를 제외하고는 디미터의 법칙을 지켰습니다.
뷰의 경우 개발자(=저)가 뷰에서 도메인을 변경하는 로직을 작성하지 않았다는 확신이 있으니, 디미터 법칙을 어기는 것에 크게 단점을 느끼지 못했습니다.

이름을 줄여 쓰지 않는다(축약 금지) -> 5점
모든 엔티티를 작게 유지한다 -> 4점
3개 이상의 스위프트 기본 데이터타입(Int, String, Double 등) 프로퍼티를 가진 타입을 구현하지 않는다 -> 5점
일급 콜렉션을 사용한다 -> 5점
getter/setter를 구현하지 않는다 -> 3점

getter를 사용하여 메세지를 던지지 않고 구현하는 방식을 최대한 피했으나, Players에서 players 필드를 가져와서 player 각각에게 메세지를 던지는 방식으로 getter를 많이 썼던 것 같습니다.

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

코드에 리뷰 형태로 질문 달아두었습니다.

위의 내용 위주로 리뷰해주시면 감사하겠습니다!!

Chaeyoung714

안녕하세요 라라!
궁금한 점 코드에다 리뷰 형식으로 달아두었으니 확인 부탁드립니다.
감사합니다!

Chaeyoung714 · 2025-03-07T07:17:49Z

+        if (dealerScore < playerScore){
+            return GameResult.WIN;
+        }
+        return GameResult.DRAW;


ENUM은 상수에 가깝고, 객체와는 구분되는 개념이기 때문에 ENUM에 메세지를 던져서 비즈니스로직을 구현하지 않는다는 내용을 학습한 적이 있어서, GameResult를 구하는 로직을 GameResult enum에 구현하지 않고 위와 같이 구현했습니다.

그러나 계속해서 WinningResult 객체는 심판처럼 결과를 판단해주는 로직이 아니라, 외부에서 알려준 승패 결과를 플레이어 또는 딜러에게 맞춰 알려주는 역할을 하고 있기 때문에, 이 로직이 여기 들어가는 것이 적합한 책임이 아니라는 생각이 듭니다.

이럴때도 GameResult와 관련된 비즈니스로직은 GameResut와 분리되어야 하는 게 맞는지, 아니면 GameResult 안에 두어도 괜찮을지 의견이 궁금합니다.

Enum이 비즈니스 로직을 가져도 되는가? 에 대한 좋은 고민인 것 같아요 👍🏻
요것도 정답이 정해져있다고 생각하진 않아요. 본인만의 근거와 기준을 정하면 될 것 같은데요~

저는 GameResult 가 승패를 나타내는 값이기 때문에, 자신을 생성하는 로직을 가지는 것이 자연스럽다고 생각해요. 🙂

public static GameResult fromScores(int dealerScore, int playerScore) { if (playerScore > dealerScore) { return WIN; } if (playerScore < dealerScore) { return LOSE; } return DRAW; }

Chaeyoung714 · 2025-03-07T07:21:41Z

Dealer와 Participant클래스의 중복 로직을 제거하기 위해 두 객체 모두 Participant를 상속받고, Participant는 ParticipantHand 클래스를 조합해서 구현했습니다.

이에 대해 몇가지 질문이 있습니다.

Chaeyoung714 · 2025-03-07T07:25:13Z

+    }
+
+    public Card getFirstHand() {
+        return getParticipantHand().getCards().getFirst();


Dealer 내부에서 부모인 Participant 메서드를 불러서 구현을 하는 것

Dealer와 Participant는 상속관계이지만, Participant의 필드가 필요해 조합처럼 부모클래스의 메서드를 불러와 사용했습니다.

이런 상속 방식 + 부모의 메서드를 호출하는 방식이 모두 섞이니 정확한 상황을 설명하긴 어렵지만 '위험할 것 같다'는 생각이 막연하게 듭니다.
그런데 위험할 수 있는 상황이 어떤 상황일지 아직 잘 모르겠어서 설명이 어렵습니다..
그래서 이 코드에 대한 라라의 생각을 먼저 여쭙고 싶습니다. 제가 상속을 제대로 활용한 것일까요? 라라가 보시기엔 어떤 위험성이 예상되시나요?

답변에 앞서 먼저 모다에게 질문을 드리고 싶습니다!

추상화하는 방법으로 인터페이스도 존재하는데 상속을 선택하신 이유가 무엇인가요?

둘의 장/단점 차이는 무엇이 있을까요?

현재 대부분의 기능 구현이 부모 클래스에 구현되어 있기 때문에, 부모의 변경이 자식에게 의도치 않은 영향을 줄 가능성이 있다고 생각합니다.
그리고 객체지향의 SOLID 원칙 키워드를 학습하시고,
해당 상속 구조가 리스코프 치환 원칙(LSP)을 잘 지키는 구조인지 고민해보셔도 좋을 것 같아요 🙂

추상화하는 방법으로 인터페이스도 존재하는데 상속을 선택하신 이유가 무엇인가요?

일단 인터페이스와 상속은 목적이 다르다고 생각했습니다.

인터페이스는 역할만 명시하고, 구현체는 아예 따로 만듦으로써 역할과 구현을 분리하는 것이 목적이라고 생각했어요. 대신 상속은 중복 코드를 제거하는 것이 목적이라고 생각했습니다. 대신 중복이면 다 상속으로 처리하진 않고, 중복인 이유가 자식클래스가 부모클래스와 is-A 관계라서 일부 역할을 공유할 때에만 필요하다고 생각했어요.

그리고 지난번 미션에서 인터페이스를 다양하게 도입해봤는데, 구현체끼리 중복되는 코드가 많은 경우 굳이 구현체 2개에 같은 코드를 복붙하는 점이 비효율적이어 보였습니다.

이번엔 Dealer나 Player와 Participant 간의 is-A 관계가 명확해 보였고, 중복코드 제거가 가장 큰 목적이었기 때문에 자연스럽게 상속이 적합하다고 판단했습니다.

둘의 장/단점 차이는 무엇이 있을까요?

인터페이스의 경우 주어진 역할이 정확히 같지만 구현 방식이 다른 객체에게 적용했을 때 가장 효과적이라고 생각합니다.
그러한 객체에 인터페이스를 적용하면, 해당 인터페이스를 의존했을 때 쉽게 구현방식을 갈아끼울 수 있다는 장점이 있는 것 같습니다. 이것이 OCP에도 많은 도움이 되는 것 같아요.

다만 인터페이스는 자신의 메서드를 구현할 것을 강제한다는 점에서 단점도 있는 것 같아요.
인터페이스가 강제한 메서드와 역할이 완전히 같지 않다면 구현체 중 일부는 인터페이스에서 강제로 오버라이딩하는 메서드 중 필요없는 메서드가 있을 수 있습니다.
또한 앞서 말했듯 구현체 간 구현방식이 비슷한 경우 중복되는 오버라이딩 코드가 많아져 코드의 효율성이 떨어질 수 있습니다.

상속의 경우 중복을 쉽게 제거할 수 있습니다.
또한 자신이 정의하지 않은 메서드라도(부모 메서드) 쉽게 사용하면서 'is-A 관계면 기능을 사용 가능하다'는 점이 명확하게 드러납니다.

다만 부모와의 강결합이 여러모로 문제가 되는 것 같습니다!
부모 메서드가 바뀌면 의도치않고 자식 메서드에도 영향을 줍니다.
또 자식 메서드에서 부모 메서드를 오버라이딩해 사용한다면 부모메서드가 변경되었을 때 자식메서드는 이를 아예 알지 못한다는 단점도 있는 것 같아요

현재 대부분의 기능 구현이 부모 클래스에 구현되어 있기 때문에, 부모의 변경이 자식에게 의도치 않은 영향을 줄 가능성이 있다고 생각합니다

사실 이부분에 대해 이론적으론 완전히 동의하지만,
'상속이면 is-A 관계인데, 부모가 변하면 자식이 변하는 게 당연한 것 아닌가?'라는 생각도 들어서 단점을 실감하기가 어려웠던 것 같아요 🥲
가령 Dealer, Player의 상위인 Participant.receiveCard()의 정책이 바뀌었다면, 이는 곧 딜러와 플레이어가 카드를 뽑는 정책도 바뀌었으니 함께 적용되는 것이 맞지 않나 싶습니다.
오히려 부모의 변경이 자식에게 의도치않게 영향을 준다는 건 상속관계를 잘못 설정한 거 아닐까? (is-A 관계가 아니라던지..) 라는 생각도 듭니다. 여기에 대한 라라의 의견이 궁금합니다!

객체지향의 SOLID 원칙 키워드를 학습하시고,
해당 상속 구조가 리스코프 치환 원칙(LSP)을 잘 지키는 구조인지 고민해보셔도 좋을 것 같아요 🙂

좋은 학습키워드 감사합니다!
라라의 질문에 답을 쓰면서도 제 스스로 확신이 없는 부분도 많아서, 객체지향을 다시 공부해봐야겠다는 필요성이 드네요..!!

Chaeyoung714 · 2025-03-07T07:33:36Z

+    }
+
+    public Card getFirstHand() {
+        return getParticipantHand().getCards().getFirst();


상속을 써도 될까?

여러 책을 읽으면 상속은 많이들 지양을 하고, 그 이유에 대해서 (ex. 부모의 강한 결합..) 충분히 납득했습니다.

그러나 막상 구현을 해보니 그런 단점이 잘 느껴지지 않았고, '부모가 변경되면 자식도 함께 변경되어야 한다'와 같은 단점도 저희가 부모메서드를 오버라이딩해서 쓰지 않았기 때문에 잘 느껴지지 않았습니다. (오버라이딩을 했다면 부모와 자식의 메서드가 일치하지 않았기때문에 단점이 더 느껴졌을 것 같습니다.)

오히려 중복이 잘 제거되고, Dealer,Player-Participant-ParticipantHand 사이 is-A관계와 has-A가 관계가 잘 드러나고 있다고 생각했습니다.
그나마 느낀 단점은 Dealer의 관점에서 봤을땐 이 클래스가 어떤 책임을 가지는지 잘 드러나지 않는다는 것이었지만, 이는 반대로 생각해보면 상속이 가진 장점이기 때문에 단점일지도 잘 모르겠습니다.

저희는 '지금 단점이 안느껴진다면 진행하지 말자'라는 생각으로 다른 크루들이 썼다는 추상클래스, 인터페이스 등을 도입하지 않았습니다.
라라라면 이와 같은 상황에서 어떤 방식을 시도하셨을 것 같은가요?(멈추기, 그래도 다른 방법 도입해보기 등등..)
그리고 라라의 시선에서 봤을땐 이 클래스가 가진 위험성이 있는지 궁금합니다.

저는 다양한 방법을 시도해보고 직접 장/단점을 느껴보고 본인만의 기준을 만들어가는 것을 추천해요. 🙂
미션을 기회로 추상클래스/인터페이스 다양하게 시도해보고 생각만 해본 장/단점을 직접 느껴보시는건 어떨까요? 후기도 궁금하네요 😀

라라의 시선에서 봤을땐 이 클래스가 가진 위험성이 있는지 궁금합니다.

지금 부모 클래스의 메서드를 공통적으로 사용하고 있는데요.
dealInitialCards()를 예시로 player만 초키 카드를 3장씩 받는 방식으로 변경되면 어떻게 될까요? 🤔

동의합니다! 그래서 저는 추상클래스로 시도를 해봤는데요, 시도하고 장단점을 적으면서 답을 잘 못하는 부분은 추가로 학습을 해보고 정리해보려 합니다
~~이부분은 따로 디엠으로 답장드려도 될까요?~~ 아래 비교정리한 내용 공유드립니다!

추상클래스(상속)

장점

일반 상속에 비해, 클래스의 정체성을 명확히 하고 클래스를 오용할 가능성을 방지할 수 있다.

조합에 비해, is-A 관계를 명확하게 표현할 수 있다. (Dealer, Player는 Participant다)

인터페이스에 비해, 중복되는 필드까지 중복을 제거할 수 있다. (ParticipantHand)

인터페이스에 비해, 공통되는 구현은 부모역할 클래스에 정의해 중복을 제거할 수 있다. (receiveCard, isBurst 등)

단점

조합에 비해, 부모 역할 클래스의 내부구현이 변경되면 그대로 자식역할 클래스가 영향을 받는다.

인터페이스에 비해, 다중 상속이 불가능하다

조합

장점

추상클래스에 비해, 코드의 재사용에 유리하다.

추상클래스에 비해, 부모역할 클래스의 내부구현이 변경되어도 영향을 받지 않는다. 오직 인풋-아웃풋이 변경될 때만 자식역할 클래스도 영향을 받는다.

추상클래스에 비해, 런타임에 동적으로 객체를 교체할 수 있다 (런타임 의존성)

인터페이스

장점

추상클래스에 비해, 오직 메세지에만 집중해 정의하고 싶을 때 유리하다.

추상클래스에 비해, 적극적으로 다형성을 사용할 수 있다.

단점

구현체별로 구현내용에 차이가 있는 게 있다면 유지보수가 어려울 수 있다. (ex. 특정 구현체에서만 쓰는 메서드를 전체 인터페이스의 메세지로 강제해야 하거나, 특정 구현체에서만 쓰는 메서드는 따로 사용해야 할때)

인터페이스가 갖고 있는 메세지’만’ 사용할 때 유리하기 때문이다.

위와 같이 비교를 진행했는데요, 결론적으로는 Dealer, Player - Participant 의 관계를 표현하기 위해선 추상클래스가 가장 적합하다고 느꼈습니다!

두 클래스 간의 관계는 단순히 코드 재사용 뿐만 아니라 is-A 관계 표현도 필요하므로, 조합을 통한 코드재사용보다는 상속을 통해 관계를 표현하고 Participant가 변화할때 Dealer, Player까지 변화하도록 할 수 있었습니다.

(아무리 생각해도 Player, Dealer is Participant라는 관계가 성립하는 이상 부모가 변하면 자식도 변하는 게 맞는 것 같아서, 상속의 문제점인 강한 결합의 문제는 느끼기가 어려웠습니다. 이부분에 대해서 제가 못 생각하고 있는 게 있으면 말씀해주세요!!)

또한 인터페이스에서는 불가능한 중복 필드 제거, 중복 구현 메서드 코드 제거가 가능해서 훨씬 더 '중복 제거'라는 목표에 집중할 수 있었습니다.

지금 부모 클래스의 메서드를 공통적으로 사용하고 있는데요.
dealInitialCards()를 예시로 player만 초키 카드를 3장씩 받는 방식으로 변경되면 어떻게 될까요? 🤔

Participant를 추상클래스로 만들었으니, dealInitialCards를 추상메서드로 만들어 Player, Dealer에서 각각 이를 다르게 구현하도록 만들 것 같습니다!

사실 이부분에 대해 이론적으론 완전히 동의하지만,
'상속이면 is-A 관계인데, 부모가 변하면 자식이 변하는 게 당연한 것 아닌가?'라는 생각도 들어서 단점을 실감하기가 어려웠던 것 같아요 🥲
가령 Dealer, Player의 상위인 Participant.receiveCard()의 정책이 바뀌었다면, 이는 곧 딜러와 플레이어가 카드를 뽑는 정책도 바뀌었으니 함께 적용되는 것이 맞지 않나 싶습니다.
오히려 부모의 변경이 자식에게 의도치않게 영향을 준다는 건 상속관계를 잘못 설정한 거 아닐까? (is-A 관계가 아니라던지..) 라는 생각도 듭니다. 여기에 대한 라라의 의견이 궁금합니다!

이론적으로는 "부모가 바뀌면 자식이 바뀌는 게 당연한 것 아닌가?" 라는 생각이 맞을 수도 있지만, 현실적으로는 예상하지 못한 유지보수 문제가 발생하는 경우가 많아요.

1. 부모 변경 → 자식 변경이 당연한가?

Dealer, Player의 상위인 Participant.receiveCard()의 정책이 바뀌었다면, 이는 곧 딜러와 플레이어가 카드를 뽑는 정책도 바뀌었으니 함께 적용되는 것이 맞지 않나 싶습니다.

Participant가 모든 자식에게 완전히 동일한 방식으로 적용되는 개념(즉, is-A 관계)이라면, 부모 변경 → 자식 변경이 자연스러운 흐름이에요. 하지만 자식마다 다르게 적용되어야 할 수도 있다면 상황이 달라질 것 같아요.

public class Participant { public void receiveCard(Card card) { if (isSpecialEvent()) { System.out.println("이벤트 보너스 카드 지급!"); } System.out.println("카드를 한 장 받았습니다."); } }

만약 정책이 바뀌어서 보너스 카드를 지급해야 한다면, Dealer도 보너스 카드를 받아야 하는 문제가 발생하죠.
원래는 플레이어만 보너스 카드를 받아야 했는데, 부모 클래스가 변경되면서 딜러까지 영향을 받게되죠. 이런 부분이 부모-자식간의 높은 강결합을 갖게되는 상속의 단점이라고 생각해요.

2. 상속의 다형성
저는 상속의 가장 큰 장점은 다형성이라고 생각해요.
모다는 주로 Dealer, Player 자식 클래스 타입을 사용하고 있기 때문에 큰 단점을 못느끼실 수도 있을 것 같아요.
상속을 제대로 활용하려면 Participant 타입으로 객체를 다룰 수 있어야하는데요.
이처럼 구현하였을 때, Dealer와 Player 는 예상대로 동작하나요?

Participant participant = new Dealer(); Participant participant = new Player();

Participant 와 Dealer, Player가 is-A 관계가 정말 맞을까요? 🤔
저번 리뷰에 말씀드린 리스코프 치환 원칙(LSP)와 함께 해당 부분을 고민해보시면 좋을 것 같아요. 🙂

현재 코드에서는 다른 코멘트에 남겨둔 것처럼 instanceof, 강제 타입변환을 사용하면서 잠재적 문제를 갖고있는데요.
중복 코드 제거를 위해 상속을 사용하는 의도는 너무 좋은 것 같아요. 👍🏻
하지만 상속을 잘 사용하고 있는지 고민해보시고 구조를 조금만 바꿔보아도 훨씬 좋아질 것 같아요!

Chaeyoung714 · 2025-03-07T07:44:53Z

+public class ParticipantWinningResult {
+    private final Map<Player, GameResult> result;
+
+    public static ParticipantWinningResult of(Players players, Dealer dealer) {


저는 특정 객체를 만들 때, 해당 객체를 만드는 과정이 복잡하다면 정적팩토리메서드를 사용해 객체 생성 로직을 캡슐화합니다.
그런데 이번 미션에서도 많은 클래스에 정적팩토리메서드를 쓰면서, 혹시 팩토리메서드인데 너무 많은 로직이 들어있는 건 아닌가라는 생각이 들었어요.

특히 위 객체의 경우, result 필드는 모든 플레이어에 대해 각자 승무패 결과를 저장하는 맵인데요, Player객체들과 Dealer 객체를 받은 후 이들이 가진 손패를 비교해서 승무패를 결정하는 로직까지 모두 of() 안에 들어있습니다.

이런 경우 단순히 '생성을 캡슐화한다'라는 말로 설명이 안되는 것 같기도 하면서도, 어쨌든 자신의 필드를 규정하는 과정이기 때문에 구체적인 생성 로직은 숨기고 정적팩토리에 넣는것이 적합하지 않을까 하는 생각도 듭니다.

그리고 만약 정적팩토리를 쓰지 않는다면, 객체생성 시점에서는 그냥 빈 map 을 필드에 넣었다가 이후에 다른 메서드를 써서 스스로 값을 갱신할 수도 있는데, 굳이 객체생성 이후에 필드를 변경하는 일도 없도록 만들고 싶었습니다.
혹은 외부에서 결과를 계산해서 맵을 생성한 뒤 ParticipantWinningREsult 생성자에 바로 전달해주는 방법도 있지만, 이역시 객체의 책임이 응집력이 떨어지는 것이라고 생각이 들었습니다.

라라가 보시기에 정적 팩토리 메서드의 역할은 어디까지로 제한해야 하나요? 제가 만든 정적팩토리 메서드는 적절한 역할을 수행중인 것인지 궁금합니다.

정적 팩토리 메서드는 모다의 목적과 더불어 보통 요런 이유들로 사용되는데요.

복잡한 객체 생성을 캡슐화

생성자보다 더 의미 있는 메서드명을 제공하여 객체 생성의 의도를 명확히 하기 위해

객체를 캐싱하거나 같은 인스턴스를 재사용할 수 있도록

여러 방식으로 객체를 만들 때

현재 of()의 역할을 살펴보았을 때, 두가지 역할을 하고 있는데요!

객체 생성 (정적 팩토리 메서드 역할)

비즈니스 로직 수행 (승패 계산)

정적 팩토리 메서드는 객체 생성만을 담당하는 것이 이상적이지만, 현실적으로 객체가 의미 있는 상태를 갖기 위해 필요한 최소한의 로직까지 포함하는 경우가 많긴 한 것 같아요.
현재 of() 메서드도 의미 있는 객체가 되기 위해 필요한 승패 결정 로직까지 포함하고 있는데요. 객체의 상태를 결정하는 최소한의 로직까지 포함하는 것은 크게 문제되진 않는다고 생각해요. 🙂

메서드분리 정도는 진행하여 메서드별 역할을 명확히 하는 것은 좋을 것 같아요!

이해되었습니다!! 객체 생성과 관련된 복합성이 반드시 비즈니스 로직을 의미하는 건 아닐수도 있겠군요.
캐싱이나 여러 방식으로 객체를 만드는 것도 복잡한 로직에 들어갈 수 있겠네요..!
감사합니다!!

sure-why-not

모다 안녕하세요!
리뷰어 라라입니다. 반가워요 😀
블랙잭 미션 진행하느라 고생하셨습니다. 👏🏻
질문에 대한 답변과 몇가지 코멘트 남겼으니 확인 부탁드릴게요~!

sure-why-not · 2025-03-07T16:04:48Z

+        OutputView.printInitialDealResult(dealer, players);
+    }
+
+    private static void printWinningResult(Players players, Dealer dealer) {


메서드들을 static으로 선언하신 이유가 무엇인가요~?

메서드 분리를 인텔리제이 단축키로 자동화할때, 자동으로 static으로 추가해줘서 지우질 못했습니다ㅠㅠ!
생각해보니 메서드를 분리하는 시점에서 인스턴스 변수를 참조하지 않으면 자동으로 static을 붙여주는 것 같아요 (static 메서드로 만들 수 있으니)
그래도 메서드를 객체 상태와 관련없이 쓰는 일을 방지하기 위해 다음부턴 잘 정리해두겠습니다

반영해서 수정완료했습니다!

sure-why-not · 2025-03-07T16:06:43Z

+        boolean flag = true;
+        while (flag == dealer.checkScoreUnderSixteen()) {


flag 변수명이 구체적인 의미가 담기면 좋을 것 같아요. ㅎㅎ
그리고 flag 변수 없이도 가능할 것 같아요.

Suggested change

boolean flag = true;

while (flag == dealer.checkScoreUnderSixteen()) {

while (dealer.checkScoreUnderSixteen()) {

넵 반영했습니다!

sure-why-not · 2025-03-07T16:07:47Z

+        boolean flag = true;
+        while ((flag == InputView.readHit(player))) {


이곳도 동일해요.

Suggested change

boolean flag = true;

while ((flag == InputView.readHit(player))) {

while (InputView.readHit(player)) {

sure-why-not · 2025-03-07T16:09:49Z

+        return Arrays.stream(names).toList();
+    }
+
+    public static boolean readHit(Player player) {


readHeat()은 값을 읽어오고있는 의미인데, 추가로 카드를 받을 것인지를 판단하고 있네요. 🤔

아 블랙잭에서 hit이 한장 더 받는 행위를 칭한다고 해서, 'hit할것인지 여부를 읽어온다'라는 의미로 지었습니다..!

한장 더 카드를 받아서 값을 읽어오라 라는 의미로 읽히려나요 🤔

값을 리턴하는 경우에는 주로 get, find 등을 사용해서 조금 어색하게 느껴지기도 했던 것 같아요.
읽다 라는 의미로 읽혀서 리턴값이 없는 행위를 한다고 느껴졌었는데요.
읽어오다 라는 의미라고 설명해주신 부분도 납득이 가서 그대로 진행하시는 것도 좋을 것 같아요 👍🏻

sure-why-not · 2025-03-07T16:10:02Z

+public class InputView {
+    private static final String YES = "y";
+    private static final String NO = "n";
+    private static Scanner SCANNER = new Scanner(System.in);


Suggested change

private static Scanner SCANNER = new Scanner(System.in);

private static final Scanner SCANNER = new Scanner(System.in);

final로 선언해도 될 것 같아요.

네 반영해두었습니다!

sure-why-not · 2025-03-07T16:35:57Z

+        if (gameResult == GameResult.WIN){
+            dealerResult.merge(GameResult.LOSE, 1, Integer::sum);
+        }
+        if (gameResult == GameResult.LOSE){
+            dealerResult.merge(GameResult.WIN, 1, Integer::sum);
+        }
+        if (gameResult == GameResult.DRAW){
+            dealerResult.merge(GameResult.DRAW, 1, Integer::sum);
+        }


Suggested change

if (gameResult == GameResult.WIN){

dealerResult.merge(GameResult.LOSE, 1, Integer::sum);

}

if (gameResult == GameResult.LOSE){

dealerResult.merge(GameResult.WIN, 1, Integer::sum);

}

if (gameResult == GameResult.DRAW){

dealerResult.merge(GameResult.DRAW, 1, Integer::sum);

}

dealerResult.merge(gameResult, 1, Integer::sum);

요렇게 중복코드를 제거할 수 있을 것 같아요.

네 참고해서 반영해두었습니다!
사실 첫번째랑 두번째 분기문은 gameResult 변수의 값이랑 merge에 넘겨주는 변수의 값이 달라서 메서드 길이가 줄진 않았지만, 중복은 제거했네요..😅

sure-why-not · 2025-03-07T16:47:29Z

+public class ParticipantWinningResult {
+    private final Map<Player, GameResult> result;
+
+    public static ParticipantWinningResult of(Players players, Dealer dealer) {


정적 팩토리 메서드는 모다의 목적과 더불어 보통 요런 이유들로 사용되는데요.

복잡한 객체 생성을 캡슐화

생성자보다 더 의미 있는 메서드명을 제공하여 객체 생성의 의도를 명확히 하기 위해

객체를 캐싱하거나 같은 인스턴스를 재사용할 수 있도록

여러 방식으로 객체를 만들 때

현재 of()의 역할을 살펴보았을 때, 두가지 역할을 하고 있는데요!

객체 생성 (정적 팩토리 메서드 역할)

비즈니스 로직 수행 (승패 계산)

정적 팩토리 메서드는 객체 생성만을 담당하는 것이 이상적이지만, 현실적으로 객체가 의미 있는 상태를 갖기 위해 필요한 최소한의 로직까지 포함하는 경우가 많긴 한 것 같아요.
현재 of() 메서드도 의미 있는 객체가 되기 위해 필요한 승패 결정 로직까지 포함하고 있는데요. 객체의 상태를 결정하는 최소한의 로직까지 포함하는 것은 크게 문제되진 않는다고 생각해요. 🙂

메서드분리 정도는 진행하여 메서드별 역할을 명확히 하는 것은 좋을 것 같아요!

sure-why-not · 2025-03-07T16:52:42Z

+        if (dealerScore < playerScore){
+            return GameResult.WIN;
+        }
+        return GameResult.DRAW;


Enum이 비즈니스 로직을 가져도 되는가? 에 대한 좋은 고민인 것 같아요 👍🏻
요것도 정답이 정해져있다고 생각하진 않아요. 본인만의 근거와 기준을 정하면 될 것 같은데요~

저는 GameResult 가 승패를 나타내는 값이기 때문에, 자신을 생성하는 로직을 가지는 것이 자연스럽다고 생각해요. 🙂

public static GameResult fromScores(int dealerScore, int playerScore) { if (playerScore > dealerScore) { return WIN; } if (playerScore < dealerScore) { return LOSE; } return DRAW; }

sure-why-not · 2025-03-07T17:04:04Z

+    }
+
+    public Card getFirstHand() {
+        return getParticipantHand().getCards().getFirst();


답변에 앞서 먼저 모다에게 질문을 드리고 싶습니다!

추상화하는 방법으로 인터페이스도 존재하는데 상속을 선택하신 이유가 무엇인가요?

둘의 장/단점 차이는 무엇이 있을까요?

현재 대부분의 기능 구현이 부모 클래스에 구현되어 있기 때문에, 부모의 변경이 자식에게 의도치 않은 영향을 줄 가능성이 있다고 생각합니다.
그리고 객체지향의 SOLID 원칙 키워드를 학습하시고,
해당 상속 구조가 리스코프 치환 원칙(LSP)을 잘 지키는 구조인지 고민해보셔도 좋을 것 같아요 🙂

sure-why-not · 2025-03-07T17:11:52Z

+    }
+
+    public Card getFirstHand() {
+        return getParticipantHand().getCards().getFirst();


저는 다양한 방법을 시도해보고 직접 장/단점을 느껴보고 본인만의 기준을 만들어가는 것을 추천해요. 🙂
미션을 기회로 추상클래스/인터페이스 다양하게 시도해보고 생각만 해본 장/단점을 직접 느껴보시는건 어떨까요? 후기도 궁금하네요 😀

라라의 시선에서 봤을땐 이 클래스가 가진 위험성이 있는지 궁금합니다.

지금 부모 클래스의 메서드를 공통적으로 사용하고 있는데요.
dealInitialCards()를 예시로 player만 초키 카드를 3장씩 받는 방식으로 변경되면 어떻게 될까요? 🤔

Chaeyoung714

안녕하세요 라라!
좋은 코멘트 정말 감사합니다 :)
피드백해주신 부분 반영해서 수정하고, 추상클래스로 중복 제거를 시도해서 좀더 개선을 해보았습니다.
답글 한번 확인해주시면 감사하겠습니다!!

Chaeyoung714 · 2025-03-09T04:33:28Z

+    }
+
+    public Card getFirstHand() {
+        return getParticipantHand().getCards().getFirst();


추상화하는 방법으로 인터페이스도 존재하는데 상속을 선택하신 이유가 무엇인가요?

일단 인터페이스와 상속은 목적이 다르다고 생각했습니다.

인터페이스는 역할만 명시하고, 구현체는 아예 따로 만듦으로써 역할과 구현을 분리하는 것이 목적이라고 생각했어요. 대신 상속은 중복 코드를 제거하는 것이 목적이라고 생각했습니다. 대신 중복이면 다 상속으로 처리하진 않고, 중복인 이유가 자식클래스가 부모클래스와 is-A 관계라서 일부 역할을 공유할 때에만 필요하다고 생각했어요.

그리고 지난번 미션에서 인터페이스를 다양하게 도입해봤는데, 구현체끼리 중복되는 코드가 많은 경우 굳이 구현체 2개에 같은 코드를 복붙하는 점이 비효율적이어 보였습니다.

이번엔 Dealer나 Player와 Participant 간의 is-A 관계가 명확해 보였고, 중복코드 제거가 가장 큰 목적이었기 때문에 자연스럽게 상속이 적합하다고 판단했습니다.

Chaeyoung714 · 2025-03-09T04:46:58Z

+    }
+
+    public Card getFirstHand() {
+        return getParticipantHand().getCards().getFirst();


둘의 장/단점 차이는 무엇이 있을까요?

인터페이스의 경우 주어진 역할이 정확히 같지만 구현 방식이 다른 객체에게 적용했을 때 가장 효과적이라고 생각합니다.
그러한 객체에 인터페이스를 적용하면, 해당 인터페이스를 의존했을 때 쉽게 구현방식을 갈아끼울 수 있다는 장점이 있는 것 같습니다. 이것이 OCP에도 많은 도움이 되는 것 같아요.

다만 인터페이스는 자신의 메서드를 구현할 것을 강제한다는 점에서 단점도 있는 것 같아요.
인터페이스가 강제한 메서드와 역할이 완전히 같지 않다면 구현체 중 일부는 인터페이스에서 강제로 오버라이딩하는 메서드 중 필요없는 메서드가 있을 수 있습니다.
또한 앞서 말했듯 구현체 간 구현방식이 비슷한 경우 중복되는 오버라이딩 코드가 많아져 코드의 효율성이 떨어질 수 있습니다.

상속의 경우 중복을 쉽게 제거할 수 있습니다.
또한 자신이 정의하지 않은 메서드라도(부모 메서드) 쉽게 사용하면서 'is-A 관계면 기능을 사용 가능하다'는 점이 명확하게 드러납니다.

다만 부모와의 강결합이 여러모로 문제가 되는 것 같습니다!
부모 메서드가 바뀌면 의도치않고 자식 메서드에도 영향을 줍니다.
또 자식 메서드에서 부모 메서드를 오버라이딩해 사용한다면 부모메서드가 변경되었을 때 자식메서드는 이를 아예 알지 못한다는 단점도 있는 것 같아요

Chaeyoung714 · 2025-03-09T04:50:47Z

+    }
+
+    public Card getFirstHand() {
+        return getParticipantHand().getCards().getFirst();


현재 대부분의 기능 구현이 부모 클래스에 구현되어 있기 때문에, 부모의 변경이 자식에게 의도치 않은 영향을 줄 가능성이 있다고 생각합니다

사실 이부분에 대해 이론적으론 완전히 동의하지만,
'상속이면 is-A 관계인데, 부모가 변하면 자식이 변하는 게 당연한 것 아닌가?'라는 생각도 들어서 단점을 실감하기가 어려웠던 것 같아요 🥲
가령 Dealer, Player의 상위인 Participant.receiveCard()의 정책이 바뀌었다면, 이는 곧 딜러와 플레이어가 카드를 뽑는 정책도 바뀌었으니 함께 적용되는 것이 맞지 않나 싶습니다.
오히려 부모의 변경이 자식에게 의도치않게 영향을 준다는 건 상속관계를 잘못 설정한 거 아닐까? (is-A 관계가 아니라던지..) 라는 생각도 듭니다. 여기에 대한 라라의 의견이 궁금합니다!

객체지향의 SOLID 원칙 키워드를 학습하시고,
해당 상속 구조가 리스코프 치환 원칙(LSP)을 잘 지키는 구조인지 고민해보셔도 좋을 것 같아요 🙂

좋은 학습키워드 감사합니다!
라라의 질문에 답을 쓰면서도 제 스스로 확신이 없는 부분도 많아서, 객체지향을 다시 공부해봐야겠다는 필요성이 드네요..!!

Chaeyoung714 · 2025-03-09T04:55:27Z

+public class Deck {
+    private final List<Card> deck;
+
+    public static Deck of() {


다른 질문에서도 언급된 내용이긴 한데,
기본 생성자 안에는 생성자한테 기대하는 '객체 생성'이라는 로직만 두는 게 맞다고 생각했습니다!
생성자에게 넘겨준 파라미터 그대로 반영해서 객체가 생성될 것이라 보통 예측하는데, 그외의 로직이 섞이면 당황스러울 수 있으니까요.

하지만 정적팩토리메서드는 그 특성상 그안에 생성 외의 로직이 있다는 게 예측 가능하기 때문에 더욱 적합하다고 생각했어요!

이 메서드같은 경우는 단순 생성이 아닌 특정 로직에 기반해서 생성을 하는 것이기 때문에
정적팩토리메서드를 써서 마치 List.of()처럼 내부 로직에 따라서 알아서 덱을 생성하라~ 라는 의미를 전달해줄 수 있다고 생각했습니다!

혹시 이에 대해서는 어떻게 생각하시나요?

Chaeyoung714 · 2025-03-09T04:58:40Z

+        return cards.stream()
+                .filter(card -> card.getCardRank() == CardRank.ACE)
+                .mapToInt(Card::getCardRankDefaultValue)
+                .sum();


아, 이부분은 제가 메서드명을 잘못 지은 것 같습니다!

ace 점수값을 계산하는데, ace 점수를 1로 고정하고 계산을 하기 때문에 ace의 개수를 세어도 되겠다 싶어서 calculateAceCount() 메서드명을 짓고 라라가 적어주신대로 count를 썼었는데요,

만약 나중에 점수가 1점이 아닌 다른 값으로 변경되면 결과값에 오류가 생기기 때문에 (ace 개수 * ace의 점수(1점))을 계산해서 ace의 totalScore를 계산한다는 의미의 메서드로 변경이 되었습니다.

그런데 이에 맞춰서 메서드명을 수정하지 못했네요..!! ㅠㅠ 수정해보겠습니다!

Chaeyoung714 · 2025-03-09T10:08:13Z

+    }
+
+    public List<Card> getCards() {
+        return cards;


아니요! 이부분을 못봤더라구요ㅠㅠ 다른 곳도 확인하면서 collections.unmodifiable~~ 로 바꿀 수 있는 것은 모두 변환해 수정을 막았습니다.

Chaeyoung714 · 2025-03-09T10:08:26Z

+    DIAMOND("다이아몬드"),
+    CLOVER("클로버");
+
+    private String shapeMeaning;


네 반영해두었습니다!

Chaeyoung714 · 2025-03-09T10:09:39Z

+        if (gameResult == GameResult.WIN){
+            dealerResult.merge(GameResult.LOSE, 1, Integer::sum);
+        }
+        if (gameResult == GameResult.LOSE){
+            dealerResult.merge(GameResult.WIN, 1, Integer::sum);
+        }
+        if (gameResult == GameResult.DRAW){
+            dealerResult.merge(GameResult.DRAW, 1, Integer::sum);
+        }


네 참고해서 반영해두었습니다!
사실 첫번째랑 두번째 분기문은 gameResult 변수의 값이랑 merge에 넘겨주는 변수의 값이 달라서 메서드 길이가 줄진 않았지만, 중복은 제거했네요..😅

Chaeyoung714 · 2025-03-10T06:31:06Z

+    }
+
+    public Card getFirstHand() {
+        return getParticipantHand().getCards().getFirst();


동의합니다! 그래서 저는 추상클래스로 시도를 해봤는데요, 시도하고 장단점을 적으면서 답을 잘 못하는 부분은 추가로 학습을 해보고 정리해보려 합니다
~~이부분은 따로 디엠으로 답장드려도 될까요?~~ 아래 비교정리한 내용 공유드립니다!

Chaeyoung714 · 2025-03-10T06:33:38Z

+public class ParticipantWinningResult {
+    private final Map<Player, GameResult> result;
+
+    public static ParticipantWinningResult of(Players players, Dealer dealer) {


이해되었습니다!! 객체 생성과 관련된 복합성이 반드시 비즈니스 로직을 의미하는 건 아닐수도 있겠군요.
캐싱이나 여러 방식으로 객체를 만드는 것도 복잡한 로직에 들어갈 수 있겠네요..!
감사합니다!!

sure-why-not

모다 안녕하세요! 😀
전체적으로 리뷰 반영 잘해주셨네요. 💯
질문에 대한 답변과 몇가지 코멘트 남겨두었으니 확인 부탁드려요~!

sure-why-not · 2025-03-11T02:57:30Z

+        if (gameResult == GameResult.WIN){
+            updateDealerResult(GameResult.LOSE, dealerResult);
+        }
+        if (gameResult == GameResult.LOSE){
+            updateDealerResult(GameResult.WIN, dealerResult);
+        }
+        if (gameResult == GameResult.DRAW){
+            updateDealerResult(GameResult.DRAW, dealerResult);
+        }


이전 리뷰에서 제가 LOSE <-> WIN 부분을 놓쳤군요. ㅎㅎ

gameResult == GameResult.DRAW 값을 직접 비교하는 것이 아니라, gameResult.isDraw() 와 같이 메세지를 던져 캡슐화를 강화할 수 있을 것 같아요.

반대 결과를 GameResult 에게 위임하고 이렇게 단축시킬 수 있을 것 같아요.

Suggested change

if (gameResult == GameResult.WIN){

updateDealerResult(GameResult.LOSE, dealerResult);

}

if (gameResult == GameResult.LOSE){

updateDealerResult(GameResult.WIN, dealerResult);

}

if (gameResult == GameResult.DRAW){

updateDealerResult(GameResult.DRAW, dealerResult);

}

updateDealerResult(gameResult.reverse(), dealerResult);

네 이해했습니다! gameResult.reverse를 만들어서 일부책임을 GameResult에게 넘겨주는 것 정말 좋네요 ㅎㅎ 반영 완료했습니다!!

sure-why-not · 2025-03-11T02:55:58Z

+    }
+
+    public String getCardName() {
+        return cardRank.getName() + cardSuit.getShapeMeaning();


OutputView 에서 사용되는 출력 형식을 도메인에서 관리하고 있네요.
출력 형식이 변경되면 도메인에게 영향이 전파될 것 같아요. 🤔

오..! 그렇게 볼 수도 있을 것 같습니다.
전에도 비슷한 고민을 한 적이 있는데, 개인적으로는 cardName 자체를 카드가 가진 하나의 특징으로 사용할 수 있지 않을까 싶어서 도메인에 가깝게 두었어요.
'카드번호가 3이고 도형이 스페이스인 카드를 3스페이스라고 부른다'라는 책임을 충분히 도메인에 둘 수 있고, 나중에 출력이 아닌 곳에서도 사용할 수 있지 않을까 생각이 들었습니다.

그래서 도메인에 두고, 나중에 출력형식이 바뀌면 OutputView에서 이를 가져와서 수정하는 게 맞지 않을까 싶었는데, 이에 대해선 어떻게 생각하시나요?

+추가의견) tooString()으로 표현하는 건 어떻게 생각하시나요?
리팩토링때 생각한 점인데, Card에 무엇이 들어갔는지 보기 위해 각 필드가 가진 대표적인 밸류값을 반환하도록 하면 디버깅도 충분히 되고, 출력에도 유연하게 사용할 수 있을 것 같습니다.

toString()은 객체를 문자열로 표현하는 오버라이드 메서드로, 주로 로깅이나 디버깅 용도로 사용되는 편이에요. ㅎㅎ
도메인에서 사용되는 것이라면 문제 없을 것 같은데 출력용으로 사용되고 있어서요.
만약 3스페이스를 3-스페이스 와 같은 형태로 출력 형식이 변경되면 도메인에도 영향이 있을 것 같아요.
OutputView에서 Name + Shape를 출력 형식에 맞게 조합해서 사용하면 출력 형식이 변경되어도 OutputView에만 영향이 가지 않을까요?🙂

sure-why-not · 2025-03-11T02:56:08Z

+        if (maxValue == 0) {
+            throw new IllegalStateException("최대 값을 선택할 수 없는 카드입니다.");
+        }


maxValue 에 임의값을 넣어두고, 사용할수 없는 값처럼 활용하는 것이 어색한 것 같아요.
임의값을 넣어두고 이렇게 관리한다면, 추후 어떤 CardRank의 maxValue가 실제로 0으로 변경될 경우 이 곳도 함께 관리되어야합니다.
ACE 를 제외하고는 maxValue가 존재하지 않는 것이기 때문에 null로 존재하는게 맞지 않을까요?
테스트도 존재하지 않네요. 🥲

앗 테스트는 보충해두었습니다 😢

null로 존재해야 하는 점은 이해했습니다!

다만 int 변수에 null을 대입하면 컴파일에러가 날 뿐더러 (int에 null이 들어가지 못해서, 0으로 초기화해야 합니다) npe가 실수로 날 수 있으므로
카드의 value가 될 수 없는 -1로 표현하면 좋을 것 같아서 -1로 초기화하도록 수정했습니다!

이렇게 되면 -1이 null을 대체한다는 의미도 충분히 갖고 있고, 실제로 카드값이 0을 가지는 경우도 대비할 수 있을 것 같아요.

sure-why-not · 2025-03-11T02:57:01Z

+                .filter(participant -> participant instanceof Player)
+                .map(participant -> (Player) participant)


상속을 사용하셨지만, 다형성이 활용되지 못하고 있네요.
instanceof와 강제타입캐스팅의 문제점은 무엇이 있을까요?

일단 강제타입캐스팅은 컴파일에러로 못잡는 게 가장 안좋은 문제 같습니다!

또한 instanceOf는 부모가 자식을 알아야 한다는 것이 문제인 것 같아요.
즉, 부모가 자식 구현체 각각에 대해서 알고 매번 따로 정의를 해줘야 합니다.
지금은 participant instanceOf Dealer, participant instanceOf Player에 대해서만 정의했지만, 후에 수백개의 자식클래스가 생기면 이를 일일이 알고 처리해야 한다는 점에서 다형성을 활용하지 못한 것 같아요.

리팩토링하고 다른 크루들이랑 얘기해보면서 instanceOf와 강제타입캐스팅을 쓴 것을 어떻게 해결해야 할지 생각해보았습니다!

제가 생각한 가능한 원인은 'Dealer는 완전히 Participant는 아니다'라는 것이었습니다.
원래 Dealer는 Participant다 라는 관계에 의심이 없었는데, 다시 따져보니 Dealer는 '일부만 Participant다'라는 생각이 들었습니다.
즉, Player is Participant는 자명하지만 Dealer is Participant라고 하기엔 딜러가 가지고 있는 고유한 역할 (카드를 분배하는 역할 등) 이 있어서 완전히 is 관계로 보기 어려웠습니다.

이에 대해 두가지정도 해결책이 있었습니다.

1. 애초에 Dealer와 Player가 Participant로 묶지 않는다 (is-A로 상속할 관계가 아니다)

둘을 명확하게 같은 역할로 볼수 없으니, 둘을 상속으로 묶지 않습니다.
대신 조합을 활용할 수 있습니다. (Dealer와 Player가 각각 ParticipantHand를 들고 있는 것으로)

2. Participant로 묶을 순 있으나, 매번 다형성을 활용하진 않는다

직전 코드에선 최대한 모든 로직에서 Dealer와 Player를 Participant로 묶어서 다형성을 활용했는데,
라라의 리뷰를 바탕으로 다시 보니 캐스팅이나 instanceOf를 사용한다는 것 자체가 두 타입의 구현방식이 다른 일부 로직에서는 타입을 구분해서 사용해야 했었다는 증거라고 느꼈습니다.

상속 관계를 유지하되, Participant라는 타입으로 항상 다형적으로 사용하진 않고, Dealer와 Player가 완전히 역할이 겹칠 때에만다형적으로 쓰도록 수정하면 캐스팅이나 instanceOf를 사용하는 문제가 해결될 수 있었습니다.

이번에는 2번 방식을 써서 문제를 해결했습니다.
다만 조합이 Dealer와 Player가 가진 본질적인 차이를 무시하지 않는 방법이라고 생각이 들고, 이는 step2가 되면 더욱 명확해질 것 같습니다.

따라서 step2에선 상속관계를 없애고 조합으로 관계를 구현할 예정입니다.

이러한 결론에 대한 라라의 생각이 궁금합니다!

고민 내용이 좋은 것 같아요. 정리도 잘 해주셨네요.👍
저는 1단계에서는 상속을 사용해주셨으니, 이번에는 조합에 대해서도 학습해보시고 조합의 장단점과 상속/조합의 비교점을 직접 느껴보시면 좋을 것 같아요.
저는 A,B가 고민된다면 두가지 모두 경험해보고, 직접 장단점을 느껴보고, 본인만의 기준을 만들어가는 것을 추천하는 편이에요. ㅎㅎ

sure-why-not · 2025-03-11T02:57:17Z

+    }
+
+    @Override
+    public boolean canHit() {


테스트가 없네요. 🧐

반영했습니다!

sure-why-not · 2025-03-11T02:57:14Z

+                .toList();
+    }
+
+    public Dealer getDealer() {


테스트가 없네요. 🧐

클래스 리팩토링을 하면서 수정되었습니다!

sure-why-not · 2025-03-11T02:59:48Z

+        return Arrays.stream(names).toList();
+    }
+
+    public static boolean readHit(Player player) {


값을 리턴하는 경우에는 주로 get, find 등을 사용해서 조금 어색하게 느껴지기도 했던 것 같아요.
읽다 라는 의미로 읽혀서 리턴값이 없는 행위를 한다고 느껴졌었는데요.
읽어오다 라는 의미라고 설명해주신 부분도 납득이 가서 그대로 진행하시는 것도 좋을 것 같아요 👍🏻

sure-why-not · 2025-03-11T03:00:12Z

+        for (Player player : players) {
+            player.dealInitialCards(deck);
+        }


단순 for문은 사실 취향차이라서, for문으로 사용해도 문제는 없습니다!
리뷰들은 제안일 뿐이니 항상 반영하지 않아도 되고 모다가 동의하는 리뷰들만 반영해주시면 됩니다. ㅎㅎ

sure-why-not · 2025-03-11T03:02:30Z

+public class Deck {
+    private final List<Card> deck;
+
+    public static Deck of() {


#780 (comment)

저도 해당 답변에서 말씀드린 것처럼 생성되기 위한 필수 로직들을 포함한 생성자를 정적팩토리로 제공하는 것에 동의해요. ㅎㅎ

sure-why-not · 2025-03-11T03:47:32Z

+    }
+
+    public Card getFirstHand() {
+        return getParticipantHand().getCards().getFirst();


사실 이부분에 대해 이론적으론 완전히 동의하지만,
'상속이면 is-A 관계인데, 부모가 변하면 자식이 변하는 게 당연한 것 아닌가?'라는 생각도 들어서 단점을 실감하기가 어려웠던 것 같아요 🥲
가령 Dealer, Player의 상위인 Participant.receiveCard()의 정책이 바뀌었다면, 이는 곧 딜러와 플레이어가 카드를 뽑는 정책도 바뀌었으니 함께 적용되는 것이 맞지 않나 싶습니다.
오히려 부모의 변경이 자식에게 의도치않게 영향을 준다는 건 상속관계를 잘못 설정한 거 아닐까? (is-A 관계가 아니라던지..) 라는 생각도 듭니다. 여기에 대한 라라의 의견이 궁금합니다!

이론적으로는 "부모가 바뀌면 자식이 바뀌는 게 당연한 것 아닌가?" 라는 생각이 맞을 수도 있지만, 현실적으로는 예상하지 못한 유지보수 문제가 발생하는 경우가 많아요.

1. 부모 변경 → 자식 변경이 당연한가?

Dealer, Player의 상위인 Participant.receiveCard()의 정책이 바뀌었다면, 이는 곧 딜러와 플레이어가 카드를 뽑는 정책도 바뀌었으니 함께 적용되는 것이 맞지 않나 싶습니다.

Participant가 모든 자식에게 완전히 동일한 방식으로 적용되는 개념(즉, is-A 관계)이라면, 부모 변경 → 자식 변경이 자연스러운 흐름이에요. 하지만 자식마다 다르게 적용되어야 할 수도 있다면 상황이 달라질 것 같아요.

public class Participant { public void receiveCard(Card card) { if (isSpecialEvent()) { System.out.println("이벤트 보너스 카드 지급!"); } System.out.println("카드를 한 장 받았습니다."); } }

만약 정책이 바뀌어서 보너스 카드를 지급해야 한다면, Dealer도 보너스 카드를 받아야 하는 문제가 발생하죠.
원래는 플레이어만 보너스 카드를 받아야 했는데, 부모 클래스가 변경되면서 딜러까지 영향을 받게되죠. 이런 부분이 부모-자식간의 높은 강결합을 갖게되는 상속의 단점이라고 생각해요.

2. 상속의 다형성
저는 상속의 가장 큰 장점은 다형성이라고 생각해요.
모다는 주로 Dealer, Player 자식 클래스 타입을 사용하고 있기 때문에 큰 단점을 못느끼실 수도 있을 것 같아요.
상속을 제대로 활용하려면 Participant 타입으로 객체를 다룰 수 있어야하는데요.
이처럼 구현하였을 때, Dealer와 Player 는 예상대로 동작하나요?

Participant participant = new Dealer(); Participant participant = new Player();

Participant 와 Dealer, Player가 is-A 관계가 정말 맞을까요? 🤔
저번 리뷰에 말씀드린 리스코프 치환 원칙(LSP)와 함께 해당 부분을 고민해보시면 좋을 것 같아요. 🙂

현재 코드에서는 다른 코멘트에 남겨둔 것처럼 instanceof, 강제 타입변환을 사용하면서 잠재적 문제를 갖고있는데요.
중복 코드 제거를 위해 상속을 사용하는 의도는 너무 좋은 것 같아요. 👍🏻
하지만 상속을 잘 사용하고 있는지 고민해보시고 구조를 조금만 바꿔보아도 훨씬 좋아질 것 같아요!

Chaeyoung714

안녕하세요 라라!
좋은 답변들 감사합니다. 덕분에 상속에 대한 오해나 풀리지 않았던 것들이 많이 해결되었어요!
꼼꼼히 리뷰해주셔서 다시 코드를 잘 보완할 수 있었습니다.
코멘트와 리팩토링 사항 다시한번 확인해주시면 감사하겠습니다! 감사합니다 😊

Chaeyoung714 · 2025-03-11T04:43:13Z

+    }
+
+    public String getCardName() {
+        return cardRank.getName() + cardSuit.getShapeMeaning();


오..! 그렇게 볼 수도 있을 것 같습니다.
전에도 비슷한 고민을 한 적이 있는데, 개인적으로는 cardName 자체를 카드가 가진 하나의 특징으로 사용할 수 있지 않을까 싶어서 도메인에 가깝게 두었어요.
'카드번호가 3이고 도형이 스페이스인 카드를 3스페이스라고 부른다'라는 책임을 충분히 도메인에 둘 수 있고, 나중에 출력이 아닌 곳에서도 사용할 수 있지 않을까 생각이 들었습니다.

그래서 도메인에 두고, 나중에 출력형식이 바뀌면 OutputView에서 이를 가져와서 수정하는 게 맞지 않을까 싶었는데, 이에 대해선 어떻게 생각하시나요?

+추가의견) tooString()으로 표현하는 건 어떻게 생각하시나요?
리팩토링때 생각한 점인데, Card에 무엇이 들어갔는지 보기 위해 각 필드가 가진 대표적인 밸류값을 반환하도록 하면 디버깅도 충분히 되고, 출력에도 유연하게 사용할 수 있을 것 같습니다.

Chaeyoung714 · 2025-03-11T04:53:12Z

+import java.util.List;
+
+public final class Deck {
+    private final List<Card> deck;


좋은 지적 감사합니다!!
ArrayList는 삽입삭제가 빈번히 일어날때 유리한 자료구조가 아닐뿐더러,
매번 removeFirst를 해서 O(N)의 시간이 걸리고 있던 걸 생각 못했네요 😢

LinkedList를 사용해서 리팩토링해보았습니다!

Chaeyoung714 · 2025-03-11T05:11:01Z

+                .filter(participant -> participant instanceof Player)
+                .map(participant -> (Player) participant)


일단 강제타입캐스팅은 컴파일에러로 못잡는 게 가장 안좋은 문제 같습니다!

또한 instanceOf는 부모가 자식을 알아야 한다는 것이 문제인 것 같아요.
즉, 부모가 자식 구현체 각각에 대해서 알고 매번 따로 정의를 해줘야 합니다.
지금은 participant instanceOf Dealer, participant instanceOf Player에 대해서만 정의했지만, 후에 수백개의 자식클래스가 생기면 이를 일일이 알고 처리해야 한다는 점에서 다형성을 활용하지 못한 것 같아요.

Chaeyoung714 · 2025-03-12T06:06:32Z

+    }
+
+    @Override
+    public boolean canHit() {


반영했습니다!

Chaeyoung714 · 2025-03-12T06:07:08Z

@@ -0,0 +1,42 @@
+package model;


반영완료했습니다!

Chaeyoung714 · 2025-03-12T06:18:18Z

+        if (maxValue == 0) {
+            throw new IllegalStateException("최대 값을 선택할 수 없는 카드입니다.");
+        }


앗 테스트는 보충해두었습니다 😢

null로 존재해야 하는 점은 이해했습니다!

다만 int 변수에 null을 대입하면 컴파일에러가 날 뿐더러 (int에 null이 들어가지 못해서, 0으로 초기화해야 합니다) npe가 실수로 날 수 있으므로
카드의 value가 될 수 없는 -1로 표현하면 좋을 것 같아서 -1로 초기화하도록 수정했습니다!

이렇게 되면 -1이 null을 대체한다는 의미도 충분히 갖고 있고, 실제로 카드값이 0을 가지는 경우도 대비할 수 있을 것 같아요.

Chaeyoung714 · 2025-03-12T06:22:38Z

+        if (gameResult == GameResult.WIN){
+            updateDealerResult(GameResult.LOSE, dealerResult);
+        }
+        if (gameResult == GameResult.LOSE){
+            updateDealerResult(GameResult.WIN, dealerResult);
+        }
+        if (gameResult == GameResult.DRAW){
+            updateDealerResult(GameResult.DRAW, dealerResult);
+        }


네 이해했습니다! gameResult.reverse를 만들어서 일부책임을 GameResult에게 넘겨주는 것 정말 좋네요 ㅎㅎ 반영 완료했습니다!!

Chaeyoung714 · 2025-03-12T08:55:10Z

+        if (participant instanceof Dealer) {
+            return "딜러";
+        }
+        Player player = (Player) participant;


넵! 반영해보았습니다.

Player와 Dealer의 로직이 다른 경우에는 Participant로 억지로 다형성을 이용하지 않고, Player와 Dealer 타입을 쓰게 수정했어요.

대신 OutputView와 같은 곳에서 Dealer와 Player 타입구분없이 기능을 쓸 수 있는 경우에는 Participant 타입으로 범용적으로 쓸수있도록 했습니다.

Chaeyoung714 · 2025-03-12T09:09:12Z

+                .filter(participant -> participant instanceof Player)
+                .map(participant -> (Player) participant)


리팩토링하고 다른 크루들이랑 얘기해보면서 instanceOf와 강제타입캐스팅을 쓴 것을 어떻게 해결해야 할지 생각해보았습니다!

제가 생각한 가능한 원인은 'Dealer는 완전히 Participant는 아니다'라는 것이었습니다.
원래 Dealer는 Participant다 라는 관계에 의심이 없었는데, 다시 따져보니 Dealer는 '일부만 Participant다'라는 생각이 들었습니다.
즉, Player is Participant는 자명하지만 Dealer is Participant라고 하기엔 딜러가 가지고 있는 고유한 역할 (카드를 분배하는 역할 등) 이 있어서 완전히 is 관계로 보기 어려웠습니다.

이에 대해 두가지정도 해결책이 있었습니다.

1. 애초에 Dealer와 Player가 Participant로 묶지 않는다 (is-A로 상속할 관계가 아니다)

둘을 명확하게 같은 역할로 볼수 없으니, 둘을 상속으로 묶지 않습니다.
대신 조합을 활용할 수 있습니다. (Dealer와 Player가 각각 ParticipantHand를 들고 있는 것으로)

2. Participant로 묶을 순 있으나, 매번 다형성을 활용하진 않는다

직전 코드에선 최대한 모든 로직에서 Dealer와 Player를 Participant로 묶어서 다형성을 활용했는데,
라라의 리뷰를 바탕으로 다시 보니 캐스팅이나 instanceOf를 사용한다는 것 자체가 두 타입의 구현방식이 다른 일부 로직에서는 타입을 구분해서 사용해야 했었다는 증거라고 느꼈습니다.

상속 관계를 유지하되, Participant라는 타입으로 항상 다형적으로 사용하진 않고, Dealer와 Player가 완전히 역할이 겹칠 때에만다형적으로 쓰도록 수정하면 캐스팅이나 instanceOf를 사용하는 문제가 해결될 수 있었습니다.

이번에는 2번 방식을 써서 문제를 해결했습니다.
다만 조합이 Dealer와 Player가 가진 본질적인 차이를 무시하지 않는 방법이라고 생각이 들고, 이는 step2가 되면 더욱 명확해질 것 같습니다.

따라서 step2에선 상속관계를 없애고 조합으로 관계를 구현할 예정입니다.

이러한 결론에 대한 라라의 생각이 궁금합니다!

Chaeyoung714 · 2025-03-12T09:11:37Z

+                .toList();
+    }
+
+    public Dealer getDealer() {


클래스 리팩토링을 하면서 수정되었습니다!

sure-why-not

모다 안녕하세요!
리뷰 반영 잘해주셨네요.💯
질문에 대한 답변 남겨두었으니 확인 부탁드려요!
1단계는 잘 마무리해주셔서 여기서 머지해도 괜찮을 것 같아요.
그럼 2단계도 화이팅입니다~💪🏻💪🏻

Chaeyoung714 added 29 commits March 4, 2025 15:11

feat : 카드 뽑는 동작 구현

4542671

refactor : 덱 세팅 및 덱에서 카드 가져오기 로직 리팩토링

ad6e3ed

refactor : 덱 생성 관련 객체 분리

c848bfe

refactor : Deck 클래스 수정에 따른 테스트 수정

3fc0572

docs : 카드 기능 구현 완료 체크

51038a7

feat : 뽑은 카드의 합 계산 기능 구현

42eaf14

feat : 딜러의 카드 점수 합이 16 이하인지 확인하는 기능 구현

0b26b65

feat : 카드의 합이 21 초과인지 확인 기능 구현

7e4cb4e

feat : 받은 카드 중에 ace가 포함되었을 경우 버스트 계산 로직 구현

3475ec5

feat : 플레이어와 딜러의 점수 비교 기능 구현

a6dac29

feat : ACE를 뺸 나머지의 합이 10이하인지 체크하는 기능 구현

4ac56dc

feat : 최종 점수 계산 기능 구현

9a74e09

feat : 딜러의 승패 계산 기능 구현

c79f54d

feat : 플레이어 간 중복 이름 예외 처리

1c943bc

feat : 입력 뷰 로직 구현

22730ee

feat : 초기 카드 배분, 게임 진행, 결과 출력 기능 구현

4cee1f1

feat : 승패 결과 출력 기능 구현

8eb43d9

refactor : 최종 결과 구하는 로직 v2 구현

9bc2be9

refactor : 최종 결과 구하는 로직 v3 구현

7a9b42d

test : judge 테스트 수정

8ff2bcd

refactor : 메서드명 수정, 외부에서 사용하지 않는 메서드 private로 변경

b69d277

refactor : Participant 상속을 통한 Dealer-Player 간 중복 제거

99a6dd5

refactor : 디미터의 규칙에 맞게 리팩토링

532fc0e

refactor : 상수화 진행

eb9c5e9

refactor : ace의 초기값과 최대값 모두 CardRank에서 관리하도록 리팩토링

71c7497

refactor : 메서드 분리 및 메서드명 변경

13fd85a

refactor : 단위 테스트 추가 및 사용하지 않는 클래스 삭제

7ab074f

refactor : 코드 포매팅 및 테스트 중복 코드 제거

76c77c0

docs : readme 체크리스트 체크

2b80d1a

Chaeyoung714 commented Mar 7, 2025

View reviewed changes

sure-why-not requested changes Mar 7, 2025

View reviewed changes

Chaeyoung714 added 14 commits March 9, 2025 14:37

refactor : 중복 코드 제거

4be42ab

refactor : for문을 stream으로 리팩토링

3507846

feat : 예외 발생 시 에러메세지 출력하는 로직 추가

e2dcbdc

refactor : 메서드명 수정, 컬렉션 get 시 수정 불가능하도록 추가

1af3e22

feat : 파라미터 및 클래스의 final 불변 보장 추가

d1bf3cf

refactor : 패키지 구체화

f368cb3

experiment(Participant) : abstract 클래스로 바꾸었을 때 장단점 비교

edb2954

feat : 도메인 검증 로직 추가

83c7886

refactor : 불필요한 flag 변숫 삭제

7afbf9b

refactor : Players -> Participants(Dealer&Players)로 리팩토링

515ed28

test : Participants 관련 테스트 리팩토링

e6a8df5

remove : 불필요한 코드 삭제

91365d3

refactor : 추상클래스의 상속관계 정리

a79f271

feat : final 변수 추가

10c9edb

Chaeyoung714 commented Mar 10, 2025

View reviewed changes

sure-why-not requested changes Mar 11, 2025

View reviewed changes

Chaeyoung714 commented Mar 12, 2025

View reviewed changes

sure-why-not approved these changes Mar 13, 2025

View reviewed changes

sure-why-not merged commit 7b83af8 into woowacourse:chaeyoung714 Mar 13, 2025

		boolean flag = true;
		while (flag == dealer.checkScoreUnderSixteen()) {

	boolean flag = true;
	while (flag == dealer.checkScoreUnderSixteen()) {
	while (dealer.checkScoreUnderSixteen()) {

		boolean flag = true;
		while ((flag == InputView.readHit(player))) {

	boolean flag = true;
	while ((flag == InputView.readHit(player))) {
	while (InputView.readHit(player)) {

	private static Scanner SCANNER = new Scanner(System.in);
	private static final Scanner SCANNER = new Scanner(System.in);

		.filter(participant -> participant instanceof Player)
		.map(participant -> (Player) participant)

Conversation

Chaeyoung714 commented Mar 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

체크 리스트

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

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

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

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

Uh oh!

Chaeyoung714 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Chaeyoung714 Mar 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

추상클래스(상속)

조합

인터페이스

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sure-why-not left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Chaeyoung714 commented Mar 7, 2025 •

edited

Loading

Chaeyoung714 Mar 10, 2025 •

edited

Loading