Skip to content
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단계 - 블랙잭(베팅)] 돌범(조재성) 미션 제출합니다. #306

Merged
merged 28 commits into from
Mar 21, 2022

Conversation

is2js
Copy link

@is2js is2js commented Mar 16, 2022

안녕하세요 로운!

변동사항 확인에 도움이 되시라고 시간순으로 적용된 로직들을 나열해놓습니다. 이후 고민사항도 적었습니다!

STEP1 이후 시간에 따른 수정 사항

  1. 결과 객체를 생성하는 GameResult#of
    • 개별 점수들을 raw값으로 입력 -> 각 player를 받도록 수정(블랙잭 판단을 위해)
  2. is Finish / isBurst 포함 isBlackjack 까지 모두 정보와 데이터를 가지는 playingCards로 메세지 보내서 물어보도록 함
  3. 컨트롤러에서 enum을 응답받아 .equals()로 비교했던 로직 -> 이넘객체에도 다 메세지 보내는 방식으로 변경
    //변경 전 
    if (askHitOrStay(PlayerDto.from(gambler)).equals(BlackJackCommand.YES)) {
    
    //변경 후
    if (askHitOrStay(PlayerDto.from(gambler)).isHit()) {
    
    //BlackJackCommand.java
    public boolean isHit() {
          return this.equals(YES);
      }
  4. test코드 -> 같은 상태값을 공유하는 테스트들 @nest 적용
  5. test에서 전략패턴을 적용한 카드덱에 대해 수동 카드덱 생성을 위한 FixtureGenerator(testutil) 구현
  6. player 비교 결과인 GameResult에 블랙잭 결과 추가
    • gambler가 2장일 때, 합 21이면 블랙잭이다.
    • dealer가 burst일지라도, gambler는 승리가 단순 승리 뿐만 아니라 블랙잭으로 인한 승리 처리 추가
      1. gambler의 블랙잭 아닌 것으로 인한 승리 -> LOSE 반환
      2. gambler의 블랙잭으로 인한 승리 -> BLACKJACK 반환
  7. BlackJackResult에 dealerResult 상태값 삭제 -> 메서드로 dealer수익 계산하도록 함.
    • map을 list로 바꿔서 전달한 dto삭제. map을 그대로 넘김(현 프로젝트 한정)
  8. GameResult 결과에 따른 수익금액 계산 필드(Function) 추가하여 결과객체에 따라 수익 계산법 따로 개별 적용
  9. 배팅금액 BetMoney 포장

고민사항

  • 미션 진행 과정에서 gamblers와 dealer의 공통점은 희미해지고 묶을 이유가 없어지고 있습니다.
  • 피드백 강좌에서 상속을 최대한 피해서 유지보수성을 선택하는 방향이 좋겠다는 이야기를 듣고, 일부 메서드만 추상화해서 따로 관리하는게 더 좋을 듯 싶습니다!
    • Q1. 여기서 조합으로 구현하는 것은 아니겠지요? 공부해보니 전략패턴처럼 바깥에서 공통되는 코드들을 주입받아 적용시키는 것 같았는데, 해당사항이 없을 것 같습니다. 로운 생각은 어떠신지요? 인터페이스를 활용해서 일부메서드만 추상화 -> 싹다 개별 구현, 공통되지 않는 메서드들도 다 각자 구현하는 방향으로 도전을 해보는 것도 좋을 것 같습니다!

is2js added 21 commits March 15, 2022 13:57
- 기존: PlayingCards에서 점수계산 후 -> Player에서 get하여 burst비교후 판단
- 변경: PlayingCards에서 점수계산 후 -> burst판단까지 마치고 ->  Player는 판단결과만 반환
    - 데이터를 가진 도메인에게 판단도 맡기자. Player가 판단하기 보다는 이미 데이터로 판단된다.
- 자식들이 부모 필드를 상속하여 호출하는 것이 아닌, 부모 public 메서드로 메세지를 보내도록 변경
- 부모클래스로 메서드 이동
- Dealer, Gambler에서 결과 도출 -> PlayingCards로 역할 변경
- 원시값 score1, score2를 받을 경우, player의 상태 ex> 카드 갯수를 고려한 블랙잭이냐? 등 판단이 안되어서 객체를 전달하도록 수정함.
- 처음 받은 2장만 소지한 체로 블랙잭(ace+10)인 경우, 무조건 승리하여 -> dealer는 패배로 먼저 응답하도록 구현
- 테스트 코드 구현
- 게임결과로 블랙잭을 반환한다.
   - enum 핃르로서 BiPredicate라도 1개로만 메서드에서 사용하여 검사할 수 있다.
   - gambler가 2장일 때, 합 21이면 블랙잭이다.
   - dealer가 burst일지라도, gambler는 승리가 블랙잭으로 인한 승리 처리되어야한다.
   - 승무패 중에서는 가장 먼저 검사하지만, 버스트 검증보다는 느리다 -> 버스트 검증시 블랙잭 예외처리 해준다.
       - 버스트 처리시 dealer가 burst로 인한 패배가 확실시 되었을 때 아래 경우의 수를 나누었다.
           1. gambler의 블랙잭 아닌 것으로 인한 승리 -> LOSE 반환
           2. gambler의 블랙잭으로 인한 승리 -> BLACKJACK 반환일반승리
- 조건: (Bi)Predicate -> test, 함수: (Bi)Function -> apply
- gambler에 betMoney 필드 추가
- dealer의 승무패 저장 변수 삭제
- gambler 개별 수익 계산후 -> 메서드에 의해 ( 총 수익 * -1 ) dealer 수익금 계산
- blackjackResultDto 삭제후 map으로 넘기도록 변경
- null과 empty, foramt은 view에서 검증
- range는 domain에서 검증
- 계산 및 출력 단위 조정: long -> double
@lowoon
Copy link

lowoon commented Mar 17, 2022

제가 생각하는 추상클래스는 하위클래스와 정확하게 일치할 때 쓴다고 생각합니다. 추상클래스 A와 이를 상속받는 하위클래스 B는 정확하게 의미와 역할이 일치할 텐데요. B의 역할이 커지게 되면 이 때 A와 B가 같은지를 고려해야하고, 필요하면 분리하여 추상클래스로 묶지 말아야한다고 생각합니다.

돌범의 경우, 공통으로 묶은 player에 있는 메서드들이 정말 player의 역할에 맞는 것인지 한번 생각해보고 맞다고 생각하면 분리하는 것도 좋다고 생각합니다~

Copy link

@lowoon lowoon left a comment

Choose a reason for hiding this comment

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

반가워요 돌범! 2단계도 잘 부탁해요 😄
슬랙이랑 코멘트 단 내용들 고민해 보시고 수정 후에 다시 리뷰요청해주세요~
궁금한 점이 생기면 언제든지 DM 혹은 댓글남주세요 ^-^

gamblerResult.put(gambler, currentDealerResult.reverse());
return dealer.compare(gambler);
private static Double calculateProfit(final Players players, final Player gambler) {
final GameResult gameResult = players.getDealer().compare(gambler);
Copy link

@lowoon lowoon Mar 17, 2022

Choose a reason for hiding this comment

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

players가 모든 데이터를 가지고 있으니 내부에서 결과를 도출할 수도 있을텐데요. 역할을 한번 고민해봐도 좋을거 같아요~
이렇게 dealer와 gambler를 여러군데에서 가져와서 활용한다면 Players에서 dealer와 gambler를 묶어서 관리해야할 지 한번 고민해봐도 좋을거 같아요

Copy link
Author

Choose a reason for hiding this comment

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

현재 미션 주제인 상속 -> 추상클래스 사용에 맞춰 is-a관계로 짜맞추다보니, Players를 사용하려고 노력했던 것 같습니다.

  • 다음 구현시, 인터페이스를 사용하여 각각 구현한 뒤, 개별적으로 관리하도록 해보겠습니다

감사합니다. 로운!

return new BetMoney(Integer.valueOf(inputView.scanBetMoney(name)));
}

private void spreadCards(final Players players, final CardDeck cardDeck) {

Choose a reason for hiding this comment

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

이 메서드는 카드를 플레이어에게 나눠주는 작업과 메시지를 출력하는 작업 2가지 일을 하고 있습니다.
플레이어에게 카드를 나눠주는 작업을 게임 로직을 담당하는 클래스에게 넘겨주시기 바랍니다.

Copy link
Author

Choose a reason for hiding this comment

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

내부 카드1장를 나누어주는 로직 뿐만 아니라 상수+for문 등 바깥에서 돌아가는 로직 또한 특정 객체의 역할로서 넘기겠습니다.

메인 로직 구현에 대해 좋은 인사이트를 얻은 것 같습니다! 감사합니다 구구

@@ -41,18 +43,21 @@ private Player getDealer() {
private List<Player> getGambler() {
final List<String> playerNames = inputView.scanPlayerNames();
return playerNames.stream()
.map(Gambler::new)
.map(name -> new Gambler(name, getBetMoney(name)))

Choose a reason for hiding this comment

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

getBetMoney

메서드명만 보면 getter로 보이는데 실제로는 BetMoney라는 객체를 생성하고 있네요.
적절한 메서드명으로 바꿔보아요.

Copy link
Author

Choose a reason for hiding this comment

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

지금까지 도메인 객체를 얻는 최종 순간을 getXXXX()로 명명하였는데

Q. 내부 scan과정까지 포함되있으므로 scanAndGetBetMoney()로 변경하였습니다. 이대로 사용해도 될지 궁금합니다!

private boolean isHit(final Player gambler, final CardDeck cardDeck) {
if (askHitOrStay(PlayerDto.from(gambler)).equals(BlackJackCommand.YES)) {
private boolean isHitThenReceiveCard(final Player gambler, final CardDeck cardDeck) {
if (askHitOrStay(PlayerDto.from(gambler)).isHit()) {

Choose a reason for hiding this comment

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

  1. dto가 필요할까요? dto를 제거해봅시다
  2. 객체지향 생활체조에 "한 줄에 한 점만 찍는다"는 규칙이 있습니다. 어떤 것을 의미하는 걸까요? 해당 규칙에 따라 코드를 수정해보시기 바랍니다.

Copy link
Author

Choose a reason for hiding this comment

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

scan하는 부분에 Player Name이 필요해서 사용해주었었는데,
player는 불변객체가 아닌데, 말씀처럼 Dto를 제거하고 player를 넘겨도 view에 넘겨도 될지 궁금합니다.
Q. view에서는 getName()을 사용할텐데, 방어적복사없이 응답하면 원본 오염시킬 수 있는 일급컬렉션이 아니라, 원시값을 응답해주므로 사용해도 상관없을까요?

현재는 이 코드에 대한 PlayerDto만 걷어냈습니다.

Q. 혹시 다른 부분에서 쓰인 PlayerDto 및 (Playerssss를 포장한 )PlayerssssDto.from()에 대해서도 삭제하라는 말씀이실까요??

@@ -21,6 +21,8 @@

private static final int DEFAULT_SPREAD_COUNT_START_INDEX = 0;

Choose a reason for hiding this comment

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

BlackJackGame는 어떤 역할을 맡은 객체일까요?
BlackJackGame에 정의한 상수들을 적절한 클래스로 옮겨보시기 바랍니다.

Copy link
Author

Choose a reason for hiding this comment

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

도메인 역할로 옮겨서 상수를 제거하도록 하였습니다 감사합니다 구구!

@@ -83,11 +88,12 @@ private void playGame(Player gambler, CardDeck cardDeck) {
}

private boolean canGamblerReceiveCard(final Player gambler, final CardDeck cardDeck) {
return isHit(gambler, cardDeck) && isNotBurst(gambler) && gambler.isNotFinished();
return isHitThenReceiveCard(gambler, cardDeck) && isNotBurst(gambler) && gambler.isNotFinished(

Choose a reason for hiding this comment

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

  1. 조건이 너무 복잡한데 조금 더 단순화시킬 방법은 없을까요?
  2. 게임 규칙과 관련된 조건들은 해당 클래스로 옮길 방법 없을까요?

Copy link
Author

Choose a reason for hiding this comment

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

현재 추상클래스 -> 상속이라는 주제에 맞춰서 짜다보니, dealer와 gambler의 로직을 추상클래스의 is-a 관계로서, 최대한 비슷하게 맞추려고 하다가 복잡해진 것 같습니다.

다시 구현해도 비슷하게 구현될 것 같긴한데, 혹시 조언 좀 해주실 수 있을까요?!

지금 생각엔, 인터페이스를 통해 dealer<-> gamblers를 구분해서 구현하면, 메세지를 편하게 던질 수 있을 것 같아 로직이 많이 풀릴 것 같긴합니다! 고민해보겠습니다!

}
}

public Integer getValue() {

Choose a reason for hiding this comment

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

Suggested change
public Integer getValue() {
public int getValue() {

래핑 클래스를 사용할 필요가 없어보이네요.

Copy link
Author

Choose a reason for hiding this comment

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

해당 메서드는 제거되었습니다 감사합니다 구구~!

final Map<Player, GameResult> gamblerResult = new LinkedHashMap<>();
final EnumMap<GameResult, Integer> dealerResult = players.getGamblers()
public static BlackJackResult from(final Players players) {
return new BlackJackResult(players.getGamblers()

Choose a reason for hiding this comment

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

한 줄에 너무 많은 코드가 들어가있네요.
stream의 결과를 변수로 분리해보시고 의미있는 이름을 붙여주세요.

Copy link
Author

@is2js is2js Mar 21, 2022

Choose a reason for hiding this comment

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

stream의 결과를 변수로 분리하라는 피드백을 2번 연속 받았습니다.

주의해서 체크리스트화 해야겠네요! 감사합니다.

Q. 만약 분리하라는 피드백만 받았다면, 메서드로 추출했을 것 같은데, 메서드 추가로 인한 가독성 << 지역변수 추가로 인한 가독성일까요??

    //변경 전
    public double calculateDealerProfit() {
        return this.value.values()
            .stream()
            .mapToDouble(Double::valueOf)
            .sum() * DEALER_FLIP_UNIT;
    }

    //변수로 추출
    public double calculateDealerProfit() {
        final double dealerTotalProfit = this.value.values()
            .stream()
            .mapToDouble(Double::valueOf)
            .sum();
        return dealerTotalProfit * DEALER_FLIP_UNIT;
    }

    //메서드로 추출
    public double calculateDealerProfit() {
        return getDealerToTalProfit() * DEALER_FLIP_UNIT;
    }

    private double getDealerToTalProfit() {
        return this.value.values()
            .stream()
            .mapToDouble(Double::valueOf)
            .sum();
    }
``` java

public Map<GameResult, Integer> getDealerResult() {
return dealerResult;
public double calculateDealerProfit() {
return this.value.values()

Choose a reason for hiding this comment

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

stream 결과를 변수로 추출하고, 그 변수와 DEALER_FLIP_UNIT를 곱한 결과를 return하도록 작성해주세요.
코드 가독성이 훨씬 높아질거에요.

Copy link
Author

Choose a reason for hiding this comment

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

피드백 반영했습니다. 위에 같은 맥락의 질문도 올렸습니다!

- 계산을 Function에서 직접하는 것 -> 메세지보내 계산하도록 수정
@kang-hyungu
Copy link

몇 가지 추가 코멘트 남겼습니다.
리뷰어께서는 제 코멘트와 별개로 리뷰 마무리하셨으면 머지 처리해주시기 바랍니다!

…yers 기능 분리

- 한가지 역할만 하도록 수정
- 그에 따른 for문 및 상수 등은 도메인 내부로 이동
// 변경 전(get 후에 바로 메서드 호출)
if (askHitOrStay(gambler).isHit()) {
// 변경 후
final BlackJackCommand inputCommand = askHitOrStay(gambler);
if (inputCommand.isHit()) {
@is2js
Copy link
Author

is2js commented Mar 21, 2022

하이 로운! 블랙잭에 대한 우테코 피드백 강의 이후 많은 생각을 했습니다!

상태패턴에 대해 학습하였으며, 인터페이스와 추상클래스 사용에 대해 많은 고민을 해보았습니다.
결과적으로 고민만 하고, 로직 수정은 없는 상태이며, 구구가 남겨준 피드백에 대해 약간 수정을 해보았습니다.

1. 로운이 DM으로 주신 피드백 중에 isBurst나 isBlackJack 관련 내용을 GameResult 같은 클래스로 분리하는게 어떻느냐 하셨습니다.

  • 일단 GameResult.of( , ) 로 넘어간 dealer, gambler에 대해서 내부에서 isBusrt 등으로 메세지를 보내서 처리한다는 점에서, player.isBusrt등의 로직은 다른 곳으로 넘기지 않는 것으로 결정했습니다.

    • player가 Cards를 들고 있는 한 무조건 Player에게 물어본다고 생각했기 때문입니다.
  • 상태패턴을 사용하면, Player가 State를 상태값으로 들고 있는 대신, Cards는 State가 상태값으로 들고 있게 되지만, 결국엔 player에게 isBurst를 물어볼 것은 마찬가지라 생각하여 주말간 공부만 하고, 현재 미션에는 적용하지 않은 상태입니다.

  • 다음에 기회가 된다면, 추상클래스는 사용하지 않고 인터페이스로만 dealer와 gambler를 구현한 뒤 -> Dealer + Gamblers로 나누어서 개별적으로 관리할 것 같습니다.

    • 공통점이 그리 크지도 않고, 사용도 많이 나누어서 되는 상황인데, 억지로 추상화해서 사용할려다보니 모든 로직이 is-a형태로 짜맞추려다가 복잡해진 것 같습니다. 이 부분은 새롭게 구현해보면서 고민해보겠습니다!
  • 그동안 감사했습니다. 로운!


2. 구구 피드백에 대한 질문입니다.

  1. 중간에 PlayerDto.from ( ) 코드를 보고 dto는 현재 불필요하다. 삭제를 권유해주셨습니다.

    • 혹시 view에서 원본오염 걱정이 없는 원시값(getName())을 요구하는 Player에 대해서만 Dto를 삭제해야할지, 아니면 PlayerSSSssDto를 포함해서 모든 Dto를 삭제해야할지 고민입니다. -> 만약 삭제한다면, Players를 불변List로 만들어서 list를 방어적 복사가 적용된 getter로 view에서 편하게 뽑아쓰라는 것인지 궁금합니다.
  2. 억지로 추상클래스 사용 및 is-a 형태를 맞추려고 Gambler, Dealer의 로직을 짜맞추다보니, 말씀대로 복잡해진 부분 BlackjackGame 부분에서 while문에 조건이 복잡하게 걸린 부분이 있습니다.

    • 다음 추상클래스가 아닌 인터페이스 사용 및 개별 구현하면 조금 더 좋아질 것 같습니다. 고민을 해보겠습니다!
    • 관련 질문은 피드백 주신 곳에 달아놓겠습니다!

Copy link

@lowoon lowoon left a comment

Choose a reason for hiding this comment

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

안녕하세요 돌범!😄
간단한 코멘트만 남겼으니 확인 부탁드려요.
블랙잭 미션을 이만 마무리해도 될거 같아요!
미션은 끝났지만 궁금한점 있으면 DM이나 코멘트로 남겨주세요!

1, 2단계 블랙잭 미션 수고하셨습니다!

@@ -38,26 +37,22 @@ private Player getDealer() {
return new Dealer();
}

private List<Player> getGambler() {
private List<Player> getGamblers() {
Copy link

@lowoon lowoon Mar 21, 2022

Choose a reason for hiding this comment

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

gambler를 생성한다는 의미로 수정하는게 더 명확할거 같아요.

Copy link
Author

Choose a reason for hiding this comment

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

오랜만에 다시 로운을 뵙습니다! 1단계 마지막 미션을 수행한 뒤, 블랙잭 최종 피드백 반영하러 왔습니다^^

getter도 역할도 아닌데 getXXXX는 어색할 것 같아서, create or receive Gameblers로 수정하겠습니다!

@@ -83,32 +78,33 @@ private void playGame(Player gambler, CardDeck cardDeck) {
}

private boolean canGamblerReceiveCard(final Player gambler, final CardDeck cardDeck) {
return isHit(gambler, cardDeck) && isNotBurst(gambler) && gambler.isNotFinished();
return isHitThenReceiveCard(gambler, cardDeck) && isNotBurst(gambler) && gambler.isNotFinished();
Copy link

Choose a reason for hiding this comment

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

isNotFinished()에서 burst를 이미 검증하고 있기 때문에 isNotBurst는 필요가 없을거 같아요.

Copy link
Author

Choose a reason for hiding this comment

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

gambler.isNotFinished() 자체가 카드합이 17이하인 상태이므로, isNotBust 20이하인지 검사는 이미 포함관계네요!
조건이 여러개일 때, 포함관계를 잘 확인해야겠습니다!

요즘은 &&로 걸린 조건들을 드모르간의 법칙으로 쪼개는 연습도 해보았습니다^^

@@ -24,12 +25,12 @@ public Players(final Player dealer, final List<Player> gamblers) {

public void receiveCard(final CardDeck cardDeck) {
for (Player player : value) {
cardDeck.drawTo(player);
cardDeck.drawTo(player, DEFAULT_DRAW_COUNT);
Copy link

@lowoon lowoon Mar 21, 2022

Choose a reason for hiding this comment

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

cardDeck.drawTo가 역할에 맞는 메서드일까요??

    for (Player player : value) {
         player.receiveCard(cardDeck.pop());
    }

로 만들고 blackJack에서 2번 호출하는게 맞지 않을까요??
처음에 분배를 몇개하는지는 어디에서 아는것이 역할과 책임에 맞을까요??

Copy link
Author

@is2js is2js Apr 16, 2022

Choose a reason for hiding this comment

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

처음에 controller라 생각하고, controller내 로직을 제거하기 위해 players에게 메세지를 보내서 처리했던 것 같습니다.
하지만, player에게 역할을 줬음에도, 내부에서는 cardDeck이 역할을 수행하고 있는 모순된 구조네요.
또한, 로운의 말처럼 blackjackGame은 처음 card를 2개만 정확히 줬음을 알고 있어야할 것 같습니다!

도메인에 메세지를 보낼 때, 어떠한 재료?를 보내지, 역할을 수행하는 도메인을 넘기는 것은 고려해보아야겠습니다!

public void printBurst(final PlayerDto playerDto) {
System.out.println(playerDto.getName() + BURST_INSTRUCTION);
public void printBurst(final Player player) {
System.out.println(player.getName() + BURST_INSTRUCTION);
Copy link

Choose a reason for hiding this comment

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

player에 getName메서드가 있는데요.
도메인에 view에서만 사용하는 메서드를 만드는 것이 맞을까요??
사용자에게 어떻게 보여줄지는 누구의 책임일까요??

Copy link
Author

Choose a reason for hiding this comment

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

dto를 사용했다면, 어차피 PlayeDto.from(player) 내부에서 player.getName()을 사용하게 되었었습니다.

dto에서 호출하는 것 또한 domain이 아닌 곳에서 호출하는 것이기 때문에, 어차피 도메인이 아닌 곳에서 사용되어야한다고 생각합니다.

이 때, 구구의 피드백으로 dto를 한번 제거해보자고 하셔서 이렇게 진행했는데, 흠.. dto를 쓰더라도, getName()은 도메인이 아닌 곳에서 사용되니 고민이네요

private int adjustSumByAce(int currentSum) {
int aceCount = aceCount();
int aceCount = getAceCount();
while (currentSum > BLACKJACK_NUMBER && aceCount > NO_COUNT) {
Copy link

@lowoon lowoon Mar 21, 2022

Choose a reason for hiding this comment

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

count만큼 -로 처리하기 때문에 로직에 while문이 들어가는데요.
처음부터 ace를 1로 처리를 하면 ace가 있어도 로직상 한번밖에 더할 수 없기 때문에 if문 한번으로 끝날 수 있습니다~

Copy link
Author

Choose a reason for hiding this comment

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

ace를 큰 숫자로 두고, 최대한 큰 숫자를 유지하려는 의도가 있었습니다.
만약, ace를 작은 숫자로 둔다면, +10이 가능한가? 물어보고 -> 가능하다면 +10해서 return해줘
형태로 로직이 늘어날 것 같았는데요.. 실제로 ace의 점수보정은 단 한번만 가능하네요!

음.. 만약, ace와 4를 받았다면, -> 5 -> 14가능? -> 14 -> 더 받는다. -> + ace -> 15 -> 24 가능?
최소값: ace + 1 -> 2 -> 12가능? 물어보고 12을 선택
+ace : 12 -> 22가능? 은 물어볼 필요가 없네용..

ace는 끽해봐야 1장만 1-> 11로 변환이 가능하네요!

만약, 큰 숫자로 유지한다면? 애초에 존재못하는 보정점수를 가지게 되서 쓸데없이 로직이 늘어네요!

상한 있을 땐, 점수보정을 작은 곳에서 시작하도록 하겠습니다! 감사합니다 로운

public int getCardSum() {
return adjustSumByAce(getCurrentSum());
}

private int getCurrentSum() {
Copy link

Choose a reason for hiding this comment

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

일반적으로 get은 가지고 있는 필드를 반환하는데 쓰이는데요.
조금이라도 로직이 들어가면 그에 맞는 메서드명을 정해주는 것이 더 명확할 거 같아요~

Copy link
Author

Choose a reason for hiding this comment

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

get-> calculate로 변경하겠습니다!

@lowoon lowoon merged commit bed7976 into woowacourse:is2js Mar 21, 2022

public class BlackJackResult {

private final Map<GameResult, Integer> dealerResult;
private final Map<Player, GameResult> gamblerResult;
private static final int DEALER_FLIP_UNIT = -1;
Copy link

Choose a reason for hiding this comment

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

dealer의 결과를 만들기위해 상수가 생겼는데요.
Double로 처리를 한 이유를 잘 모르겠지만 Long을 사용하면 Long.reverse를 사용할 수도 있습니다~

Copy link
Author

Choose a reason for hiding this comment

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

x 0.5 계산이 있어서 double로 줬는데, 돈은 항상 10원이상 단위라서 무조건 정수가 나오겠네요!

검색해보니 Long.reverse()는 부호를 바꾸는 것이 아니라 푸리에변환으로 image 벡터 변환시 사용되는 메서드인 것 같은데 의도하신 메서드가 맞나 궁금합니다!

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.

None yet

3 participants