[🚀 사이클2 - 미션 (블랙잭 베팅)] 제이콥 미션 제출합니다.

[khcho96]

체크 리스트

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

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

사이클1 마지막 코멘트에서 반영한 내용

1. 말씀주신대로 Participants가 단순히 Dealer와 Players를 꺼내기만 하는 것이 아니라 Dealer와 Players에게 명령을 수행하거나 다루는 방향으로 책임을 부여해봤습니다.
2. Result와 HitOrStand 클래스를 domain으로 옮겼습니다.
3. Hand에 10이라는 매직넘버를 상수화했습니다.
4. Hand에 점수계산 로직을 메서드로 추출했습니다.

질문

1. 의도된 중복에 대한 질문

중복을 제거하라 하지만 떄로는 의도된 중복을 만들기도 합니다 😄

의도된 중복을 만들기도 한다는 사실은 충격적이네요 😮
"중복"에 대해 100% 부정적으로 바라봤었는데, 그렇지 않을 수도 있다는 사실을 인지하고 유연하게 생각해봐야겠습니다!
한 가지 궁금한 점이 있는데, 찰리는 constant 패키지를 사용하시는 편인가요?
만약 사용하신다면 어떤 경우에, 어떤 클래스를 공통으로 관리하는 constant 패키지로 분리하시나요?

2. 책임 분리에 대한 질문

BlackjackGameManager 가 승패에 대한 판단을 하는것은 책임이 과할것 같네요 🤔 승패 판단을 하는 객체를 분리해보는것도 좋겠어요~

제가 생각했을 때 현재 승무패 판단을 하는 메서드는 BlackjackGameManager 클래스의 getBlackjackStatistics 메서드 로직인데, 특히 해당 메서드 내 아래 3줄의 로직인 듯 합니다.

Result result = judgePlayerResult(participants.dealer(), player);
int playerProfit = calculatePlayerProfit(player, result);
dealerProfit += playerProfit * -1;

이 로직을 가지는 객체(A라고 지칭하겠습니다)를 따로 만든다면 A는 judgePlayerResult 메서드와 calculatePlayerProfit 메서드를 클래스 내에서 가질텐데, 이때 보시는 것처럼 메서드의 인수로 Dealer와 Player가 들어가기때문에 A가 Dealer와 Player에게 의존하게 되고, 결합도가 커지게 되지 않을까라는 부분이 염려됩니다. 결합도가 커지면 수정범위가 커지게 되는 단점이 있다고 생각합니다.

이 부분에 대한 찰리의 의견이 궁금합니다!

3. 원시값 포장에 대한 질문

Rank 클래스에서 score의 원시값 포장에 대해 말씀해주셨는데, 단순히 상수를 저장하고 있고 블랙잭 점수 규칙에 대한 내용은 다른 객체에서 메서드로 구현했기때문에 score에서 어떤 추가적인 규칙을 검증하거나 다룰지 아이디어가 떠오르지 않네요😂
혹시 찰리 입장에서 score를 원시값 포장한다면 어떤식으로 구현하실지 궁금합니다!

1차 피드백 수정 요소

1. 딜러가 블랙잭인 경우 고려해서 승패 로직 수정 구현
2. Participant에 getName 메서드 추상화
3. 베팅금액을 다 입력받은 후 플레이어를 생성하도록 수정 -> Player의 BetAmount이 참조값을 null로 가지지 않도록 안정성을 높여줌
4. PlayerName도 컨트롤러에서 생성(검증 포함)해서 넘기도록 수정
5. BetAmount의 isPositive 메서드 isNotPositive로 메서드명 수정
6. 플레이어가 카드를 뽑아 버스트가 되면 다음 차례로 넘어가도록 수정
7. Hand의 calculateAceCount 수정
8. OutputView 안쓰는 메서드 삭제
9. 결과에 대한 배당을 BlackjackGameManager의 calculatePlayerProfit 메서드 삭제 및 해당 로직에 대한 책임을 Result가 가지도록 수정
10. 승패 및 수익 계산 로직 객체 분리 및 승패판단 로직 수정 -> BlackjackJudge

 1. 플레이어가 버스트 -> 플레이어 LOSE
 2. 플레이어가 블랙잭(전제: 플레이어는 버스트 아님) && 딜러가 블랙잭이면 -> DRAW
 3. 플레이어가 블랙잭(전제: 플레이어는 버스트 아님) && 딜러가 블랙잭아니면 -> 플레이어 BLACKJACK_WIN
 4. 딜러가 버스트(전제: 플레이어는 버스트 또는 블랙잭 아님) -> 플레이어 WIN
 5. 플레이어 점수 > 딜러 점수(전제: 플레이어와 딜러는 버스트 또는 블랙잭 아님) -> 플레이어 WIN
 6. 플레이어 점수 == 딜러 점수(전제: 플레이어와 딜러는 버스트 또는 블랙잭 아님) -> DRAW
 7. 1~6 아님 -> 플레이어 LOSE

11. record 클래스는 equals, hashCode 메서드 자동 구현이므로 Card의 equals 삭제

2차 피드백 수정 요소

1. dto 테스트 삭제
2. Hand의 isBlackjack() 메서드 수정 및 calculateInitialScore, calculateInitialBaseScore 메서드 삭제
3. BlackjackGameManager의 필드(CardMachine, BlackjackJudge)값 의존성 주입으로 구현 -> BlackjackFactory로 이름 수정
4. BlackjackController가 BlackjackGameManager를 필드로 가지지 않고 메서드들의 파라미터로 두도록 수정

3차 피드백 수정 요소

1. dto 테스트 코드 삭제
2. BlackjackGameManager의 Participants 필드값을 불변으로 지정하고 생성자에서 값이 할당되도록 수정
3. BlackjackJudge의 승패판단 메서드에서 분기 코드 중복 제거
4. 컨트롤러의 start 메서드 내에서 BlackjackGameManager를 생성하도록 수정
5. BlackjackGameManager를 생성하는 BlackjackGameManagerFactory 클래스 생성 및 도메인 패키지에 위치
6. Participants, Players의 playerIsBust 메서드 isPlayerBust로 메서드명 수정

4차 피드백 수정 요소

1. BlackjackJudge클래스의 judgePlayerResult 메서드에 딜러가 블랙잭이고 플레이어가 버스트가 아닌 21점인 경우 추가
2. BlackjackJudge클래스의 judgePlayerResult 메서드에 있는 여러 개의 분기문 중 버스트나 블랙잭인 경우가 아닌 일반적인 점수 비교를 통한 승패 결정 케이스를 judgeScore 메서드로 분리
3. BetAmount 베팅 금액 범위 검증 로직 추가
4. drawCard 두 번 호출하는 메서드를 drawInitialCards 메서드로 수정

[Gomding]

안녕하세요 제이콥!
더 반영하고 싶은것이 있다고 요청하셔서 머지는 보류할게요~
논의 필요한 부분 있으면 DM 이나 코멘트 남겨주세요! 🙇

[Gomding]

요건 아직 검증이 없네요~

[Gomding]

BlackjackJudge 클래스는 final 로 만들어주셨는데 어떤 의도로 만들어주신걸까요? 😄

[Gomding]

BlackjackJudge 는 객체 생성 없이 static 메서드 만을 가지는 util 클래스로 사용해도 될 것 같은데
해당 부분도 고민해보셨을까요?

[Gomding]

사용자 입력 화면에서는 금액 입력 범위를 모르니
처음 사용하는 사용자는 당황할 수 있겠네요 :)
InputView 에서 사용자가 미리 금액 범위를 알 수 있게 설명해주는게 좋겠어요~

[Gomding]

👍

[Gomding]

가독성을 위해서
if 문을 3개로만 줄여보는것도 방법일 것 같아요
각 메서드안에 어떤 상황에 패배로 판단되는지 모아두면 명확하게 확인할 수 있다는 장점이 있어요

if (isLose(dealer, player)) {
    return PlayerResult.LOSE;
}
if (IsWin(dealer, player)) {
    return PlayerResult.WIN;
}
if (IsBlackjackWin(dealer, player)) {
    return PlayerResult.BLACKJACK_WIN;
}
if (isDraw(dealer, player)) {
    return PlayerResult.DRAW;
}

[Gomding]

다음 미션에서는 커밋을 실행가능한 단위로 나눠보는것도 도전해보시면 좋겠어요 :)
수정을 했다면 실행을 위한 수정, 테스트 코드 실행 + 수정 등을 묶어서
모든 커밋들이 실행 가능한 상태로 만드는것이죠
실제로 모든 커밋은 실행이 가능해야한다 <-- 이게 커밋의 조건 입니다 :)

버그가 있는데 커밋을 해버렸다면 git commit --amend 기능을 학습해보시고 사용해보세요!

[Gomding]

안녕하세요 제이콥!
미션 진행하느라 고생하셨어요 👍 👍 👍
이미 많은 고민들을 해주셨는데 앞으로 많은 성장을 이루실거라 생각해요~

앞으로의 우테코 생황도 미션도 화이팅입니다! 💪