-
Notifications
You must be signed in to change notification settings - Fork 75
[김성현] 연료 주입 및 블랙잭(1단계) #23
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
base: kim-svadoz
Are you sure you want to change the base?
Conversation
- 이동거리로 주입해야할 연료량을 계산하는 로직 구현
- generateChargeQuantityByName()
- 자동차_이름과_연료량을_받아_map을_반환한다()
- List<String>으로 반환
및 Names 생성자 시그니처 추가
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 CardPack(List<Card> cardPack) { | ||
this.cardPack = cardPack; | ||
public CardPack() { | ||
this.cardPack = create(); |
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.
블랙잭 게임이 한 번이 아닌 계속 할 수 있다면 매번 카드를 52장 생성할 것 같아요
이런 객체의 생성을 어떻게 줄여볼 수 있을까요?
} | ||
|
||
public static CardPack createWithShuffling() { | ||
private List<Card> create() { |
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.
create 되어 상태 값인 cardPack 을 테스트할 때 shuffle 로 인해 테스트가 어려울 것 같아요
어떻게 단위 테스트를 진행해 볼 수 있을까요?
} | ||
|
||
public List<Card> getCardPack() { | ||
return Collections.unmodifiableList(cardPack); |
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.
unmodifiableList
는 참조를 완전히 끊어내지 못합니다~
new ArrayList<>() 를 활용해 참조를 완전히 끊어 보는 것이 어떨까요?
Assertions.assertThat(report.getReport().get("Avante")).isEqualTo(20); | ||
Assertions.assertThat(report.getReport().get("K5")).isEqualTo(20); | ||
Assertions.assertThat(report.getReport().get("Sonata")).isEqualTo(15); |
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.
assertAll 을 활용해 보면 좋을 것 같아요~
assertAll 을 사용하는 것과 assertThat 을 여러개 사용하는 것에 대한 차이도 알면 좋을 것 같아요~
@Test | ||
public void 카드가_정상적으로_셔플되어진다() { | ||
//given | ||
CardPack cardPack = new CardPack(); |
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.
어떻게 하면 CardPack 에 대한 테스트를 제어하면서 테스트를 만들어 볼 수 있을까요?
- new ArrayList<>()의 인자로 미리 생성한 카드팩을 넘겨준다.
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.
피드백 반영 잘 해주셨어요 👍
한 가지 아쉬운 점이 있어서 코멘트 남겼어요~
확인하고 다시 요청주세요 🙇
|
||
@Test | ||
public void 카드가_정상적으로_셔플되어진다() { | ||
public 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.
로직을 분기처리하여 카드 상태를 확인하는 것도 좋지만 로또에서 수동과 자동 로또를 어떻게 발급했는지 생각해보면 좋을 것 같아요~
구조적으로 많은 고민을 하며 틈틈히 리팩토링도 함께 진행하였습니다.
잘 부탁드립니다.