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단계 - 블랙잭(베팅)] 테오(최우성) 미션 제출합니다. #527

Merged
merged 36 commits into from
Mar 19, 2023
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
917f950
refactor: 딜러의 이름은 예약어이기 때문에 딜러 객체에서 관리
woosung1223 Mar 8, 2023
fb46c3c
refactor: 의도된 예외만 catch하도록 변경
woosung1223 Mar 8, 2023
d88b82f
refactor: 불필요한 협력 단축
woosung1223 Mar 8, 2023
a5e4170
refactor: 카드를 랜덤으로 뽑을 때, 원본 카드 리스트를 수정하지 않도록 변경
woosung1223 Mar 8, 2023
4d74e40
feat: 딜러는 처음에 카드 한 장을 숨긴다
woosung1223 Mar 8, 2023
7bafa7e
chore: 메소드 선언 순서 조정 및 사용하지 않는 메소드 제거
woosung1223 Mar 8, 2023
aad2642
docs: 새로운 요구사항을 반영해 기능목록 최신화
woosung1223 Mar 8, 2023
e850be5
feat: 배팅 금액을 관리하는 객체 생성
woosung1223 Mar 9, 2023
cce418b
feat: 플레이어는 배팅금액을 가질 수 있다
woosung1223 Mar 9, 2023
ffbcd20
chore: 도메인 패키지 구조와 테스트 패키지 구조가 일치하도록 변경
woosung1223 Mar 9, 2023
baadb43
refactor: 이익까지 계산할 수 있기에 클래스명을 조금 더 추상적인 개념으로 변경
woosung1223 Mar 9, 2023
2b3c1a8
feat: 플레이어나 딜러가 이기면 배팅 금액이 조정된다
woosung1223 Mar 9, 2023
7e74cd6
feat: 플레이어와 딜러 모두 블랙잭인 경우 배팅 금액은 조정되지 않는다
woosung1223 Mar 9, 2023
58af9fb
feat: 배팅 금액을 넣는 기능 생성
woosung1223 Mar 9, 2023
b84edd1
refactor: 배팅 금액은 0보다 작거나 같을 수 있도록 변경
woosung1223 Mar 9, 2023
049716f
refactor: 딜러가 deposit을 가지도록 변경
woosung1223 Mar 10, 2023
8c99d4b
feat: 배팅 금액을 입력받는 기능 생성
woosung1223 Mar 10, 2023
3bf36e2
feat: 플레이어 및 딜러 수익 계산기능 완성
woosung1223 Mar 10, 2023
f4fdd0c
refactor: 메소드 분리 및 선언 순서 조정
woosung1223 Mar 10, 2023
e4c38d0
test: 게임 결과에 대한 테스트케이스 추가
woosung1223 Mar 11, 2023
7f1c46e
refactor: 승패 출력 제거
woosung1223 Mar 11, 2023
0c743ae
refactor: enum 객체와의 직접 비교 삭제
woosung1223 Mar 11, 2023
67ad274
refactor: Fuxture를 통한 테스트코드 가독성 개선
woosung1223 Mar 11, 2023
33776c2
docs: 고민사항 작성
woosung1223 Mar 11, 2023
741daf0
refactor: 테스트 코드 포맷팅, Parameterized 사용 가독성 개선
woosung1223 Mar 11, 2023
4d0b87d
refactor: 요구사항 변경에 따라 필요 없는 메소드 삭제
woosung1223 Mar 11, 2023
2b03754
docs: 의존성 그래프 추가, 고민사항 추가 기재
woosung1223 Mar 11, 2023
c883c86
chore: TODO 삭제
woosung1223 Mar 11, 2023
54d484b
refactor: 덱을 섞는 메소드의 네이밍 변경
woosung1223 Mar 12, 2023
7ed05fa
chore: 파라미터 final화
woosung1223 Mar 12, 2023
d7f9fcb
refactor: 카드 합을 계산하는 메소드 네이밍 수정
woosung1223 Mar 12, 2023
5bb69e0
refactor: 딜러 카드를 보여주는 메소드 네이밍 수정 및 구현 단순화
woosung1223 Mar 12, 2023
17e3295
refactor: 배팅 금액은 항상 0원부터 시작하도록 캡슐화
woosung1223 Mar 12, 2023
6339186
refactor: 변수 네이밍 변경
woosung1223 Mar 12, 2023
b1d6097
refactor: 딜러가 카드를 보여주는 기능에 대한 테스트 작성
woosung1223 Mar 12, 2023
fbbfc9c
feat: 예약어 관리가 용이하도록 enum 정의
woosung1223 Mar 12, 2023
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
70 changes: 53 additions & 17 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,14 @@
- [x] 플레이어
- [x] 이름을 가진다
- [x] 카드들을 가진다
- [x] 배팅 금액을 가질 수 있다
- [x] 가지고 있는 카드의 숫자를 계산할 수 있다
- [x] 다른 사람의 점수와 비교할 수 있다
- [x] 카드를 한장 받는다

- [x] 딜러
- [x] 딜러는 플레이어이다
- [x] 카드의 합이 임계값보다 작거나 같으면 히트이다
- [x] 딜러는 카드를 1장 덜 보여준다

- [x] 이름
- [x] 1~5자의 길이만 허용된다
Expand All @@ -46,33 +47,68 @@
- [x] 숫자를 가진다

- [x] 덱
- [x] 숫자 위치의 카드를 반환한다
- [x] 모든 카드를 가진다
- [x] 카드를 섞는다
- [x] 카드를 한 장 반환한다

- [x] 카드풀
- [x] 카드를 가질 수 있다
- [x] 카드 숫자의 합을 계산한다.
- [x] 에이스는 11일 때 숫자의 합이 21이 넘으면 1로 계산한다
- [x] 에이스가 있는지 판단할 수 있다.

- [x] 블랙잭 게임
- [x] 모든 참여자와 딜러의 승패를 가지고 있다
- [x] 모든 딜러와 참여자에게 카드를 뽑는 명령을 내린다
- [x] 모든 카드를 받아서 숫자를 비교한다
- [x] 덱을 초기화 한다
- [x] 21(버스트 숫자)을 넘으면 버스트(죽는다).

- [x] 숫자 생성기
- [x] 랜덤으로 숫자를 생성한다

## 객체 다이어그램

<img src="diagram.png" style="width: 80%; height= 80%">


- [x] 모든 딜러와 참여자에게 카드를 뽑는 명령을 내린다
- [x] 모든 참여자와 딜러의 승패를 알고 있다
- [x] 하나 이상의 플레이어가 이기면, 딜러는 각각의 플레이어의 총 배팅 금액만큼을 잃는다
- [ ] 어떤 플레이어의 처음 두 장의 카드가 블랙잭이면, 1.5배를 잃는다
- [ ] 딜러와 플레이어 모두가 블랙잭이라면 아무도 잃지 않는다
- [x] 딜러가 이기면, 상대 플레이어는 배팅 금액을 전부 잃는다

- [x] 카드 섞기 규칙
- [x] 랜덤으로 섞는다

## 객체 다이어그램(초기 설계)

<img src="diagram.png" style="width: 80%; height: 80%">

## Main Dependency Graph

```mermaid
classDiagram
BlackjackController-->ClientCommand
BlackjackController-->InputView
BlackjackController-->OutputView
BlackjackController-->BlackjackGame
BlackjackGame-->Deck
BlackjackGame-->GameParticipant
Deck-->ShuffleStrategy
GameParticipant-->Player
GameParticipant-->Dealer
GameParticipant-->GameResult
Player-->Bet
Player-->CardPool
Player-->PlayerName
Dealer-->Bet
Dealer-->CardPool
Dealer-->PlayerName

```

### 고민사항
- DeckMaker 객체를 둘지, Deck 안에서 생성 책임까지 가질지?
- 플레이어들을 getter를 사용해 가져와 반복문을 돌리는 경우가 있는데, Iterator 생성은 어떤가?
내부 구현은 숨기면서, 내부에 접근할 수 있다는 점에서 좋은 것 같은데, 만들기에는 너무 오버헤드가 큰지 고민임.
- 개발자들을 위해 존재하는 예외 메세지는 개발자만 이해할 수 있도록 정보를 제공해도 될까?
-

### 2단계 고민사항
- TDD 사이클에서 ParameterizedTest를 사용해도 될까? ParameterizedTest를 사용하면 여러 케이스를 동시에 검증하기에 TDD에 부적절하지는 않을까?
- `딜러는 처음에 한 장의 카드를 숨겨야 한다`는 요구사항을 잊고 있다가 구현하고 보니 getter에서 리스트를 변형하는 등 많은 작업을 하고 있는 것 같다. 괜찮을까?
- 요구사항 중 `인스턴스 변수를 3개 이상 쓰지 않는다`가 있었다. 하지만 플레이어가 배팅 금액을 가지는 과정에서 인스턴스 변수가 3개가 되어버린다. <br>
그렇다고 해서 배팅 금액을 Player에서 분리하느냐? 이것은 예측하기 어려운 설계라고 생각한다. 프로그래밍 요구사항을 어기는 것은 금기인가? 좋은 설계가 우선이 아닐까?
- void 타입 메소드를 선호하는데, 괜찮은걸까? 객체지향에서 사실 void 타입은 존재하지 않아도 되는데.
- fixture를 처음 사용함. 다만 fixture 안에 값에 맞는 enum 객체를 찾기 위한 로직이 들어가게 됨. 괜찮을까?
- blackjackGame 객체에서 `hitFor` 메소드에서 Player를 인자로 받기에, 바로 player.draw로 히트하는 책임을 수행할 수 있음. 다만 이렇게 하는 경우 <br>
blackjackGame에서 Player 객체와의 의존성이 생겨버림. blackjackGame은 gameParticipant에 인자만 넘겨주면서, 의존성을 줄이는게 좋은 전략 아닐까?
- 의존성은 import문의 개수로 취급되어야 할까? 그렇게 판단하면 의존성이 너무 많다고 생각하는데.. 잘못된 설계인가?
36 changes: 26 additions & 10 deletions src/main/java/controller/BlackJackController.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import domain.game.BlackjackGame;
import domain.strategy.RandomShuffleStrategy;
import domain.user.Dealer;
import domain.user.GameParticipant;
import domain.user.Player;
import view.InputView;
Expand All @@ -24,27 +25,38 @@ public void run() {
try {
initializeGame();
startGame();
} catch (RuntimeException e) {
} catch (IllegalArgumentException e) {
outputView.printExceptionMessage(e.getMessage());
}
}

private void initializeGame() {
List<String> playersName = inputView.inputParticipantsName();
blackjackGame = new BlackjackGame(playersName, "딜러", new RandomShuffleStrategy());
blackjackGame = new BlackjackGame(playersName, new RandomShuffleStrategy());
}

private void startGame() {
blackjackGame.startHit();
List<Player> players = blackjackGame.getGameParticipant().getPlayers();
Dealer dealer = blackjackGame.getGameParticipant().getDealer();

startBet(players);
startHit();
outputView.printPlayersInfoWhenGameStarted(dealer, players);

GameParticipant gameParticipant = blackjackGame.getGameParticipant();
outputView.printPlayersInfoWhenGameStarted(gameParticipant.getDealer(), gameParticipant.getPlayers());
letPlayerChoiceWhetherHit(players);
blackjackGame.updateBetAmount();
printGameResult(players, dealer);
}

letPlayerChoiceWhetherHit(gameParticipant.getPlayers());
outputView.printGameScore(gameParticipant.getDealer(), gameParticipant.getPlayers());
private void startBet(final List<Player> players) {
for (Player player : players) {
int amount = inputView.inputBetAmount(player.getPlayerName().getName());
player.addAmount(amount);
}
}

outputView.printDealerRecord(gameParticipant.getDealer(), blackjackGame.getDealerRecord());
outputView.printPlayerRecord(blackjackGame.getGameResultForAllPlayer());
private void startHit() {
blackjackGame.startHit();
}

private void letPlayerChoiceWhetherHit(List<Player> players) {
Expand All @@ -65,7 +77,7 @@ private void hitByPlayerChoice(Player player) {
}

private boolean needToQuit(Player player) {
if (blackjackGame.isBurst(player)) {
if (player.isBurst()) {
return true;
}

Expand All @@ -81,4 +93,8 @@ private void hitByDealer() {
}
}

private void printGameResult(final List<Player> players, final Dealer dealer) {
outputView.printGameScore(dealer, players);
outputView.printRevenueForAll(dealer, players);
}
}
18 changes: 4 additions & 14 deletions src/main/java/domain/game/BlackjackGame.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import domain.user.Player;

import java.util.List;
import java.util.Map;

public class BlackjackGame {

Expand All @@ -16,11 +15,10 @@ public class BlackjackGame {

public BlackjackGame(
final List<String> playerNames,
final String dealerName,
final ShuffleStrategy shuffleStrategy
) {
this.deck = new Deck(shuffleStrategy);
this.gameParticipant = new GameParticipant(playerNames, dealerName);
this.gameParticipant = new GameParticipant(playerNames);
}

public void letDealerHitUntilThreshold() {
Expand All @@ -37,20 +35,12 @@ public void startHit() {
}
}

public boolean isBurst(final Player player) {
return gameParticipant.isBurst(player);
}

public void hitFor(final Player player) {
gameParticipant.letPlayerToHit(player, deck);
}

public Map<Player, GameResult> getGameResultForAllPlayer() {
return gameParticipant.makeGameResultForAllPlayer();
player.draw(deck.serve());
}

public Map<GameResult, Integer> getDealerRecord() {
return gameParticipant.getDealerRecord(getGameResultForAllPlayer());
public void updateBetAmount() {
gameParticipant.updateBetAmountByGameResult();
}

public GameParticipant getGameParticipant() {
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/domain/game/Deck.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ private static List<Card> makeCardsOfOneCardType(final CardType cardType) {
}

public Card serve() {
shuffleStrategy.shuffle(cards);
return cards.get(TOP_CARD);
List<Card> shuffledCards = shuffleStrategy.shuffle(cards);
return shuffledCards.get(TOP_CARD);
}
}
8 changes: 8 additions & 0 deletions src/main/java/domain/game/GameResult.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,12 @@ public static Map<GameResult, Integer> makeDealerRecord(final Map<Player, GameRe
private static int countGameResult(final Collection<GameResult> gameResults, final GameResult gameResultType) {
return Collections.frequency(gameResults, gameResultType);
}

public boolean isWin() {
return this == WIN;
}

public boolean isLose() {
return this == LOSE;
}
}
7 changes: 5 additions & 2 deletions src/main/java/domain/strategy/RandomShuffleStrategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,16 @@

import domain.card.Card;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;

public class RandomShuffleStrategy implements ShuffleStrategy {

@Override
public void shuffle(final List<Card> cards) {
Collections.shuffle(cards);
public List<Card> shuffle(final List<Card> cards) {
begaonnuri marked this conversation as resolved.
Show resolved Hide resolved
List<Card> shuffledCards = new ArrayList<>(cards);
Collections.shuffle(shuffledCards);
return shuffledCards;
Comment on lines +13 to +15
Copy link
Member

Choose a reason for hiding this comment

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

자명한 라이브러리 이외의 로직이 추가되었으니 테스트가 필요해보입니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

저는 내부적으로 로직이 추가되었기는 하나, 단순히 생성 흐름이라 판단해서
테스트할 것이 없다고 생각했었습니다. 또한 무엇을 테스트해야 할지도 명확하지가 않다고 생각했어요.

고민해보았는데, 말씀해주신 부분에 대해 아직 제가 감을 잘 못잡는 것 같습니다 😅
혹시 조금만 더 짚어주실 수 있을까요!?

Copy link
Member

Choose a reason for hiding this comment

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

기존엔 Collections.shuffle(shuffledCards)로만 구성된 메서드였지만,

현재는 새로운 리스트를 생성해서 반환하는 테오만의 로직이 추가되었으니 이 부분에 대한 테스트가 필요하지 않을까요? 라는 리뷰였습니다. 예를들어 입력한 리스트와 반환된 리스트가 동일한 레퍼런스를 갖지 않는 테스트가 존재할 수 있겠네요!

}
}
2 changes: 1 addition & 1 deletion src/main/java/domain/strategy/ShuffleStrategy.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@

public interface ShuffleStrategy {

void shuffle(final List<Card> cards);
List<Card> shuffle(final List<Card> cards);
}
38 changes: 38 additions & 0 deletions src/main/java/domain/user/Bet.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package domain.user;

public class Bet {
begaonnuri marked this conversation as resolved.
Show resolved Hide resolved

private static final double REVENUE_BONUS_RATE = 1.5;

private int credit;
private int revenue;

public Bet(final int credit) {
begaonnuri marked this conversation as resolved.
Show resolved Hide resolved
this.credit = credit;
this.revenue = 0;
}

public void addAmount(int amount) {
this.credit += amount;
}

public void addRevenue() {
revenue += credit;
}

public void addBonusRevenue() {
revenue += credit * REVENUE_BONUS_RATE;
}

public void decreaseRevenue() {
revenue -= credit;
}

public void payFor(final Bet otherBet) {
revenue -= otherBet.revenue;
}

public int getRevenue() {
return revenue;
}
}
4 changes: 4 additions & 0 deletions src/main/java/domain/user/CardPool.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,8 @@ private boolean containsAce() {
public List<Card> getCards() {
return Collections.unmodifiableList(cards);
}

public boolean isSumSameAsLimit() {
return sumCardNumbers() == CARD_POINT_LIMIT;
begaonnuri marked this conversation as resolved.
Show resolved Hide resolved
}
}
17 changes: 15 additions & 2 deletions src/main/java/domain/user/Dealer.java
Original file line number Diff line number Diff line change
@@ -1,14 +1,27 @@
package domain.user;

import domain.card.Card;

import java.util.List;

public class Dealer extends Player {

private static final int DEALER_THRESHOLD = 16;
private static final int NUMBERS_OF_CARDS_TO_HIDE = 1;
private static final int DEPOSIT = 100_000;

private static final String DEFAULT_NAME = "딜러";
Copy link
Member

Choose a reason for hiding this comment

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

이전에 드렸던 리뷰에 공감되서 옮기신 거라면 플레이어의 이름 검증 부분이 추가되어야하지 않을까요? 그렇게되면 이 상수는 어디에 위치하는게 적절할까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

고민해 본 결과,

터틀이 말씀해주신 의도가 예약어이기 때문에 플레이어가 이를 사용할 수 없게 해야 한다임을 알았습니다. (맞겠죠..?!)

즉, 플레이어 이름을 향한 검증 로직이 '딜러'라는 예약어와 함께 존재해야 하고,
결국 '딜러'라는 리터럴은 PlayerName 객체에 존재해야 한다라는 결론에 도달했습니다.

하지만 '딜러'라는 이름이 PlayerName 객체에 존재하게 되면 Dealer는 더 이상 생성자를 통해 이름을 초기화할 수 없습니다.
따라서 protected 생성자를 정의하고, Dealer가 호출 가능하게 정의했습니다.

이 과정에서 PlayerName까지 생성자가 추가되었는데, 이런 접근방식은 처음이라 좋은 설계인지 궁금합니다!

그리고, 리터럴 상수로 "딜러"만 정의해두고 검증에서 이를 사용하는 것은 확장에 닫혀있다 생각하여 enum 객체로 분리하였는데 이것도 괜찮은 방법이었는지 궁금해요 🙇‍♂️

Copy link
Member

@begaonnuri begaonnuri Mar 19, 2023

Choose a reason for hiding this comment

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

많은 고민을 해주셨군요 👍🏻

간단히 PlayerNameGameParticipantName로 바꾸면 구현이 가능하지 않을까요? 🙂


public Dealer(final String playerName, final CardPool cardPool) {
super(playerName, cardPool);
public Dealer(final CardPool cardPool) {
super(DEFAULT_NAME, cardPool, DEPOSIT);
}

public boolean needsHit() {
return sumCardPool() <= DEALER_THRESHOLD;
}

public CardPool getHiddenCardPool() {
begaonnuri marked this conversation as resolved.
Show resolved Hide resolved
List<Card> originalCards = super.getCardPool().getCards();
return new CardPool(originalCards.subList(0, originalCards.size() - NUMBERS_OF_CARDS_TO_HIDE));
}
begaonnuri marked this conversation as resolved.
Show resolved Hide resolved
}
Loading