Skip to content

Conversation

hyeonho8453
Copy link

안녕하세요!
연료주입, 블랙잭 미션 제출합니다.
리뷰 잘 부탁드립니다! 감사합니다!

hyeonho8453 and others added 30 commits February 11, 2022 16:28
- 기능 요구 사항 작성
Copy link

@liquidjoo liquidjoo left a comment

Choose a reason for hiding this comment

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

안녕하세요 이번 블랙잭 리뷰어를 맡은 김성주 입니다
미션 구현 잘해주셨어요 👍
몇 가지 코멘트 남겼어요 확인하고 다시 요청 주세요 🙇

public static Deck createDeck() {
List<Card> blackJack = makeBlackJackCards();

Collections.shuffle(blackJack);

Choose a reason for hiding this comment

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

랜덤적인 요소가 있을 때 테스트하기 어려운 상태 값을 어떻게 테스트를 진행해 볼 수 있을까요?

Comment on lines +26 to +32
private static List<Card> makeBlackJackCards() {
return Arrays.stream(CardNumber.values())
.flatMap(number ->
Arrays.stream(CardType.values())
.map(type -> new Card(number, type)))
.collect(Collectors.toList());
}

Choose a reason for hiding this comment

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

게임이 한번이 아닌 여러번 실행할 수 있을 때 카드를 매번 52장을 생성해야할까요?
카드의 생성을 어떻게 줄여볼 수 있을까요?

}

public List<Card> getCards() {
return cards;

Choose a reason for hiding this comment

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

cards 의 주솟값이 외부에 노출이 되면 어떤 일이 일어날 수 있을까요?

@@ -0,0 +1,39 @@
package blackjack.domain;

public abstract class GamePlayers {

Choose a reason for hiding this comment

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

클래스 명이 복수 보다는 단수가 어울리네요 :)

Comment on lines +10 to +12
private final int TOP_OF_CARDS = 0;
private final String INVALID_DRAW = "남아있는 카드가 없습니다";
private final List<Card> cards;

Choose a reason for hiding this comment

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

Suggested change
private final int TOP_OF_CARDS = 0;
private final String INVALID_DRAW = "남아있는 카드가 없습니다";
private final List<Card> cards;
private final int TOP_OF_CARDS = 0;
private final String INVALID_DRAW = "남아있는 카드가 없습니다";
private final List<Card> cards;

상수 영역과 변수 영역의 구분을 지어주는게 가시성이 더 좋을 것 같네요~

Comment on lines +28 to +40
@Test
void 카드_두장_뽑기_테스트() {
dealer.initOwnCards(deck);

assertThat(dealer.ownCards.getOwnCards().size()).isEqualTo(2);
}

@Test
void 카드_한장_뽑기_테스트() {
dealer.drawOneCards(deck);

assertThat(dealer.ownCards.getOwnCards().size()).isEqualTo(1);
}

Choose a reason for hiding this comment

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

덱에서 어떤 카드를 뽑았는지 테스트 해보려면 어떻게 해보는 것이 좋을까요?

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

class DeckTest {

Choose a reason for hiding this comment

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

단위 테스트가 더 필요해 보여요

);
}

} No newline at end of file

Choose a reason for hiding this comment

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

파일 마지막에 엔터(개행문자)를 넣어주세요!!
이유는 리뷰를 진행할 때 깃허브에서 경고메시지를 지우기 위함입니다.

좀 더 알고 싶으시면 재미삼아 아래의 링크를 보시는 것도 추천 드립니다.
https://minz.dev/19

Intellij 를 사용하실 경우엔
Preferences -> Editor -> General -> Ensure line feed at file end on save 를 체크해주시면
파일 저장 시 마지막에 개행문자를 자동으로 넣어줍니다!


public class BlackJackController {

public static void playBlackJack() {

Choose a reason for hiding this comment

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

메서드 분리를 통해서 각 로직의 책임을 나누어 보는 것이 어떨까요?

Comment on lines +44 to +45
resultView.dealerGameResultPrint(gameResult);
resultView.playerGameResultPrint(gameResult);

Choose a reason for hiding this comment

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

resultView 를 두 번 호출하는 행위는 return 을 두 번 하는 것과 유사한 것 같아요
한 번의 호출로 바꾸어 보는 것이 어떨까요?

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