Skip to content

[1단계 - 블랙잭] 초코칩(권기호) 미션 제출합니다. #617

Merged
Hyunta merged 85 commits intowoowacourse:chocochip101from
Chocochip101:step1
Mar 11, 2024
Merged

[1단계 - 블랙잭] 초코칩(권기호) 미션 제출합니다. #617
Hyunta merged 85 commits intowoowacourse:chocochip101from
Chocochip101:step1

Conversation

@Chocochip101
Copy link

@Chocochip101 Chocochip101 commented Mar 8, 2024

블랙잭

안녕하세요, 아서. 6기 백엔드 초코칩입니다. 이번 미션에서는 블랙잭을 구현했습니다.

부탁

커뮤니케이션 비용을 줄이기 위한 Pn 룰

PR 내용을 보시기 전에 저와 리뷰어님의 커뮤니케이션 비용을 줄이기 위해 Pn 룰을 부탁드리고 싶습니다. Pn룰을 통해 리뷰어님이 저에게 꼭 고쳤으면 하는 리뷰와 그렇지 않은 리뷰를 범위로 나누어 더욱 전달이 효과적으로 이루어질 것 같습니다.

  • [P1]: 꼭 반영해주세요 (Request changes)
  • [P2]: 적극적으로 고려해주세요 (Request changes)
  • [P3]: 웬만하면 반영해 주세요 (Comment)
  • [P4]: 반영해도 좋고 넘어가도 좋습니다 (Approve)
  • [P5]: 그냥 사소한 의견입니다 (Approve)
  • [질문]: 의견에 대한 질문입니다. (Question)

기능 요구사항(step1)

  • 생성 시점에서 카드를 섞는다.
  • 카드를 뽑을 수 있다.
  • 카드가 존재하지 않을 경우, 예외가 발생한다.

플레이어들

  • 승패를 계산해서 반환한다.
    • 딜러는 모든 플레이어와 결과를 계산한다.
    • 플레이어는 딜러와의 결과만을 계산한다.
    • 딜러가 bust되었을 경우
      • 21이하 참가자는 모든 1승
      • bust된 참가자는 패로 기록한다.
    • 딜러가 21 이하인데, 플레이어랑 다른 겨우
      • 참가자는 숫자를 비교하여 21에 가까운 사람이 승리한다.
      • 딜러와 참가자의 수가 같은 경우, 패로 기록한다.
      • bust된 참가자는 패로 기록한다.
    • 딜러와 플레이어의 점수가 같은 경우
      • 블랙잭인 사람이 승리한다.
      • 그래도 무승부일 경우는 무승부로 판정한다.

딜러

  • 참가자가 카드를 뽑는다.
  • 딜러의 손패는 한장만 반환한다.
  • 딜러는 16점 이하면 카드를 추가 지급한다.
    • 16점 비교 로직을 딜러 내에서 관리한다.

플레이어

  • 참가자가 카드를 뽑는다.
  • 참가자가 뽑을 수 있는 상태인지 확인한다.

카드 손패

  • 들고 있는 카드의 전체 점수를 반환한다.
  • 카드는 점수를 반환한다.
    • Ace는 1 또는 11점으로 계산한다.
    • J,Q,K는 10으로 계산한다.
    • Ace는 21보다 클 경우에는 1로, 그렇지 않으면 11로 계산한다.

카드

  • 카드의 기호와 숫자를 반환한다.

입출력

  • 참가자 이름을 입력 받는다.
    • 각 참가자는 ,로 구분한다.
    • 참가자 이름의 중복을 검증한다.
    • 위의 형식에 맞지 않은 이름 형식은 예외가 발생한다.
  • 딜러의 카드는 한 장만 공개한다.
  • 게임 완료 후 승패를 출력한다.
  • 카드 분배 결과를 출력한다.
  • 카드를 추가 지급 받을지 물어본다.
    • 응답은 y/n로만 받을 수 있다.
    • 응답이 n이거나 카드의 합이 21일때까지 물어본다.
    • y/n 여부와 상관없이 현플레이어의 손패를 출력한다.
    • 위의 형식에 맞지 않은 응답은 예외가 발생한다.

리팩터링

  • 플레이어가 딜러로부터 카드를 받도록 리팩터링한다.
    • 딜러는 덱을 필드로 갖는다.
  • 딜러의 이름 필드를 제거한다.
  • protected 필드 직접 접근 문제 해결한다.
  • ParticipantGamer로 변경한다.
  • 불변 리스트로 변환한다.
  • Judge의 테스트를 진행한다.
  • Rank, Suit를 card 패키지로 변경한다.
  • calculateAce 메서드명을 변경한다.
  • BlackjackResult에서 count 대신 getter로 변경한다.
  • BlackjackGame 메서드 분리
  • 호출되는 메서드는 호출하는 메서드 아래에 위치한다.
    • 여러 곳에서 호출될 경우, 마지막 호출 메서드 아래에 위치한다.
    • 순서가 conflict될 경우, 함수 내의 호출 순서로 메서드 위치를 결정한다.

질문

  • IntelliJ의 자동 final 키워드를 이용하여 매개변수에 재할당을 기본적으로 방지하고 있습니다. 그러다보니 매개변수 부분의 길이가 아래와 같이 길어져 매개변수 부분에 개행이 들어가게 됩니다. 아서는 final 키워드에 대한 컨밴션을 어떻게 적용하는지 궁금합니다. 또한 일반 변수에도 final을 붙이는지도 궁금합니다.
  1. 매개변수의 final
public void foo(final int arguementOne, final int arguementTwo, final int arguementThree){
  // Do something
}
  1. 일반 변수의 final
public void foo(){
  final int var1 = boo1();
  final int var2 = boo2();
  final int var3 = boo3();
  final int var4 = boo4();
  // ...
}

  • CardFactory에서 다른 카드도 구현받을 수 있게 설계하고 싶었으나, 결국 getSuitgetRank 때문에 CardFactoryTrumpCard에 종속적이게 되었습니다. 이를 수정할 수 있는 방법이 있을까요?

아쉬운점

  • Player가 draw하는 과정에서 Dealer를 넘기게 했기 때문에 테스트가 많이 힘들었습니다. Dealer가 아닌 Card만 넘겨줬다면, 결합도를 줄여 테스트를 용이하게 할 수 있었을 것 같습니다.

감사합니다.

Kwoun Ki Ho and others added 30 commits March 5, 2024 14:22
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Co-Authored-By: eunjungL <62099953+eunjungL@users.noreply.github.com>
Copy link

@Hyunta Hyunta left a comment

Choose a reason for hiding this comment

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

안녕하세요 초코칩,
블랙잭 미션 잘 구현해주셨네요ㅎㅎ
피드백이 너무 많아져서 나눠서 리뷰해드리도록 하겠습니다.
커멘트 남긴 내용들 확인해보시고 궁금한 내용은 DM이나 커멘트로 남겨주세요.

}

do {
player.draw(dealer);
Copy link

Choose a reason for hiding this comment

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

[P2]
Player가 Dealer를 알 필요가 있을까요?
구조에 대해서 한번 고민을 해보시면 좋을 것 같습니다.

Player, Dealer, Deck 3가지 객체가 서로 관계가 깊어보여요.
Player는 Draw를 할 때 Deck에 접근하기 위해서 Dealer를 거쳐야 합니다.
그럼 그냥 Dealer가 draw()를 해서 바로 Player에게 전달해줄 수 있지 않을까요?

더 나아가서 지금은 BlackJackGame 객체 안에서 입출력 관리 + 게임 로직 관리가 이뤄지고 있다보니 역할이 방대해진 것 같아요.
게임 로직 관리 중에서 Player가 카드를 계속 받을지 말지 결정하는 로직 을 제외하고는 별도의 객체 안에서 관리되어지는게 어떨까 싶습니다.

Copy link
Author

Choose a reason for hiding this comment

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

그럼 그냥 Dealer가 draw()를 해서 바로 Player에게 전달해줄 수 있지 않을까요?

맞습니다. 강한 결합도 때문에 테스트까지 힘들어져서, Player에서 Card를 넘겨받아 draw()할 수 있게 수정했습니다.

더 나아가서 지금은 BlackJackGame 객체 안에서 입출력 관리 + 게임 로직 관리가 이뤄지고 있다보니 역할이 방대해진 것 같아요.
게임 로직 관리 중에서 Player가 카드를 계속 받을지 말지 결정하는 로직 을 제외하고는 별도의 객체 안에서 관리되어지는게 어떨까 싶습니다.

Blackjack의 Dealer, Player 생성과 첫 두 장 분배를 BlackjackInitializer로 분리했습니다.

Copy link

Choose a reason for hiding this comment

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

BlackjackInitializer가 꼭 필요한지 의문이 드네요,
Dealer 클래스에서 Dealer를 생성하고,
Players 클래스에서 Players를 생성하고,
BlackjackGame에서 카드를 배분해줘도 되지 않을까요?

@Hyunta
Copy link

Hyunta commented Mar 9, 2024

안녕하세요 초코칩
제안해주신 Pn 방법으로 리뷰를 남겨드렸는데,
안그래도 얼마전에 저희팀에서 이야기가 나왔었던 주제였어서 반갑게 적용을 해봤습니다.
저는 사용하면서 편하다는 느낌을 받았는데, 익숙치 않아서 매번 리뷰를 할 때마다 1~5까지 보는 점은 좀 불편하겠다 싶었어요.
그래도 의도를 잘 전달할 수 있다는 점은 좋았던 것 같습니다.
초코칩은 읽으면서 어떻게 도움됐는지 들어볼 수 있을까요?

IntelliJ의 자동 final 키워드를 이용하여 매개변수에 재할당을 기본적으로 방지하고 있습니다. 그러다보니 매개변수 부분의 길이가 아래와 같이 길어져 매개변수 부분에 개행이 들어가게 됩니다. 아서는 final 키워드에 대한 컨밴션을 어떻게 적용하는지 궁금합니다. 또한 일반 변수에도 final을 붙이는지도 궁금합니다.

저는 둘 다 final을 붙이지 않습니다, 불필요하다는 생각이 조금 강한 것 같아요.
만약 메서드 하나가 너무 길어서 언젠간 이 변수가 변할수도 있을 것 같아 라는 불안감이 들어서 final을 붙이는 거라면 메서드가 너무 큰게 아닐까 고민해봐야한다고 생각합니다. 그래서 일반적으로는 사용하지 않고 메서드의 길이가 너무 길어서 파악하기 어려울 때는 작성되어있는 경우를 본 것 같네요.

CardFactory에서 다른 카드도 구현받을 수 있게 설계하고 싶었으나, 결국 getSuit와 getRank 때문에 CardFactory가 TrumpCard에 종속적이게 되었습니다. 이를 수정할 수 있는 방법이 있을까요?

아 이런 의도에서 인터페이스를 분리하신 거군요ㅎㅎ
그런거라면 Card를 또 추상화해서 해결해야할 것 같습니다만, 지금 단계에서는 과도한 설계로 보여집니다.
추후에 트럼프카드 말고 다른 카드에 대한 요구사항이 생겼을 때 충분히 대응할 수 있을 것 같아서 미리 설계하지 않아도 괜찮을 것 같네요.
초코칩에게 반대로 물어보고 싶습니다. 인터페이스로 미리 분리해서 작업을 진행하면서 불편했던점은 없으셨나요?

Player가 draw하는 과정에서 Dealer를 넘기게 했기 때문에 테스트가 많이 힘들었습니다. Dealer가 아닌 Card만 넘겨줬다면, 결합도를 줄여 테스트를 용이하게 할 수 있었을 것 같습니다.

저도 동의합니다..ㅎㅎ
한번 그렇게 구조를 바꿔보면 어떨까요?

@Chocochip101
Copy link
Author

저는 사용하면서 편하다는 느낌을 받았는데, 익숙치 않아서 매번 리뷰를 할 때마다 1~5까지 보는 점은 좀 불편하겠다 싶었어요.
그래도 의도를 잘 전달할 수 있다는 점은 좋았던 것 같습니다.
초코칩은 읽으면서 어떻게 도움됐는지 들어볼 수 있을까요?

의도를 파악하기 좋았다고 생각합니다. 예를 들어 작성해주신 리뷰 중에 BufferedReader를 사용한 이유가 있을까요?에 대해 [질문]이나 [Pn]가 없었다면, 수정해달라는 의미로 받아들여 Scanner로 수정했을 수도 있습니다. 이런 식으로 의도를 파악하기 좋았으며, 작업 전에 어떤 작업부터 시작해야할지 중요도를 확인할 수 있어서 좋았습니다.

Copy link

@Hyunta Hyunta left a comment

Choose a reason for hiding this comment

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

안녕하세요 초코칩,
미션에 대해서 많이 고민하신게 느껴지네요.👍🏻
코드가 많이 개선된 것 같습니다
BlackjackInitializer와 관련해서 커멘트 남겼는데 확인부탁드릴게요.
고생하셨습니다!

@Hyunta Hyunta merged commit 3d31a50 into woowacourse:chocochip101 Mar 11, 2024
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