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단계 - 블랙잭(베팅)] 오찌(오지훈) 미션 제출합니다. #301

Merged
merged 70 commits into from
Mar 21, 2022
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
70 commits
Select commit Hold shift + click to select a range
d260ec1
refactor: 카드 덱의 구성 방식을 LinkedList로 바꾸고 draw 시 remove 메서드로 카드 반환하도록 변경
Ohzzi Mar 15, 2022
2ebdb05
style: 접근 제어자 순서 변경
Ohzzi Mar 15, 2022
e5b9592
refactor: 52장의 카드를 가진 덱 생성을 정적 팩토리 메서드로 변경
Ohzzi Mar 15, 2022
320970f
refactor: HitRequest 생성 시점을 view에서 입력을 받았을 때로 변경
Ohzzi Mar 15, 2022
0c7d20c
refactor: 카드 덱의 자료구조를 리스트에서 큐로 변경
Ohzzi Mar 15, 2022
c6ba660
refactor: 빈 덱인지 검증하는 로직 분리
Ohzzi Mar 15, 2022
281c0bf
refactor: 참가자가 Score의 value를 가지는 꺼내서 가지지 말고 Score 자체를 사용하도록 변경
Ohzzi Mar 15, 2022
30b9b66
fix: 잘못된 메서드 사용 수정
Ohzzi Mar 15, 2022
f94176f
refactor: 딜러 결과를 구하는 메서드 분리
Ohzzi Mar 15, 2022
d44185a
docs: 요구 기능 및 객체 별 역할 재정리
Ohzzi Mar 15, 2022
110a51c
feat: 베팅 객체 구현
Ohzzi Mar 15, 2022
4eeb85a
chore: Name을 participant 패키지로 이동
Ohzzi Mar 15, 2022
f13022f
feat: 승패 결과에 블랙잭으로 인한 승리 추가
Ohzzi Mar 15, 2022
30996e3
feat: 플레이어에 베팅 상태 추가
Ohzzi Mar 15, 2022
86765b3
refactor: 베팅 객체에서 수익률 계산 로직 제거
Ohzzi Mar 15, 2022
95d3c2b
feat: GameResult를 승무패 저장에서 수익 저장으로 변경
Ohzzi Mar 15, 2022
4e0f6e2
feat: 게임 종료 시 딜러와 플레이어의 수익 출력
Ohzzi Mar 15, 2022
490fb54
fix: 메시지 출력 시 개행 조정
Ohzzi Mar 15, 2022
3538e24
style: 잘못된 들여쓰기 조정
Ohzzi Mar 15, 2022
b1cab92
refactor: 딜러 수익을 플레이어 수익의 반대로 하는 로직 분리
Ohzzi Mar 15, 2022
29af8b8
refactor: 불필요한 로직과 테스트 제거하고 수익률 테스트 추가
Ohzzi Mar 15, 2022
d76d395
refactor: 불필요한 조건문 제거
Ohzzi Mar 16, 2022
d816dc8
refactor: 수익률 변수 명 조금 더 쉽게 변경
Ohzzi Mar 17, 2022
e131a0b
refactor: 수익금 계산 메서드를 플레이어로 이동
Ohzzi Mar 17, 2022
87e82c3
refactor: State를 없애고 해당 로직을 Cards로 이동
Ohzzi Mar 17, 2022
6c8c717
refactor: 수익 계산 로직 간소화
Ohzzi Mar 17, 2022
36855da
refactor: 사용하지 않는 생성자 삭제
Ohzzi Mar 17, 2022
80aead9
refactor: 승패 계산 시 불필요한 조건문 제거
Ohzzi Mar 17, 2022
28eb0a6
refactor: 캐싱된 카드 주 생성자를 private로 막도록 수정
Ohzzi Mar 17, 2022
919372c
refactor: 불필요한 참조 및 필드 제거
Ohzzi Mar 17, 2022
19f07b4
test: 카드 생성자를 사용하는 테스트 정적 팩토리 메서드를 사용하는 방식으로 변경
Ohzzi Mar 17, 2022
f32bf69
test: 테스트에서 객체를 만드는 중복 로직들을 유틸 클래스로 분리
Ohzzi Mar 17, 2022
f990136
feat: 플레이어 이름 중복 검사 기능 추가
Ohzzi Mar 17, 2022
758cf9f
docs: 요구 기능 및 객체 별 역할 재정리
Ohzzi Mar 17, 2022
7433560
refactor: 플레이어 출력 시 뷰의 메서드를 연속적으로 출력하지 않고 리스트를 넘겨서 한번에 뷰에서 처리하도록 변경
Ohzzi Mar 18, 2022
3890608
refactor: 로컬 변수명 수정
Ohzzi Mar 18, 2022
d116b8f
docs: 게임 상태 목록 정리
Ohzzi Mar 18, 2022
01fbd6d
feat: 처음 시작 상태 구현
Ohzzi Mar 18, 2022
0c47442
feat: Started 상태에서 hit 하는 기능 및 기본 상태 구현
Ohzzi Mar 18, 2022
1785af3
feat: 블랙잭 상태(카드가 2장이고 합이 21) 구현
Ohzzi Mar 18, 2022
d89f026
feat: stand 기능 구현
Ohzzi Mar 18, 2022
322ee9e
feat: Stand 상태 세부 사항 구현
Ohzzi Mar 18, 2022
500a55a
refactor: Cards에 add 시 새 Cards를 반환하도록 변경
Ohzzi Mar 18, 2022
44ca077
feat: Hit 상태 구현
Ohzzi Mar 18, 2022
ed50441
feat: Started의 hit에 Bust 로직 추가
Ohzzi Mar 18, 2022
45fbc6f
fix: cards.add 시 반환값을 무시하는 오류 수정
Ohzzi Mar 18, 2022
453ae04
feat: Bust 상태 구현
Ohzzi Mar 18, 2022
0cd60cf
refactor: 게임 진행을 더 할 수 없는 상태들을 추상 클래스 Finished의 상속 관계로 묶어줌
Ohzzi Mar 18, 2022
79a8465
refactor: 게임이 진행중인 상태들을 추상 클래스 Running의 상속 관계로 묶어줌
Ohzzi Mar 18, 2022
4f6e5a7
refactor: Bust 생성자 public에서 default로 변경
Ohzzi Mar 18, 2022
c8d8697
fix: 테스트 시 카드 중복 오류 수정
Ohzzi Mar 18, 2022
158061e
feat: 각 상태에 blackjack, bust, finished 인지 여부를 반환하는 기능 추가
Ohzzi Mar 18, 2022
f773465
refactor: Score와 Score 비교에 메서드 체이닝 사용하도록 변경
Ohzzi Mar 18, 2022
2477149
refactor: Participant가 State를 가지고 사용하도록 변경
Ohzzi Mar 18, 2022
f40aa15
refactor: 점수 계산을 Cards를 getter로 꺼내지 않고 State 안에서 처리하도록 이동
Ohzzi Mar 18, 2022
21a914d
refactor: Hit, Started, Running을 Running 하나로 통합
Ohzzi Mar 18, 2022
9119d44
refactor: State와 Running, Finished 사이에 공통 부분을 AbstractState로 묶어줌
Ohzzi Mar 18, 2022
a0dd0eb
refactor: getCards가 Cards가 아닌 리스트 반환하도록 변경
Ohzzi Mar 18, 2022
3641a00
refactor: 게임 진행 시 플레이어의 hit 조건 판정 로직을 변경
Ohzzi Mar 18, 2022
9be5ffa
test: 카드 덱의 draw 테스트를 조금 더 의미 있는 테스트로 변경
Ohzzi Mar 19, 2022
eb9631d
refactor: State 객체에서 isFinished를 제외한 상태 판정 메서드를 instnaceOf로 대체
Ohzzi Mar 21, 2022
d933b6b
refactor: Running을 딜러와 플레이어 별로 분리
Ohzzi Mar 21, 2022
e9f038d
refactor: "플레이어의 이름은 중복될 수 없다"는 규칙 검증 책임을 Name의 컬렉션에서 Player의 일급 컬렉션으…
Ohzzi Mar 21, 2022
3eddd19
feat: "플레이어는 2~8인이어야 한다"라는 규칙 추가
Ohzzi Mar 21, 2022
0a96c63
fix: 중복 및 인원수 체크 로직 시 무한루프 발생하는 오류 수정
Ohzzi Mar 21, 2022
62f21df
refactor: 불필요한 메서드와 테스트 삭제
Ohzzi Mar 21, 2022
d1ee8a2
docs: 객체 별 역할 재정리
Ohzzi Mar 21, 2022
b750865
refactor: 상태들의 접근 제어자 조정
Ohzzi Mar 21, 2022
f828c08
refactor: view 클래스들의 생성자 호출을 막기 위해 private으로 변경
Ohzzi Mar 21, 2022
537e3c6
refactor: 사용하지 않는 메서드 제거
Ohzzi Mar 21, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 24 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
- [x] 카드 덱 생성하기
- [x] 플레이어 이름 입력 받기
- [x] 쉼표로 분리
- [ ] 플레이어 별로 베팅 금액 입력 받기
- [ ] 숫자가 아니면 예외 발생
- [x] 딜러 및 플레이어 생성하기
- [x] 딜러 및 플레이어들에게 카드를 각각 두 장씩 주기
- [x] 딜러의 카드는 첫 장만 공개
Expand All @@ -24,7 +26,15 @@
- [x] 플레이어의 숫자가 21을 넘기면 무조건 플레이어의 패배
- [x] 플레이어의 숫자가 딜러보다 21에 가까우면 플레이어의 승리
- [x] 플레이어의 숫자가 21을 넘기지 않고 딜러의 숫자가 21을 넘기면 플레이어의 승리
- [x] 최종 승패 결과를 출력
- [x] 같은 21이어도 블랙잭(2장)이 승리
- [x] ~~최종 승패 결과를 출력~~
- [x] 베팅 계산
- [x] 일반적인 상황에 대한 계산
- [x] 이기면 베팅 금액 만큼 수익
- [x] 지면 베팅 금액 만큼 손실
- [x] 비기면 베팅 금액을 돌려받음
- [x] 블랙잭으로 이기면 1.5배의 수익
- [ ] 최종 수익을 출력

---

Expand Down Expand Up @@ -53,12 +63,18 @@
- 카드를 생성하는 전략을 제공하는 인터페이스
- BlackJackCardsGenerator
- CardsGenerator의 구현체로 서로 다른 52장의 카드를 가진 덱을 생성한다.
- Rule
- 게임의 규칙을 담고 있다.
- 카드의 총합을 계산한다.
- 버스트(총합이 21을 넘는 경우) 여부를 판단한다.
- Result
- State
- 플레이어가 가질 수 있는 상태(버스트, 블랙잭)
- 보유한 카드를 받아서 상태를 판정한다.
- Outcome
- 게임의 승, 무, 패
- 플레이어를 기준으로 딜러와 비교해서 판단한다.
- Betting
- 플레이어의 베팅 정보를 저장한다.
- 상태
- GameResult
- 플레이어와 딜러의 카드를 받는다.
- 승패를 결정한다.
- Judgement
- 승, 무, 패 값을 가진 enum
- 승패를 바탕으로 수익률을 저장한다
- Outcome
- 승, 무, 패 값을 가진 enum
9 changes: 0 additions & 9 deletions src/main/java/blackjack/BlackJackApplication.java

This file was deleted.

9 changes: 9 additions & 0 deletions src/main/java/blackjack/BlackjackApplication.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package blackjack;

public class BlackjackApplication {

public static void main(String[] args) {
BlackjackGame blackjackGame = new BlackjackGame();
blackjackGame.play();
}
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package blackjack;

import blackjack.domain.Betting;
import blackjack.domain.GameResult;
import blackjack.domain.Name;
import blackjack.domain.card.CardDeck;
import blackjack.domain.participant.Dealer;
import blackjack.domain.participant.Name;
import blackjack.domain.participant.Player;
import blackjack.dto.HitRequest;
import blackjack.dto.ParticipantInitialResponse;
Expand All @@ -13,10 +14,10 @@
import java.util.List;
import java.util.stream.Collectors;

public class BlackJackGame {
public class BlackjackGame {

public void play() {
CardDeck deck = new CardDeck();
CardDeck deck = CardDeck.createGameDeck();
Dealer dealer = new Dealer(deck.drawDouble());
List<Player> players = createPlayers(inputPlayerNames(), deck);
proceed(deck, dealer, players);
Expand All @@ -36,10 +37,19 @@ private List<Name> inputPlayerNames() {

private List<Player> createPlayers(List<Name> playerNames, CardDeck deck) {
return playerNames.stream()
.map(name -> new Player(name, deck.drawDouble()))
.map(name -> new Player(name, deck.drawDouble(), inputBetting(name)))
Copy link
Author

Choose a reason for hiding this comment

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

코멘트에 남겼던 부분의 연장선인데요, Names 객체가 생성 시 중복 검증이라는 로직 하나를 한 뒤 다시 getter로 꺼내줘서 List으로 처리하고 있어요. 중복 검증 하나 만으로도 불필요한 래핑이 아니라고 볼 수 있을까요?

Copy link

Choose a reason for hiding this comment

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

1단계 코멘트에도 남겼지만 도메인에 대한 로직이 도메인에 있는건 크게 어색하지 않은 거 같아요.
여기서 로직은 이름이 중복되면 안된다보다는 플레이어들의 이름이 중복되면 안된다가 적당하지 않을까요?
해당 도메인 로직을 처리하기 위해서는Players 객체가 필요하지 않을까요?

아마도 검증 이후에 getter를 사용해서 데이터를 꺼내쓰면 의미가 없지 않나에 대해서 고민하고 계신 거 같아요.
이 부분은 콜백 함수를 통해서 해결할 수 있습니다. 여유가 되신다면 한 번쯤 구현해보시고 지금 코드와 변경된 코드의 장단점을 비교해보는 것도 좋을 거 같네요.

Copy link
Author

Choose a reason for hiding this comment

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

자바에도 콜백이 있는줄은 처음 알았네요! 지금은 적용할 여유가 없을 것 같고 따로 공부를 해보도록 하겠습니다 :)

.collect(Collectors.toUnmodifiableList());
}

private Betting inputBetting(Name name) {
try {
return new Betting(InputView.inputBetMoney(name.getValue()));
} catch (IllegalArgumentException e) {
OutputView.printErrorMessage(e.getMessage());
return inputBetting(name);
}
}

private void proceed(CardDeck deck, Dealer dealer, List<Player> players) {
alertStart(dealer, players);
proceedPlayers(players, deck);
Expand Down Expand Up @@ -70,15 +80,15 @@ private void proceedPlayer(Player player, CardDeck deck) {

private HitRequest inputHitRequest(Player player) {
try {
return HitRequest.find(InputView.inputHitRequest(player.getName()));
return InputView.inputHitRequest(player.getName());
} catch (IllegalArgumentException e) {
OutputView.printErrorMessage(e.getMessage());
return inputHitRequest(player);
}
}

private void showStopReason(Player player) {
if (player.isBlackJack()) {
if (player.isBlackjack()) {
OutputView.printBlackJackMessage(player.getName());
return;
}
Expand All @@ -98,6 +108,6 @@ private void showResult(Dealer dealer, List<Player> players) {
OutputView.printCardResultMessage();
OutputView.printParticipantCards(new ParticipantResponse(dealer));
players.forEach(player -> OutputView.printParticipantCards(new ParticipantResponse(player)));
OutputView.printWinResult(GameResult.of(dealer, players));
OutputView.printGameResult(GameResult.of(dealer, players));
}
}
21 changes: 21 additions & 0 deletions src/main/java/blackjack/domain/Betting.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package blackjack.domain;

public class Betting {

private final int betMoney;

public Betting(int betMoney) {
validate(betMoney);
this.betMoney = betMoney;
}

private void validate(int betMoney) {
if (betMoney <= 0) {
throw new IllegalArgumentException("[ERROR] 베팅 금액은 0원 이하일 수 없습니다.");
}
}

public int getValue() {
return betMoney;
}
}
35 changes: 15 additions & 20 deletions src/main/java/blackjack/domain/GameResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,47 +2,42 @@

import blackjack.domain.participant.Dealer;
import blackjack.domain.participant.Player;
import java.util.Arrays;
import java.util.Collections;
import java.util.EnumMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;

public class GameResult {

private final Map<Outcome, Integer> dealerResult;
private final Map<String, Outcome> playersResult;
private final Map<String, Integer> earnings;

private GameResult(Map<Outcome, Integer> dealerResult, Map<String, Outcome> playersResult) {
this.dealerResult = Collections.unmodifiableMap(new EnumMap<>(dealerResult));
this.playersResult = Collections.unmodifiableMap(new LinkedHashMap<>(playersResult));
public GameResult(Map<String, Integer> earnings) {
this.earnings = Collections.unmodifiableMap(new LinkedHashMap<>(earnings));
}

public static GameResult of(Dealer dealer, List<Player> players) {
Map<Outcome, Integer> dealerResult = new EnumMap<>(Outcome.class);
Map<String, Outcome> playersResult = new LinkedHashMap<>();
Map<String, Integer> playersEarnings = new LinkedHashMap<>();

initDealerResult(dealerResult);
for (Player player : players) {
Outcome playerOutcome = Outcome.judge(player, dealer);
playersResult.put(player.getName(), playerOutcome);
dealerResult.merge(playerOutcome.getOpposite(), 1, Integer::sum);
int playerEarnings = calculateEarnings(player, playerOutcome);
int dealerEarnings = getDealerEarnings(playerEarnings);
playersEarnings.merge(dealer.getName(), dealerEarnings, Integer::sum);
playersEarnings.put(player.getName(), playerEarnings);
}

return new GameResult(dealerResult, playersResult);
return new GameResult(playersEarnings);
}

private static void initDealerResult(Map<Outcome, Integer> dealerResult) {
Arrays.stream(Outcome.values())
.forEach(value -> dealerResult.put(value, 0));
private static int getDealerEarnings(int playerEarnings) {
return playerEarnings * (-1);
}

public Map<Outcome, Integer> getDealerResult() {
return dealerResult;
private static int calculateEarnings(Player player, Outcome outcome) {
return (int) (player.getBetting().getValue() * outcome.getEarningsRate());
Copy link

Choose a reason for hiding this comment

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

이 부분 관련해서 코멘트 남겼는데요, 점점 더 도메인 로직들이 많아지네요. 한 번 확인부탁드립니다.

Copy link
Author

Choose a reason for hiding this comment

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

확인했습니다 😄 확실히 제가 봐도 로직이 이상하게 복잡해지네요. 최대한 로직을 줄이면서도 동작이 같도록 리팩토링 해볼게요 :)

Copy link

Choose a reason for hiding this comment

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

최대한 인스턴스 필드 수를 줄이면서 하자니 순환 참조가 일어났었어서 결국 인스턴스 필드는 3개 이상 만들지 않는다는 규칙을 깨고(사실 Participant의 dto에서 이미 깨지긴 했습니다만) 플레이어에 Betting 클래스를 넣어 주었습니다.

너 얼마 벌었어?라는 질문에 대답해줄 수 있는 객체는 누구일까요? Player가 이 질문에 대한 답을 해줄 수 있을 거 같지 않나요? 이를 코드로 변환한다면 player.calculateProfit(outcome)처럼 나타낼 수 있을 거 같아요.
조금 더 들어가면 player.calculateProfit(dealer)와 같이도 표현될 수 있는데 지금 구조에서는 위 방법이 조금 더 적당해보이네요.
이렇게 되면 Player가 배팅 금액을 아는 게 자연스러워집니다. 그리고 가이드가 정확히 어떻게 되는지 기억은 안나지만 아마 3개까지 허용하는 걸로 기억해요. 잘 구현해주셨습니다.

Copy link
Author

@Ohzzi Ohzzi Mar 17, 2022

Choose a reason for hiding this comment

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

생각해보니 이걸 static 메서드로 뺄 필요가 없었네요! :)

}

public Map<String, Outcome> getPlayersResult() {
return playersResult;
public Map<String, Integer> getEarnings() {
return earnings;
}
}
40 changes: 20 additions & 20 deletions src/main/java/blackjack/domain/Outcome.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,50 +4,50 @@
import blackjack.domain.participant.Player;

public enum Outcome {
WIN("승"),
DRAW("무"),
LOSE("패");
WIN("승", 1),
DRAW("무", 0),
LOSE("패", -1),
WIN_BLACKJACK("승", 1.5);
Copy link

Choose a reason for hiding this comment

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

승리를 했을 때는 2배의 금액을 돌려받고 블랙잭일 경우 1.5배를 돌려받습니다.
이율은 0.5배 아닐까요?

Copy link
Author

Choose a reason for hiding this comment

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

처음에 좀 헷갈리긴 했는데 금액으로 생각한다면 본인이 원래 베팅한 금액 + 수익 으로 해서 이겼을 때 2배를 받는거가 생각해보면 맞기는 한데, 여기서 수익으로 계산하는 부분은 원래 베팅한 금액을 배제한 금액이 되는 것 같더라고요!(그래서 LOSE면 베팅 금액을 잃으니까 수익이 -가 되는 상황이 되는거죠)

Copy link

Choose a reason for hiding this comment

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

아하 제가 룰을 오해하고 있었네요 🥲
블랙잭은 운이 좋아서 걸린거니 0.5배의 수익만 가져가는 거라고 이해하고 있었습니다.


private final String name;
private final double earningsRate;

Outcome(String name) {
Outcome(String name, double earningsRate) {
this.name = name;
this.earningsRate = earningsRate;
}

public static Outcome judge(Player player, Dealer dealer) {
if (player.isBust() || !player.isBlackJack() && dealer.isBlackJack()) {
if (player.isBlackjack() && !dealer.isBlackjack()) {
return WIN_BLACKJACK;
}
if (player.isBust() || !player.isBlackjack() && dealer.isBlackjack()) {
return Outcome.LOSE;
}
if (dealer.isBust() || player.isBlackJack() && !dealer.isBlackJack()) {
if (dealer.isBust() || player.isBlackjack() && !dealer.isBlackjack()) {
return Outcome.WIN;
}
if (player.isBlackJack() && dealer.isBlackJack()) {
if (player.isBlackjack() && dealer.isBlackjack()) {
return Outcome.DRAW;
}
return judgeByScore(player.getScore(), dealer.getScore());
}

private static Outcome judgeByScore(int score, int target) {
if (score > target) {
private static Outcome judgeByScore(Score score, Score target) {
if (score.compareTo(target) > 0) {
return Outcome.WIN;
}
if (score == target) {
if (score.equals(target)) {
return Outcome.DRAW;
}
return Outcome.LOSE;
}

public Outcome getOpposite() {
if (this == WIN) {
return LOSE;
}
if (this == LOSE) {
return WIN;
}
return DRAW;
}

public String getName() {
return name;
}

public double getEarningsRate() {
return earningsRate;
}
}
11 changes: 10 additions & 1 deletion src/main/java/blackjack/domain/Score.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package blackjack.domain;

public class Score {
public class Score implements Comparable<Score> {
Copy link

Choose a reason for hiding this comment

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

객체지향 생활 체조? 를 보면 한 줄에 점(.)은 하나만 찍으라고 되어 있는데, 지금 cards.calculateScore().getValue()로 두 번 꺼내서 쓰고 있더라고요. 우선 cards.getCards().size()는 Cards에 size를 만들면 해결이 될 것 같은데, Cards가 Score 클래스의 value 값이 int라는걸 알지 못하게 하면서 여기서 getter를 한 단계 줄일만한 좋은 방법이 생각이 나지가 않아요 ㅠㅠ 이 경우에는 점(.) 찍는 규칙을 위반하면서 짜는게 나은걸까요?

getter가 등장하면 로직을 객체 안으로 넣을 수 있는지 확인해보면 좋습니다. score.isBust() score.canBlackjack()과 같은 메서드로 감쌀 수 있을 거 같아요.

그리고나서 추천드리는 건 State 클래스를 제거해보세요. Cards 클래스의 메서드만으로도 충분히 깔끔하게 표현이 될 거 같네요.
아마 State란 단어를 사용해주신 걸 보면 상태 패턴에 대해서 알고 계신게 아닐까 추측되는데 #225 (comment) 이전에 말씀드렸던 거처럼 이 부분은 머지할 때 피드백을 드리는 게 좋을 거 같아요. 우선 State 클래스를 제거하는 걸 추천드립니다!

Copy link
Author

Choose a reason for hiding this comment

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

사실 작성할 때는 상태 패턴을 알고 작성한건 아니고 어떻게 하면 클래스를 뺄 수 있을까 생각하긴 한거였는데...ㅎ
안그래도 이 부분은 State 클래스가 꼭 필요할까 고민했던 부분이었어요! 그런데 어차피 isBust나 isBlackjack으로 호출을 할테니 별 의미가 없겠네요! 제거하는걸 고려해볼게요!


private final int value;

Expand All @@ -12,6 +12,15 @@ public int getValue() {
return value;
}

public boolean isLessThan(int value) {
return this.value < value;
}
Copy link
Author

Choose a reason for hiding this comment

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

제가 Comparable을 구현하긴 했는데요, 자체적으로 isLessThan을 쓸 일이 있어서 만들었습니다.
그런데 같은 Score 객체와 비교하는 코드가 아니라 value 값만 비교하는 코드로 사용해도 괜찮겠죠?
Score끼리 비교는 일단 compareTo로 된다고 생각해서요!

Copy link

Choose a reason for hiding this comment

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

Comparator를 사용하지 않는데 Comparable을 구현할 필요는 없는 것 같아요.
isLessThan(Score) isLessThan(int) 처럼 여러 타입에 대한 비교를 허용하는 건 좋아보이는데요, 비교하는 방법이 통일되면 좀 더 이해하기 쉬울 거 같아요.

Copy link
Author

Choose a reason for hiding this comment

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

혹시 방법이 통일된다는게 정확히 어떤 말씀을 말하시는건가요?
사실 지금 정확하게 이해가 가지 않네요 😢

Copy link

Choose a reason for hiding this comment

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

지금과 같은 구조를 의도하는 코멘트였습니다. 제대로 표현을 못했네요 🥲

사실 이 방법이나 compareTo나 가독성이 그리 좋지 않습니다. 이 부분은 자바라는 언어가 갖고 있는 한계이기 때문에 그러려니하고 넘어가시면 될 거 같아요. 연산자 오버로딩을 지원하는 다른 언어를 사용하면 해결될 문제입니다.


@Override
public int compareTo(Score target) {
return Integer.compare(value, target.value);
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand Down
31 changes: 31 additions & 0 deletions src/main/java/blackjack/domain/card/Card.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
package blackjack.domain.card;

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

public class Card {

private static final List<Card> pool = createCardPool();

private final Pattern pattern;
private final Denomination denomination;

Expand All @@ -10,6 +17,30 @@ public Card(Pattern pattern, Denomination denomination) {
this.denomination = denomination;
}

public static Card of(Pattern pattern, Denomination denomination) {
return pool.stream()
.filter(card -> card.pattern == pattern && card.denomination == denomination)
.findAny()
.orElseThrow(() -> new IllegalArgumentException("[ERROR] 올바르지 않은 카드입니다."));
}

private static List<Card> createCardPool() {
return Arrays.stream(Pattern.values())
.map(Card::createCardsBy)
.flatMap(Collection::stream)
.collect(Collectors.toList());
}

private static List<Card> createCardsBy(Pattern pattern) {
return Arrays.stream(Denomination.values())
.map(denomination -> new Card(pattern, denomination))
.collect(Collectors.toList());
}

public static List<Card> getPool() {
return pool;
}
Copy link
Author

Choose a reason for hiding this comment

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

고민을 조금 했던 부분인데요, pool을 static으로 유지해서 deck에서 가져다 쓰는 것과 getPool 이라는 static 메서드를 열어두는 것 중 어느 것이 더 나은 방법일까요?

Copy link

Choose a reason for hiding this comment

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

lazy한 초기화를 하는 등 특별히 getter가 하는게 없다면 따로 둘 필요는 없어보이긴 하네요.
이런 부분은 컨벤션의 문제인 것 같습니다.


public Pattern getPattern() {
return pattern;
}
Expand Down
Loading