-
Notifications
You must be signed in to change notification settings - Fork 0
Step2 놀러와요 누누의 step2 블랙잭에 #9
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: main
Are you sure you want to change the base?
Changes from 22 commits
dc2a806
aaf1160
86d3127
9bd63e2
092f1ab
bdd40b3
2931e75
3c963fe
886d8ae
acaf20f
adc2c80
36eb098
6e00bc0
077a5d4
2547f72
be4591d
f14ba4e
7be4015
b7f060a
6e3773c
618f993
9a4e3ab
27a8fed
7a8079d
fe7fb38
680486e
a6dd7ac
7cc4e18
35e55fb
0048059
359f18c
e3750dc
a6c01f0
0672801
0fa0ba9
e621337
8feda99
ed4540b
b4ddb0f
5a18897
5126cca
dd80fd6
6559e0a
30fad82
21d84e0
1dda7c0
2914060
7d16dcf
805def5
f677e7e
6df66b3
948dea8
1a1bef8
dbf6028
87d1212
68fa3ab
165c340
28d1fb8
40d04b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package blackjack; | ||
|
|
||
| import blackjack.controller.BlackJackController; | ||
| import blackjack.domain.card.ShuffledDeckFactory; | ||
| import blackjack.view.InputView; | ||
| import blackjack.view.OutputView; | ||
|
|
||
| public class Application { | ||
|
|
||
| public static void main(final String[] args) { | ||
| final BlackJackController blackJackController = new BlackJackController(new InputView(), new OutputView()); | ||
| blackJackController.play(new ShuffledDeckFactory()); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| package blackjack.controller; | ||
|
|
||
| import static blackjack.controller.DomainConverter.convertCard; | ||
| import static blackjack.controller.DomainConverter.convertCards; | ||
| import static blackjack.controller.DomainConverter.convertPlayersCards; | ||
| import static blackjack.controller.DomainConverter.getPlayerCards; | ||
| import static blackjack.util.Repeater.repeatUntilNoException; | ||
|
|
||
| import blackjack.domain.BlackJackRuleImpl; | ||
| import blackjack.domain.card.DeckFactory; | ||
| import blackjack.service.BlackJackGame; | ||
| import blackjack.view.DrawCommand; | ||
| import blackjack.view.InputView; | ||
| import blackjack.view.OutputView; | ||
| import java.util.List; | ||
|
|
||
| public class BlackJackController { | ||
|
|
||
| private final InputView inputView; | ||
| private final OutputView outputView; | ||
|
|
||
| public BlackJackController(final InputView inputView, final OutputView outputView) { | ||
| this.inputView = inputView; | ||
| this.outputView = outputView; | ||
| } | ||
|
|
||
| //이 메서드는 10라인도 훨씬 넘었고, 되게 복잡하게 되어있는데, 이렇게 두는 것이 좋은가요? | ||
| //원래대로 돌렸을 경우에, 훨씬 간단해보였던 것 같아요 | ||
| // public void play(final DeckFactory deckFactory) { | ||
| // final BlackJackGame blackJackGame = repeatUntilNoException( | ||
| // () -> BlackJackGame.of( | ||
| // inputPlayerNames(), | ||
| // deckFactory, | ||
| // new BlackJackRuleImpl()), | ||
| // outputView::printError); | ||
| // | ||
| // for (final String playerName : blackJackGame.getPlayerNames()) { | ||
| // blackJackGame.addPlayerMoney(playerName, inputPlayerMoney(playerName)); | ||
| // } | ||
| // | ||
| // blackJackGame.distributeInitialCard(); | ||
| // | ||
| // outputView.printInitialCards( | ||
| // convertCard(blackJackGame.getDealerFirstCard()), | ||
| // getPlayerCards(blackJackGame.getPlayers())); | ||
| // | ||
| // for (final String playerName : blackJackGame.getPlayerNames()) { | ||
| // DrawCommand playerChoice = DrawCommand.DRAW; | ||
| // while (blackJackGame.isPlayerDrawable(playerName) && playerChoice != DrawCommand.STAY) { | ||
| // playerChoice = inputPlayerChoice(playerName); | ||
| // if (playerChoice == DrawCommand.DRAW) { | ||
| // blackJackGame.drawPlayerCard(playerName); | ||
| // } | ||
| // outputView.printCardStatusOfPlayer(playerName, convertCards(blackJackGame.getPlayerCards(playerName))); | ||
| // } | ||
| // } | ||
| // | ||
| // while (blackJackGame.isDealerDrawable()) { | ||
| // blackJackGame.drawDealerCard(); | ||
| // outputView.printDealerCardDrawMessage(); | ||
| // } | ||
| // | ||
| // outputView.printFinalStatusOfDealer(blackJackGame.getDealerScore(), | ||
| // convertCards(blackJackGame.getDealerCards())); | ||
| // outputView.printFinalStatusOfPlayers(convertPlayersCards(blackJackGame.getPlayersCards()), | ||
| // blackJackGame.getPlayersScores()); | ||
| // | ||
| // outputView.printFinalMoney(blackJackGame.calculatePlayersMoney()); | ||
| // } | ||
|
|
||
| //한 메서드가 너무 길어지는 느낌이 드는 것 같은데요 이것은 어떤가요? | ||
| public void play(final DeckFactory deckFactory) { | ||
| final BlackJackGame blackJackGame = repeatUntilNoException( | ||
| () -> BlackJackGame.of( | ||
| inputPlayerNames(), | ||
| deckFactory, | ||
| new BlackJackRuleImpl()), | ||
| outputView::printError); | ||
| for (final String playerName : blackJackGame.getPlayerNames()) { | ||
| blackJackGame.addPlayerMoney(playerName, inputPlayerMoney(playerName)); | ||
| } | ||
|
|
||
| blackJackGame.distributeInitialCard(); | ||
| outputView.printInitialCards( | ||
| convertCard(blackJackGame.getDealerFirstCard()), | ||
| getPlayerCards(blackJackGame.getPlayers())); | ||
|
|
||
| for (final String playerName : blackJackGame.getPlayerNames()) { | ||
| drawPlayerCard(blackJackGame, playerName); | ||
| } | ||
| while (blackJackGame.isDealerDrawable()) { | ||
| blackJackGame.drawDealerCard(); | ||
| outputView.printDealerCardDrawMessage(); | ||
| } | ||
|
|
||
| outputView.printFinalStatusOfDealer(blackJackGame.getDealerScore(), | ||
| convertCards(blackJackGame.getDealerCards())); | ||
| outputView.printFinalStatusOfPlayers(convertPlayersCards(blackJackGame.getPlayersCards()), | ||
| blackJackGame.getPlayersScores()); | ||
| outputView.printFinalMoney(blackJackGame.calculatePlayersMoney()); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 적절한 기능별로 메서드로 묶어서 추상화해보는 것은 어떨까요?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 리뷰어의 말을 듣고 이렇게 해보았는데요
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 컨트롤러에서, private 메서드로 작업들을 하는데, 그 과정에서 blackjackGame 이 계속 인자로 넘어가다보니 가독성이 안좋은 것 같다는 말을 듣고, 이렇게 작업을 했었는데요
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 확실히 blackjackGame이 게속 인자로 넘어가니 확실히 가독성이 안좋은 느낌이기도 하네 확실히 메서드 네이밍에 신경쓴다면 읽는데 큰 문제가 없을 것 같은 느낌? |
||
| } | ||
|
|
||
| private void drawPlayerCard(final BlackJackGame blackJackGame, final String playerName) { | ||
| DrawCommand playerChoice = DrawCommand.DRAW; | ||
| while (blackJackGame.isPlayerDrawable(playerName) && playerChoice != DrawCommand.STAY) { | ||
| playerChoice = inputPlayerChoice(playerName); | ||
| if (playerChoice == DrawCommand.DRAW) { | ||
| blackJackGame.drawPlayerCard(playerName); | ||
| } | ||
| outputView.printCardStatusOfPlayer(playerName, convertCards(blackJackGame.getPlayerCards(playerName))); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2depth 😢
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이것도 리뷰어가... blackJackGame 을 인자로 넘기지 않았으면 좋겠다는 말을 듣고 시작하게 되었는데요 if 문을 물론 메서드로 분리할 수는 있겠지만, 의미를 못 드러내는 것 같아서 그냥 이대로 두어보려고 해요
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. depth는 사실 연습을 위한 도구인 느낌이라 합당한 이유가 있다면 괜찮다고 생각해 |
||
| } | ||
| } | ||
|
|
||
| private int inputPlayerMoney(final String playerName) { | ||
| return repeatUntilNoException( | ||
| () -> inputView.inputPlayerMoney(playerName), outputView::printError); | ||
| } | ||
|
|
||
| private DrawCommand inputPlayerChoice(final String playerName) { | ||
| return repeatUntilNoException( | ||
| () -> inputView.inputCommand(playerName), outputView::printError); | ||
| } | ||
|
|
||
| private List<String> inputPlayerNames() { | ||
| return inputView.inputPlayerNames(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| package blackjack.controller; | ||
|
|
||
| import blackjack.domain.card.Card; | ||
| import blackjack.domain.participant.Players; | ||
| import blackjack.response.CardResponse; | ||
| import java.util.HashMap; | ||
| import java.util.LinkedHashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| //이 클래스는 controller 의 역할이지만, controller 에다가 두면, 너무 많은 역할을 하게 되는 것 같아서 분리하였습니다 | ||
| //이 클래스가 단순하게 유틸리티 비슷한 클래스인데, 이대로 괜찮을지 여쭤보고 싶어요 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DTO로 변환시키는 것도 controller의 책임이라고 생각이 되는데
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 생각해보면, 지금은 도메인 객체를 그냥 내보냈는데, 사실 도메인 객체를 보호하기 위해서, 바로 안 내보낸다고 response를 내보내준다고 생각하면 response 객체를 blackjackGame 에서 바로 내보내도 괜찮을지도...?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 그런 고민 좋은데요? |
||
| class DomainConverter { | ||
|
|
||
| private DomainConverter() { | ||
| } | ||
|
|
||
| static CardResponse convertCard(final Card card) { | ||
| return CardResponse.from(card); | ||
| } | ||
|
|
||
| static List<CardResponse> convertCards(final List<Card> cards) { | ||
| return cards.stream() | ||
| .map(CardResponse::from) | ||
| .collect(Collectors.toList()); | ||
| } | ||
|
|
||
| static Map<String, List<CardResponse>> getPlayerCards(final Players players) { | ||
| final List<String> playerNames = players.getPlayerNames(); | ||
| final Map<String, List<CardResponse>> playerCards = new HashMap<>(); | ||
| for (final String playerName : playerNames) { | ||
| final List<CardResponse> cardResponses = players.findPlayerByName(playerName) | ||
| .getCards() | ||
| .stream() | ||
| .map(CardResponse::from) | ||
| .collect(Collectors.toList()); | ||
| playerCards.put(playerName, cardResponses); | ||
| } | ||
| return playerCards; | ||
| } | ||
|
|
||
| static Map<String, List<CardResponse>> convertPlayersCards(final Map<String, List<Card>> playersCards) { | ||
| return playersCards.entrySet().stream() | ||
| .collect(Collectors.toMap( | ||
| Map.Entry::getKey, | ||
| entry -> entry.getValue().stream() | ||
| .map(CardResponse::from) | ||
| .collect(Collectors.toList()), | ||
| (oldValue, newValue) -> newValue, | ||
| LinkedHashMap::new | ||
| )); | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. List에서 DTO로 변환시키는 부분이 중복되는 것 같아서 다음과 같이 개선할 수 있을 것 같아~ static Map<String, List<CardResponse>> getPlayerCards(final Players players) {
return players.getPlayerNames.stream()
.collect(toMap(
Function::identity,
name -> generateCardResponses(players.findPlayerByName(name).getCards())
));
}
static List<CardResponse> generateCardResponses(final List<Card> cards) {
return cards.stream()
.map(CardResponse::from)
.collect(Collectors.toList());
}
static Map<String, List<CardResponse>> convertPlayersCards(final Map<String, List<Card>> playersCards) {
return playersCards.keySet().stream()
.collect(toMap(
Function.identity(),
name -> generateCardResponses(playersCards.get(name)),
(a, b) -> a,
LinkedHashMap::new
));
}
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이거 고마워 바로 적용했어 |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| package blackjack.domain; | ||
|
|
||
| import blackjack.domain.participant.Dealer; | ||
| import blackjack.domain.participant.Player; | ||
|
|
||
| public interface BlackJackRule { | ||
|
|
||
| ResultType calculateDealerResult(Dealer dealer, Player player); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| package blackjack.domain; | ||
|
|
||
| import blackjack.domain.participant.Dealer; | ||
| import blackjack.domain.participant.Player; | ||
|
|
||
| /** | ||
| * 이 클래스가 룰이 자주 변경될 수 있기에, 인터페이스를 둔다는 점은 좋은데요 | ||
| * <p> | ||
| * 사실상 한 클래스만 존재하는 상황에서, 이를 interface로 빼는 것은 별로 의미가 없을 수 있어보이는데, 어떻게 생각하시나요? | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 인터페이스가 필요하지 않은 상황에서 사용하는 것은 별로라고 생각해!
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 라는 피드백을 듣고 interface로 분리하고 작업을 해보았는데요 |
||
| */ | ||
| public class BlackJackRuleImpl implements BlackJackRule { | ||
|
|
||
| private static final int BUST_POINT = 21; | ||
|
|
||
| @Override | ||
| public ResultType calculateDealerResult(final Dealer dealer, final Player player) { | ||
| //이 부분이 애매해서 리팩토링을 해보려고 했는데, 더 좋은 방법이 생각나지 않아서 일단 이렇게 구현했습니다. | ||
| //enum 클래스에 BiPredicate를 추가해서 구현해보려고 했는데, enum 클래스에 메서드를 추가하는 것이 맞는지 잘 모르겠습니다. | ||
| if (isBlackJackWin(dealer, player)) { | ||
| return ResultType.BLACKJACK_WIN; | ||
| } | ||
| if (isBlackJackLose(dealer, player)) { | ||
| return ResultType.BLACKJACK_LOSE; | ||
| } | ||
| final int dealerScore = dealer.currentScore(); | ||
| final int playerScore = player.currentScore(); | ||
| if (isTie(dealerScore, playerScore)) { | ||
| return ResultType.TIE; | ||
| } | ||
| if (isDealerWin(dealerScore, playerScore)) { | ||
| return ResultType.WIN; | ||
| } | ||
| return ResultType.LOSE; | ||
| } | ||
|
|
||
| private boolean isBlackJackWin(final Dealer dealer, final Player player) { | ||
| return dealer.hasBlackJack() && !player.hasBlackJack(); | ||
| } | ||
|
|
||
| private boolean isBlackJackLose(final Dealer dealer, final Player player) { | ||
| return !dealer.hasBlackJack() && player.hasBlackJack(); | ||
| } | ||
|
|
||
| private boolean isTie(final int dealerScore, final int playerScore) { | ||
| if (playerScore > BUST_POINT && dealerScore > BUST_POINT) { | ||
| return true; | ||
| } | ||
| return playerScore == dealerScore; | ||
| } | ||
|
|
||
| private boolean isDealerWin(final int dealerScore, final int playerScore) { | ||
| if (playerScore > BUST_POINT) { | ||
| return true; | ||
| } | ||
| if (dealerScore > BUST_POINT) { | ||
| return false; | ||
| } | ||
| return dealerScore > playerScore; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 딜러 기준으로 계산하는 것 같은데 (여기서는)딜러의 블랙잭 승은 의미없는 것 같아. public ResultType calculateDealerResult(final Dealer dealer, final Player player) {
if (dealer.hasBlackJack()) {
return playWithBlackjack(player);
}
if (dealer.currentScore() > BUST_POINT) {
return playWithBust(player);
}
return playWithScore(dealer, player);
}
private ResultType playWithBlackjack(final Player player) {
if (player.hasBlackJack()) {
return ResultType.TIE;
}
return ResultType.WIN
}
private ResultType playWithBust(final Player player) {
if (player.currentScore() > BUST_POINT) {
return ResultType.WIN;
}
if (player.hasBlackJack()) {
return ResultType.BLACKJACK_LOSE;
}
return ResultType.LOSE;
}
private ResultType playWithScore(final Dealer dealer, final Player player) {
if (player.hasBlackJack()) {
return ResultType.BLACKJACK_LOSE;
}
if (player.currentScore() > BUST_POINT || dealer.currentScore() > player.currentScore()) {
return ResultType.WIN;
}
return ResultType.LOSE;
}
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 일단 이렇게 바꿔봤는데 enum 쪽으로 이 메서드들을 넣어볼지 말지 고민해보아야 될 것 같긴 하네
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. enum으로 넣는다.. 음.. 감이 안잡히네 |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| package blackjack.domain; | ||
|
|
||
| public enum ResultType { | ||
| BLACKJACK_WIN(1.5) { | ||
| @Override | ||
| public ResultType getOppositeResult() { | ||
| return BLACKJACK_LOSE; | ||
| } | ||
| }, | ||
| WIN(1) { | ||
| @Override | ||
| public ResultType getOppositeResult() { | ||
| return LOSE; | ||
| } | ||
| }, | ||
| TIE(0) { | ||
| @Override | ||
| public ResultType getOppositeResult() { | ||
| return TIE; | ||
| } | ||
| }, | ||
| LOSE(-1) { | ||
| @Override | ||
| public ResultType getOppositeResult() { | ||
| return WIN; | ||
| } | ||
| }, | ||
| BLACKJACK_LOSE(-1) { | ||
| @Override | ||
| public ResultType getOppositeResult() { | ||
| return BLACKJACK_WIN; | ||
| } | ||
| }; | ||
|
|
||
| private final double playerProfit; | ||
|
|
||
| ResultType(final double playerProfit) { | ||
| this.playerProfit = playerProfit; | ||
| } | ||
|
|
||
| public abstract ResultType getOppositeResult(); | ||
|
|
||
| public double getPlayerProfit() { | ||
| return playerProfit; | ||
| } | ||
| } |

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.
main 메소드가 길어서 전 안읽었는데 분리하는 것은 어때요? 분리 안한 이유가 궁금합니다
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.
ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ
읽어주세요
#9 (comment)
여기서 볼 수 있는 것처럼 나름의 고민을 했는데, 그냥 분리 안 했어요