-
Notifications
You must be signed in to change notification settings - Fork 467
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단계 - 블랙잭 베팅] 체체(김진영) 미션 제출합니다. #878
base: cheche903
Are you sure you want to change the base?
Conversation
- BetAmount -> Betting
이에 따라 바뀌는 컨트롤러, 테스트 뷰가 수정됩니다.
- 블랙잭 없이 승리 시 - 블랙잭이며 승리 시 - 패배 시
- 각각 플레이어, 딜러로 나눈 이유는 각 구현체마다 합을 계산하는 방식이 다르기 때문에 독립적으로 구현합니다.
- 카드들의 최소 합이 21을 넘어가면 더 이상 카드를 뽑지 못합니다. 이는 버스트와는 다른 상태입니다.
- Hand 의존성 때문에 Gamer에 작성하지만, 추후에 Hand를 주입해줌으로써 해결할 예정입니다.
- 최종 결과를 담는 상태를 객체로 사용합니다. 또한 Rule을 만들어 게임의 룰과 승패 적용을 분리합니다.
- equals -> == 연산자
- 다만 뷰와 강결합된 비즈니스 로직은 제거하지 않습니다.
- 오타, 띄어쓰기, 공백 컨벤션을 지킵니다.
- 컨트롤러에서 딜러 이름을 모르게 합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요, 체체. 리뷰어 제이미입니다.
코멘트 남겨두었습니다.
@@ -12,6 +12,15 @@ | |||
- [x] 닉네임의 길이는 최소 2자, 최대 5자까지 가능하다. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
생각보다 그 전에도 못지킨 것 같아서 1점 낮췄습니당 (캡슐화를 지키지 못한 것 같아요)
이번 단계에서 지켜보도록 합시다. 지키도록 리팩터링해보세요.
딜러는 첫 에이스만 11로 취급할 수 있는 이유가 무엇인가요?
딜러는 버스트 되지 않는 한, A 는 11로 카운트한다. 예를 들어 A-6 이라면 17이 되며, 딜러 규칙에 의해서 무조건 스테이한다는 것을 의미한다.
이 말은, 버스트되지 않는 한, 즉 6-A라도 11로 카운트한다는 의미로 보이는데요.
제가 내린 결론은
성능이 중요하다 -> 성능을 챙긴다.
성능이 중요하지 않고, 협업 시 많은 사람이 건드릴 수 있는 코드이다 -> 가독성, 의도 부분을 더 챙긴다.가 되었습니다!
좋네요.
반복 사용을 하는 것이 더 가독성이 좋다면, 비교군의 코드가 잘 짠 코드라는 보장을 할 수 있나요?
가독성과 성능을 모두 가져갈 순 없나요?
닉네임이 한글도 가능합니다! "딜러"라는 이름도 있기 때문입니다 !
플레이어명은 영문만 제한하고 있는데, 이건 기능이 아닌가요?
따라서 핸드 객체가 프라이빗이기 때문에 Player에서 Hand로 접근이 불가능합니다. 그래서 hand의 카드를 얻고 싶으면 Gamer의 hand로 접근해야하기 때문에 다음과 같이 작성되었습니다.
핸드 객체가 프라이빗이라면 접근제어자를 푸는 방법도 있을텐데 왜 이러한 방법을 선택했나요?
players를 만들어서 바로 .getPlayers()를 해준다면, 포장한 의미가 있을까요?
해당 코멘트를 'players가 해결할 수 있는 비즈니스 로직이다.'로 이해하였습니다!
따라서 그렇게 고쳐보았습니다. 하지만 입력 또는 출력을 하는 부분은 (플레이어 카드 받을 것인지, 받은 다음 카드 정보 출력) 어떻게 처리해야할 지 감이 좀 안오는 것 같아요. 힌트를 받아볼 수 있을까요 ?
원랜 의미 그대로 포장한 의미가 무엇인지에 대한 질의였지만요.
다른 방식으로 사용해보려면 입/출력만 꺼내서 사용해보고, 나머지는 전부 Players를 이용해보는 것도 좋겠네요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 말은, 버스트되지 않는 한, 즉 6-A라도 11로 카운트한다는 의미로 보이는데요.
맞습니다. 6-A도 11로 카운트하고, 17을 이상이기에 딜러는 카드를 뽑지 않습니다.
16 이하면 무조건 히트, 17 이상이면 무조건 스테이.[[58]]
플레이어가 15에서 스테이했고, 딜러는 16일 때 - 딜러가 스테이하면 플레이어를 이길 수 있는 상황이지만, 딜러는 의무적으로 버스트의 위험을 감수하고 무조건 히트해야 한다.
플레이어가 18에서 스테이했고, 딜러는 17일 때 - 딜러가 히트를 해서 A~4 의 카드가 나오면 비기거나 이길 수 있다. 하지만, 딜러는 히트할 수 없고, 스테이해야 한다.
만약 5-A 라면 16으로 1장을 더뽑아야 합니다. 이때, 5-A-A가 나온다면 버스트이기 때문에 마지막 A를 1로 취급하여 17이 되고 히트를 하지 않습니다.
4-A-A인 경우도 4-11-1로, 16이 되기 때문에 한 장을 더 뽑아야 합니다.
좋네요.
반복 사용을 하는 것이 더 가독성이 좋다면, 비교군의 코드가 잘 짠 코드라는 보장을 할 수 있나요?
가독성과 성능을 모두 가져갈 순 없나요?
비교군의 코드가 잘 짠 코드라는 보장 <- 이 문장이 이해가 잘 가지 않아요. 제이미. 더 설명해주실 수 있나요?
현재 이해한 바로 답변을 먼저 하자면,
퍼블릭 인터페이스에는 Players, Delaer를 받고, 프라이빗 메서드에 원시값을 보내는 것이 가독성과 성능 둘 다 챙길 수 있다고 생각합니다.
플레이어명은 영문만 제한하고 있는데, 이건 기능이 아닌가요?
게임에 참여할 사람의 이름을 입력하세요.(쉼표 기준으로 분리)
체체,제이미
체체의 배팅 금액은?
10000
제이미의 배팅 금액은?
10000
딜러와 체체, 제이미에게 2장을 나누었습니다.
딜러카드: K스페이드
체체카드: Q다이아몬드, 9다이아몬드
제이미카드: A하트, 6하트
체체는(은) 한장의 카드를 더 받겠습니까?(예는 y, 아니오는 n)
n
체체카드: Q다이아몬드, 9다이아몬드
제이미는(은) 한장의 카드를 더 받겠습니까?(예는 y, 아니오는 n)
y
제이미카드: A하트, 6하트, 5다이아몬드
제이미는(은) 한장의 카드를 더 받겠습니까?(예는 y, 아니오는 n)
n
딜러는 16이하라 한 장의 카드를 더 받았습니다.
딜러카드: K스페이드, 4스페이드, Q클로버 - 결과: 24
체체카드: Q다이아몬드, 9다이아몬드 - 결과: 19
제이미카드: A하트, 6하트, 5다이아몬드 - 결과: 12
## 최종 수익
제이미: 10000
체체: 10000
딜러: -20000
한글도 가능합니다! 어느 부분에서 영문만 제한하고 있다는 것이 나오는 지 궁금합니다.
핸드 객체가 프라이빗이라면 접근제어자를 푸는 방법도 있을텐데 왜 이러한 방법을 선택했나요?
핸드 객체의 접근 제어자를 풀게 되면 dealer.hand.getSumOfRank();
이렇게 Gamer가 있는 곳에서는 모두 hand에게 접근이 가능해집니다. 그렇게 되면 핸드 객체의 상태를 외부에서 직접 변경할 가능성 증가한다고 판단하여 이렇게 구현했습니다.
만약 getCards() 같은 (캡슐화를 지키지 못하는) 메소드를 없애려면, 딜러와 플레이어의 다른 점인 계산 방식을 전략 패턴으로 주입해주는 방식이 있다고 찾았어요. 한 번 진행해보겠습니다.
README.md
Outdated
@@ -43,25 +52,25 @@ | |||
- [x] 딜러가 버스트면서 플레이어가 버스트가 아닐 때 | |||
- [x] 둘 다 버스트가 아니면서 딜러보다 합계가 높을 때 | |||
- [x] 무승부인 경우 | |||
- [ ] 둘 다 블랙잭일 때 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
체크가 되지 않은 항목들이 왜 존재할까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
실수로 누락되었습니다 !
추가하였습니다.
@@ -18,4 +18,8 @@ public Card drawCard() { | |||
} | |||
return cards.poll(); | |||
} | |||
|
|||
public List<Card> getInitialGameCards() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트가 되고 있나요? 해당 메서드 추가의 경우 TDD를 하지 않은 이유는 무엇인가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
테스트가 되고 있지 않습니다. 수정하겠습니다!
아마 컨트롤러에서 고치면서 급하게 생긴 메서드라 그런 것 같아요. 실수입니다.
import java.util.Arrays; | ||
import java.util.List; | ||
|
||
public class BlackJackRules { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 객체를 따로 뺀 이유는 무엇인가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
해당 객체를 뺀 이유는 객체 관리 용이 및 가독성 측면입니다.
저희 코드는 기존에 21(카드 두장으로 블랙잭)인 상태를 처리하지 않았습니다.
이번 2단계 요구사항에서 두 장 블랙잭이 가장 강한 카드 조합임을 알고, 규칙이 더 생김에 따라 따로 관리해주는 것이 더 낫다고 판단되었습니다.
이렇게 된다면 규칙이 바뀐다면 해당 객체만 수정하면 되고, 룰을 파악하기 위해서 해당 객체만 파악하면 된다는 이점이 있어 따로 객체로 관리하게 되었습니다.
import domain.gamer.Dealer; | ||
import domain.gamer.Gamer; | ||
import domain.gamer.Nickname; | ||
import domain.gamer.Player; | ||
import domain.gamer.Players; | ||
import java.util.Arrays; | ||
import java.util.LinkedHashMap; | ||
import java.util.List; | ||
import java.util.Map; | ||
import java.util.Random; | ||
import view.InputView; | ||
import view.OutputView; | ||
|
||
public class BlackJackController { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc카드: Q스페이드, A다이아몬드
cc는(은) 한장의 카드를 더 받겠습니까?(예는 y, 아니오는 n)
n
cc카드: Q스페이드, A다이아몬드
cc카드: Q스페이드, A다이아몬드 - 결과: 21
블랙잭이여도 카드를 더 뽑게 하는 건 의도된 동작인가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정하였습니다!
1b7c3d1
|
||
public class Betting { | ||
|
||
private static final int BET_AMOUNT_MIN_VALUE = 1000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
_
를 이용해 가독성을 올려봅시다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정하였습니다!
603323a
return (int) (profit + (betAmount * BLACKJACK_BONUS)); | ||
} | ||
|
||
public int winBetting(final int profit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
메서드 시그니처만 봤을 때, 어떠한 일을 할지 예측할 수 있나요?
이번 미션의 챕터답게, 전반적으로 변수, 메서드, 상수, 클래스들의 이름들에 대한 점검을 해봅시다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정하였습니다!!
729293a
Player player2; | ||
Player player3; | ||
Dealer dealer; | ||
Player defaultPlayer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
접근제어자는 왜 없을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수정하였습니다!
da4c4a4
@DisplayName("딜러의 첫번째 카드를 가져온다.") | ||
@Test | ||
void 딜러의_첫번쨰_카드를_가져온다() { | ||
void 딜러의_첫번째_카드를_가져온다() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
보다보니, displayName과 test명을 동일하게 두 번 선언하는 이유가 무엇인가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
영어로 작명하기엔 많은 시간이 걸릴 것 같아 이렇게 작성합니다.
해당 부분도 크루들과 이야기해보면 좋을 것 같아요!
@@ -1,7 +1,5 @@ | |||
package domain.gamer; | |||
|
|||
import static domain.BlackJackConstants.BUST_NUMBER; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안쓰는 변수들도 제거해봅시다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전체적으로 수정하면서 사라졌습니다!
리팩터링 과정에서 실수가 많이 나오는 것 같아요...
이러한 부분을 더 잘 캐치할 수 있도록 신중하게 커밋하겠습니다!
기존에는 첫 카드가 21(블랙잭)인 경우도 카드를 받을 수 있게 되었습니다. 이제는 첫카드 두장도 검증을하여 hit을 못하도록 수정합니다.
- 합 계산을 전략 패턴으로 리팩터링합니다.
- 기존에는 딜러와 플레이어를 통해 테스트 했지만, 이제는 각자 전략 구현체로 테스트합니다.
- 버스트, 블랙잭, 카드뽑기 상태 단위 테스트를 추가합니다.
체크 리스트
test
를 실행했을 때, 모든 테스트가 정상적으로 통과했나요?객체지향 생활체조 요구사항을 얼마나 잘 충족했다고 생각하시나요?
1~5점 중에서 선택해주세요.
선택한 점수의 이유를 적어주세요.
생각보다 그 전에도 못지킨 것 같아서 1점 낮췄습니당 (캡슐화를 지키지 못한 것 같아요)
어떤 부분에 집중하여 리뷰해야 할까요?
안녕하세요 제이미~ ! 생각보다 2단계 구현이 오래 걸렸네요.
저는 우테코 교육장에서는 책을 읽고, 밤에 집에가서 미션을 하려고 노력합니다!
근데 생각보다 잘 되진 않네요 ㅋㅋㅋ
나무위키에 나와 있는 딜러의 규칙에 의해 정했습니다!
나무위키 4.5입니다 :)
딜러는 버스트 되지 않는 한, A 는 11로 카운트한다. 예를 들어 A-6 이라면 17이 되며, 딜러 규칙에 의해서 무조건 스테이한다는 것을 의미한다.
그렇네요. 마지막에 체크를 한다고 가정하고, 이전의 커밋으로 돌아간다면 삭제되겠군요!
그렇다면 최대한 지키면서
바뀐 파일들에 대해 코드 포맷팅을 적용한다.
가 될 것 같아요.결론 - 도메인 내에서 모두 관리하되, 뷰는 신경쓰지 않고 한다. 가 될 것 같아요!
dealerSum
,player
를 보내는 것보다dealer
,player
를 보내는 것이 의도 파악이 더 쉽다.메소드는 의존도를 낮춰야 하니 원시값을 보내는 것이 맞다.
가독성이 각각 보내는 게 더 낫다.
가 있었습니다 ! 제가 내린 결론은
성능이 중요하다 -> 성능을 챙긴다.
성능이 중요하지 않고, 협업 시 많은 사람이 건드릴 수 있는 코드이다 -> 가독성, 의도 부분을 더 챙긴다.
가 되었습니다!
학습 목적입니다. :)
닉네임이 한글도 가능합니다! "딜러"라는 이름도 있기 때문입니다 !
컨트롤러(입출력과 게임 진행 담당)이 아는 것도 아니라고 판단하였고,
뷰도 상세한 이름 자체는 몰라도 된다고 생각합니다.
딜러 스스로가 이름을 정하고 그 이름을 내보내는 것이 캡슐화가 잘 지켜진 것이라고 생각됩니다!
fdc954f
찾아보니 equals는
NullPointerException
을 발생시킨다고 하네요!반면에 == 연산자는
false
를 반환하구요.이러한 점에서 == 연산자가 더 안전하다고 생각합니다!
이번 2단계 때 객체로 만들었습니다.
1단계에 유틸성 클래스로 만든 이유는 해당 상태에 대해 몰랐던 것 같아요. 승패무를 저장하는 상태가 필요하기에 이는 객체로 관리되어야 한다고 판단되어 2단계 때 수정하였습니다!
87bc539
상속을 사용하여 생긴 문제점인 것 같아요.
구조가
Gamer(private Hand, private nickname) -> Player(Betting)
-> Dealer
이렇게 되고 있습니다. 따라서 핸드 객체가 프라이빗이기 때문에 Player에서 Hand로 접근이 불가능합니다. 그래서 hand의 카드를 얻고 싶으면 Gamer의 hand로 접근해야하기 때문에 다음과 같이 작성되었습니다.
해당 코멘트를 '
players
가 해결할 수 있는 비즈니스 로직이다.'로 이해하였습니다!따라서 그렇게 고쳐보았습니다. 하지만 입력 또는 출력을 하는 부분은 (플레이어 카드 받을 것인지, 받은 다음 카드 정보 출력) 어떻게 처리해야할 지 감이 좀 안오는 것 같아요. 힌트를 받아볼 수 있을까요 ?
27bca1e
c177dfc
수정했습니다 :)