Skip to content

Conversation

LeahJiinLee
Copy link

안녕하세요

김정호 페어님과 함께 연료주입, 블랙잭 미션을 수행했습니다!
앞으로 주실 피드백으로 열심히 리팩토링 해나가겠습니다!!

감사합니다ㅎㅎ

Hoya-kim and others added 30 commits February 11, 2022 16:15
연료 주입 미션 요구 사항 정리
- 기능 요구 사항
- 프로그래밍 요구 사항
- 기능 구현 사항 리스트 정리
- Avante, K5, Sonata 클래스 생성
- 제공된 테스트 코드 추가
- constructor
- getDistancePerLiter()
- getTripDistance()
- getName()
- 기능 구현 사항 리스트 완료 처리
- 블랙잭 게임 기능 구현 사항 작성
classes
- BlackjackGame
- Dealer
- Deck
- Denomination
- Gambler
- Judgement
- InputView
- OutputView
- Player
- Suit
classes
- DealerTest
- DeckTest
- DenominationTest
- GamblerTest
- JudgementTest
- PlayerTest
- SuitTest
- 기능 구현 완료 사항 체크 표시
- enum타입의 suit, denomination값을 가지는 Card class 설계
- deck class 구현
- field : List card
- method: setupCard, setDenomination, shuffle, popCard
- 중복 코드 제거를 위함
- Dealer#allocateCard 메서드 구현
- calculateScore
- adjustScore
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.

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

Comment on lines +80 to +85
private List<Player> getAllGamblers(Dealer dealer, List<Player> players) {
List<Player> allGamblers = new ArrayList<>();
allGamblers.add(dealer);
allGamblers.addAll(players);
return allGamblers;
}

Choose a reason for hiding this comment

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

Dealer 와 Players 의 두 상태 값을 갖는 객체를 만들어 보는 것이 어떨까요?

Comment on lines +57 to +66
private boolean isPlayerGettingMoreCard(Dealer dealer, Player player) {
boolean answer = InputView.askAddCard(player);
if (answer) {
dealer.allocateCard(player);
OutputView.printJoinedCardInfo(player);
OutputView.printNextLine();
return true;
}
return false;
}

Choose a reason for hiding this comment

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

프로그래밍 요구 사항 중 하나인 핵심 로직을 구현하는 코드와 UI를 담당하는 로직을 구분한다. 를 지켜보아요

import java.util.List;
import java.util.stream.Collectors;

public class BlackjackGame {

Choose a reason for hiding this comment

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

  • 너무 많은 책임이 있는 것 같아요객체의 역할을 나누어 책임을 분리 시켜보는 것이 어떨까요?
  • BalckjackGame 의 책임들을 어떻게 테스트를 해볼 수 있을까요?

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

public class Dealer extends Player {

Choose a reason for hiding this comment

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

연료주입에서 배웠던 추상 클래스를 활용해보면 어떨까요?
Player 의 상속을 받았을 때 어떤 단점이 있으며
추상 클래스를 활용함으로써 어떤 장점이 있을까요?

Comment on lines +17 to +27
private void setupCard() {
for (Suit suit : Suit.values()) {
setDenomination(suit);
}
}

private void setDenomination(Suit suit) {
for (Denomination denomination : Denomination.values()) {
cards.add(new Card(suit, denomination));
}
}

Choose a reason for hiding this comment

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

만약 게임이 한판만에 끝나는게 아니라 계속 게임을 다시 할 수 있다면
Card 의 객체는 계속 생성될 것 같아요
객체의 생성을 어떻게 줄여볼 수 있을까요?

Comment on lines +12 to +19
@DisplayName("딜러는 카드를 한 장씩 꺼낼 수 있다")
@Test
void testAllocateCard() {
Dealer dealer = new Dealer();
Player player = new Player("플레이어");
dealer.allocateCard(player);
assertThat(player.getCards().size()).isEqualTo(ONE);
}

Choose a reason for hiding this comment

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

Dealer 와 Player 의 강한 결합이 있어서 Dealer 테스트를 할 때도 Player 가 필요하고 Player 를 테스트할 때도 Dealer 의 카드 할당이 필요해 보여요
객체의 강한 결합을 어떻게 분리할 수 있을까요?

import org.junit.jupiter.api.DisplayName;
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.

카드 생성 시 랜덤적인 요소로 인해 테스트하기 어려운 메서드를 어떻게 테스트를 진행해 볼 수 있을까요?


import static org.junit.jupiter.api.Assertions.*;

class DenominationTest {

Choose a reason for hiding this comment

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

테스트 코드가 없어요~


import static org.junit.jupiter.api.Assertions.*;

class SuitTest {

Choose a reason for hiding this comment

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

테스트 코드가 없어요~

Comment on lines +41 to +42


Choose a reason for hiding this comment

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

Suggested change

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