-
Notifications
You must be signed in to change notification settings - Fork 0
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단계] 아토(이혜린) 미션 제출합니다. #17
base: main
Are you sure you want to change the base?
[블랙잭 - 2단계] 아토(이혜린) 미션 제출합니다. #17
Conversation
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
- 기능을 추가하며 클래스 분리 (Player -> Hand) Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
- `Card`에 포함되는 모양과 수는 변하지 않고, 불변 객체이므로 그대로 전달 Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
- `toList`는 변경 불가능한 리스트를 반환하므로 `unmodifiableList` 제거 Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
Co-authored-by: donghoony <[email protected]>
- Player의 Hand 접근제한자 변경
* docs: 컨벤션 및 기능 요구사항 작성 Co-authored-by: donghoony <[email protected]> * chore: remove gitkeep Co-authored-by: donghoony <[email protected]> * feat(Card): 카드 점수 계산 Co-authored-by: donghoony <[email protected]> * feat(Deck): 덱에서 카드 한 장 드로우 Co-authored-by: donghoony <[email protected]> * feat(Deck): 비어있는 덱에서 드로우할 때 예외 처리 Co-authored-by: donghoony <[email protected]> * feat(Player): 플레이어 점수 계산 Co-authored-by: donghoony <[email protected]> * feat(Dealer): 딜러의 점수에 따른 카드 드로우 Co-authored-by: donghoony <[email protected]> * feat(Dealer): 주어진 점수에 따른 승패 판단 Co-authored-by: donghoony <[email protected]> * feat(Hand): Ace 점수를 유리한 방향으로 계산 - 기능을 추가하며 클래스 분리 (Player -> Hand) Co-authored-by: donghoony <[email protected]> * refactor(Player): 드로우 가능 여부 판단 메서드 위치 변경 Co-authored-by: donghoony <[email protected]> * feat(Name): 이름 생성 및 검증 Co-authored-by: donghoony <[email protected]> * feat(Players): 플레이어들 생성 및 검증 Co-authored-by: donghoony <[email protected]> * feat(View): 콘솔 입출력 Co-authored-by: donghoony <[email protected]> * feat(CardDisplay): 카드의 모양과 수의 표현 방식 설정 Co-authored-by: donghoony <[email protected]> * refactor(Player): 게임 상수 추출 및 드로우 조건 수정 Co-authored-by: donghoony <[email protected]> * feat(MatchResult): 게임 결과 판단 Co-authored-by: donghoony <[email protected]> * feat(MatchResults): 게임 결과 저장 Co-authored-by: donghoony <[email protected]> * feat(Deck): Shape와 Number의 조합으로 한 세트 생성 Co-authored-by: donghoony <[email protected]> * feat(Command): 사용자의 명령어 변환 Co-authored-by: donghoony <[email protected]> * refactor(OutputView): 매개변수로 `Card`를 받도록 수정 - `Card`에 포함되는 모양과 수는 변하지 않고, 불변 객체이므로 그대로 전달 Co-authored-by: donghoony <[email protected]> * feat(Display): 플레이어의 결과 표현 방식 설정 Co-authored-by: donghoony <[email protected]> * feat(BlackJackGame): 게임 흐름 제어 Co-authored-by: donghoony <[email protected]> * feat(BlackJackMain): 의존성 설정 및 실행 Co-authored-by: donghoony <[email protected]> * refactor(BlackJackGame): 메서드 분리 Co-authored-by: donghoony <[email protected]> * refactor(Players): 불필요한 래핑 제거 - `toList`는 변경 불가능한 리스트를 반환하므로 `unmodifiableList` 제거 Co-authored-by: donghoony <[email protected]> * style(Shape): 불필요한 세미콜론 제거 Co-authored-by: donghoony <[email protected]> * refactor: 상수 추출 Co-authored-by: donghoony <[email protected]> * refactor(OutputView): 메서드 분리 Co-authored-by: donghoony <[email protected]> * refactor: 패키지 이동 Co-authored-by: donghoony <[email protected]> * test(Player): 드로우 가능 여부 판단 Co-authored-by: donghoony <[email protected]> * fix(Dealer): 카드를 가지고 있지 않은 경우 예외 처리 Co-authored-by: donghoony <[email protected]> * refactor(Deck): 덱 생성 정적 메서드 추가 Co-authored-by: donghoony <[email protected]> * refactor(Hand): 접근 제한자 변경 Co-authored-by: donghoony <[email protected]> * refactor(Deck): 메서드 분리 Co-authored-by: donghoony <[email protected]> * refactor(Display): 패키지 분리 Co-authored-by: donghoony <[email protected]> * style(Number): 파일 끝 공백 추가 Co-authored-by: donghoony <[email protected]> * refactor: 메서드 순서 변경 Co-authored-by: donghoony <[email protected]> * refactor: 패키지 이동 - 컨벤션에 맞게 import 순서 변경 Co-authored-by: donghoony <[email protected]> * style: 불필요한 개행 삭제 - 컨벤션에 맞게 import 순서 변경 Co-authored-by: donghoony <[email protected]> * style(Hand): 가독성을 위한 변수 위치 및 줄바꿈 수정 Co-authored-by: donghoony <[email protected]> * refactor(MatchResult): 승리 조건 별 메서드 분리 Co-authored-by: donghoony <[email protected]> * test(MatchResult): 승리 조건 테스트 데이터 보강 Co-authored-by: donghoony <[email protected]> * style: 테스트 클래스 접근제한자 삭제 Co-authored-by: donghoony <[email protected]> * refactor(OutputView): 중복 코드 제거 및 메서드명 번경 Co-authored-by: donghoony <[email protected]> * style: 개행 및 오타 수정 Co-authored-by: donghoony <[email protected]> * docs: 기능 구현 사항 추가 * refactor(Card): this 사용 일관성 고려해 수정 - 파라미터와 구분이 되지 않는 경우만 사용 * refactor(InputView): 상수명 수정 * refactor(Display): 메서드명 수정 * test(Card): 에이스 여부 확인 * refactor(Hand): 점수 계산 로직 수정 * refactor: 드로우 관련 메서드명 변경 * refactor(Command): 클래스 내부 메서드로 No 여부 확인 * refactor(Rank): 클래스명 변경 * refactor(MatchResult): 메서드명 변경 * refactor(Players): 메서드 간소화 * refactor: 예외 메시지에서 상수 사용 * refactor: 상호 참조 제외 - 도메인에서 컨트롤러 참조하지 않게 수정 * refactor: 예외 발생시 메시지 출력 후 종료 * refactor(Display): 메서드 리턴 타입 변경 * refactor(OutputView): 상수 추출 * feat(Score): 점수 덧셈 * feat(Score): 버스트 여부 판단 * feat(Score): 점수 대소 비교 * feat(Score): 에이스 보정 점수 계산 * refactor: Score 이용 - Player의 Hand 접근제한자 변경 * test(Deck): 덱 생성 * refactor(Score): 메서드명 번경 --------- Co-authored-by: donghoony <[email protected]>
- 플레이어의 수익 확인 - 딜러의 수익 확인
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.
블랙잭 미션 마지막까지 화이팅해요오!
Deck deck = Deck.createShuffledFullDeck(); | ||
Dealer dealer = new Dealer(); | ||
Players players = createPlayers(); | ||
PlayerBettingMoney playerBettingMoney = decideBettingMoney(players); | ||
initializeGame(deck, dealer, players); | ||
proceedPlayersTurn(deck, players); | ||
proceedDealerTurn(deck, dealer); | ||
|
||
showCardsWithScore(dealer, players); | ||
showMatchResult(dealer, players, playerBettingMoney); |
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.
이번 미션 진행하면서 이 컨트롤러가 많이 커져서 불편하지 않았나요? ㅎㅎㅎ
저도 너무 커져서 Controller있고 아예 다른 Game이라는 객체를 만들어 요 객체 필드에 Dealer와 Players를 가지고 있게 구현하는 방식으로 리팩터링 해보았어요
initializeGame(deck, dealer, players);
proceedPlayersTurn(deck, players);
proceedDealerTurn(deck, dealer);
showCardsWithScore(dealer, players);
이 메서드들 모두 deck, dealer, players로 동일한데 매번 파라미터로 넘겨주는 형태가 뭔가 조금 이상한 거 같더라구요
그래서 저는 deck을 dealer 필드로 넣고, dealer와 players를 Game 이라는 필드에 넣어 컨트롤러의 무게를 조금 줄이는 방법으로 해보았는데, 나쁘진 않은 거 같더라구요!
public static double calculateRateOfPrize(Hand playerHand, Hand dealerHand) { | ||
if (isPlayerBlackJackCondition(playerHand, dealerHand)) { | ||
return PLAYER_BLACKJACK.rateOfPrize; | ||
} | ||
if (isPlayerWinningCondition(playerHand, dealerHand)) { | ||
return PLAYER_WIN.rateOfPrize; | ||
} | ||
if (isDealerWinningCondition(playerHand, dealerHand)) { | ||
return DEALER_WIN.rateOfPrize; | ||
} | ||
return TIE.rateOfPrize; | ||
} |
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.
이 부분을 BiPredicate 를 활용하여 구현할 수도 있을 거 같아요!
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.
오 한 번 알아보겠습니다
private static final int MIN_NAME_LENGTH = 2; | ||
private static final int MAX_NAME_LENGTH = 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.
이 클래스 이름에서 이미 NAME 이 들어가기에 MIN_LENGTH, MAX_LENGTH로 해두어도 나쁘지 않을 거 같아요!
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.
그러네용
private void validateInteger(String input) { | ||
try { | ||
Integer.parseInt(input); | ||
} catch (IllegalArgumentException e) { | ||
throw new IllegalArgumentException("[ERROR] 정수가 아닙니다."); | ||
} | ||
} |
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.
여기에서 걸리는건 IllegalArgumentException 이 아니라 NumberFormatException 일 거 같은데 맞을까요..?
그리고 추가로 Integer 범위가 넘어가는 수가 입력되어도(ex 3000,000,000) 예외가 터질거 같은데 이때에는 "정수가 아닙니다."라는 메시지가 맞지 않을 거 같아요..!
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.
오!!!! 그러네요..... 수정하겠습니다
그런데 왜 catch가 되는걸까요...?
private static final Hand handBlackJack = new Hand(List.of( | ||
new Card(Shape.CLOVER, Rank.ACE), | ||
new Card(Shape.DIAMOND, Rank.TEN) | ||
)); | ||
private static final Hand hand22 = new Hand(List.of( | ||
new Card(Shape.HEART, Rank.TEN), | ||
new Card(Shape.CLOVER, Rank.TEN), | ||
new Card(Shape.DIAMOND, Rank.TWO) | ||
)); | ||
private static final Hand hand21 = new Hand(List.of( | ||
new Card(Shape.CLOVER, Rank.JACK), | ||
new Card(Shape.DIAMOND, Rank.TEN), | ||
new Card(Shape.HEART, Rank.ACE) | ||
)); | ||
private static final Hand hand20 = new Hand(List.of( | ||
new Card(Shape.CLOVER, Rank.JACK), | ||
new Card(Shape.DIAMOND, Rank.TEN) | ||
)); |
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.
제가 기억하기론 Hand 한 클래스에 묶어두셨던 거 같은데 다시 분리하신 이유가 있을까요??
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.
질문이 이해가 안갑니다..!! 뭘 분리를 했을까요 제가.....??
public class Money { | ||
|
||
private final int money; | ||
|
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.
Money 의 타입을 int로 두었는데, 배팅 금액의 상한선을 정해두지 않으면 계산 과정에서 오버 플로우가 발생할 수 있습니다!
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.
아토 고생 많으셨습니다!
이번 step2는 따로 구현할게 많이 없었죠~
다음 체스 미션도 화이팅해봐요 👍
public enum Command { | ||
YES("y"), | ||
NO("n"); |
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.
만약 카드를 더 받을지 말지 결정하는 명령어를 "y"에서 다른 문자열 요구사항으로 변경된다면
아토는 어느 패키지를 먼저 열어볼 것 같으신가요??
현재 사용자 입력을 받는 view 패키지가 따로 존재하고 BlackJackGame
이 Controller
역할을 도맡아 하고있다면
추후 유지보수 측면에서 생각해봤을 때 Command
의 현재 위치는 적절하지 않아보입니다
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.
저도 약간 고민했던 부분인데...
inputView에서 커맨드를 이용해 불리안 값을 리턴해주는 방식을 고려중입니다!
public class MatchResults { | ||
|
||
private final Hand dealerHand; | ||
private final Map<Player, Integer> results; | ||
|
||
public MatchResults(Hand dealerHand) { |
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.
아토는 객체의 필드로 선택하는 본인만의 기준이 있는지 궁금해요~
저는 필드가 객체의 상태를 나타내는지, 필드가 한 객체 내에서 공통적으로 사용되는지를 종합적으로 고려해서 선택해요
DealerHand
가 MatchResults
의 상태, 속성에 해당한가? 하면은 저는 조금 의문이 들거든요 😕
어떻게 생각하시나용~?
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.
약간 결과 계산해주는 심판 느낌으로 만든건데
결과 판단에 사용되는 딜러는 항상 같은 딜러니까
한 게임 전체의 결과를 심판하기 시작할 때 사용되는 딜러의 카드가 무엇인지를 알려주면 어떨까?
하는 생각이었습니다
약간 의도가 잘 안담긴 것 같기는 하네요 😹
|
||
private static boolean isPlayerWinningCondition(Hand playerHand, Hand dealerHand) { | ||
Score playerScore = playerHand.calculateScore(); | ||
Score dealerScore = dealerHand.calculateScore(); | ||
|
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.
isDealerWinningCondition
메서드에서도 플레이어와 딜러의 점수를 계산하던데 계산을 여러번 하는 이유가 궁금합니다
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.
hand가 아니라 score를 넘겨줄까 고민을 했었는데 그러니까 메서드 호출부가 너무 길어져서 이런 선택을 했습니다...!
그렇다고 calculateRateOfPrize에서 변수로 만들면 10라인 제한을 어기게 되더라구요...
|
||
public class MatchResults { | ||
|
||
private final Hand dealerHand; | ||
private final Map<Player, Integer> results; |
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.
원시 값을 포장한 Money
클래스를 만든 만큼
블랙잭 게임 결과 Map
에서도 Money
를 사용할 수 있었으면 좋겠어요.
아마 음수의 Money
생성을 막기 위한 검증 때문이었을까요?
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.
맞습니다.... 머니를 베팅머니로 바꾸는게 좋겠어요
아직 달린 리뷰가 없어 요청한 코드로 변경해서 다시 pr 보냅니닷
제이미 안녕하세요! 또 한 번 BE 6기 아토입니다.
변경된 기능만 충족할 수 있도록 코드를 수정해보았는데요, 관련해서 설명 드리겠습니다.
미션 진행 시 생각한 부분
1. 승패를 결정하던
MatchResult
가 수익 비율을 결정2. 플레이어별 배팅 금액을 저장하는 객체
PlayerBettingMoney
3. 결국 실패한 컨트롤러 분리... 😢
읽어보시고 의견 혹은 질문 많이많이 주세요!
즐거운 주말 보내시길 바랍니다~
감사합니다!