Skip to content

Conversation

fpal95
Copy link

@fpal95 fpal95 commented Feb 18, 2022

안녕하세요. 리뷰어님!

이번 미션은 해림님(@Hae-Riri)과 진행했습니다.
페어님의 도움을 많이 받았는데, 부족한 부분 계속 보완하겠습니다.

Copy link

@vsh123 vsh123 left a comment

Choose a reason for hiding this comment

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

윤경님 안녕하세요!
1단계 미션 잘 구현해주셨네요 :)

몇가지 코멘트를 남겼으니 확인부탁드리겠습니다~

# java-blackjack
# java-blackjack

## 연료 주입 기능 목록
Copy link

Choose a reason for hiding this comment

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

기능 구현 목록 작성 💯


@Override
public double getChargeQuantity() {
return getTripDistance() / getDistancePerLiter();
Copy link

Choose a reason for hiding this comment

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

getTripDistance()대신 상태를 바로 호출하면 어떨까요?

Suggested change
return getTripDistance() / getDistancePerLiter();
return this.distance / getDistancePerLiter();


public static final int FIRST_INDEX = 0;

private List<Card> cards;
Copy link

Choose a reason for hiding this comment

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

final 선언이 가능해보여요~!

Comment on lines +22 to +24
Arrays.stream(Denomination.values())
.map(denomination -> new Card(shape, denomination))
.forEach(cards::add);
Copy link

Choose a reason for hiding this comment

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

Denomination.values를 stream으로 돌리긴 했지만 결국 depth가 2를 넘는 것과 같아보이는데요! 조금 더 작은 단위의 메서드로 분리해보면 어떨까요?

return cards;
}

public Card pickOneCard() {
Copy link

Choose a reason for hiding this comment

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

카드가 비어있을 때에 대한 validation을 추가해보면 어떨까요?

this.value = value;
}

public String getSign() {
Copy link

Choose a reason for hiding this comment

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

미사용 메서드는 제거해주세요!

public Player(String name, List<Card> cards) {
this.name = name;
this.cards.addAll(cards);
this.sum = getSumOfCards(); //TODO
Copy link

Choose a reason for hiding this comment

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

  • TODO는 아직 미완성일걸까요?
  • sum이 canHit에서만 사용되는데, getSumOfCards에도 사용되어야하지 않을까요?!

return sum <= PLAYER_MAXIMUM_SUM;
}

public int getSumOfCards() {
Copy link

Choose a reason for hiding this comment

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

블랙잭의 규칙에 따르면 ACE는 1점과 11점모두 가능합니다!

만약 KING과 ACE라면 점수는 21점이 되어야하는데, 현재는 11점이되는 것 같아요! 요고 한번만 확인해주세요~

아래는 테스트입니다.

    @Test
    void ACE는_1과_11점_두가지_모두_가능하다() {
        //given
        Player player = new Player("kim", Arrays.asList(
            new Card(Shape.SPADE, Denomination.KING),
            new Card(Shape.SPADE, Denomination.ACE)
        ));

        //when 
        int actual = player.getSumOfCards();
        
        //then
        assertThat(actual).isEqualTo(21);
    }

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.

3 participants