오잉의 블랙잭이노라 구경하노라 #12

hanueleee · 2023-03-11T06:59:47Z

No description provided.

* docs: 기능 목록 작성 * feat: 카드 숫자를 보고 점수를 반환하는 기능 구현 * docs: 추가 예외 사항 작성 * feat: 52장의 카드를 생성하는 기능 구현 * feat: 카드 뽑기 기능 구현 * feat: 초기 카드 2장을 지급받는 기능 구현 * feat: 플레이어의 카드 뽑기 기능 테스트 * feat: 플레이어 이름이 '딜러'이면 예외가 발생하는 기능 구현 * refactor: Player 카드 뽑기 테스트를 Challenger 테스트로 이동 * refactor: HoldingCards를 만들어 일급 컬렉션으로 객체 분리 * docs: 기능 목록 업데이트 * feat: 가진 카드의 합을 반환하는 기능 구현 * feat: challenger의 카드의 합이 21이 넘어가는지 확인하는 기능 구현 * refactor: 카드 뽑기 테스트를 Player 테스트로 이동 * feat: dealer의 카드의 합이 16이 넘어가는지 확인하는 기능 구현 * feat: 중복된 이름 예외 처리 기능 구현 * feat: 플레이어 이름 입력 기능 구현 * refactor: 초기 카드 세팅 메소드 파라미터 형식 변경 * feat: 도전자, 딜러 구분해주는 기능 구현 * feat: 초기 카드 지급 상태 출력 기능 구현 * feat: 카드 추가 뽑기 입력 기능 구현 * feat: 카드를 더 받지 않으면 턴이 끝나는 기능 구현 * feat: 카드를 더 받지 않으면 턴이 끝나는 기능 구현 * feat: 최종 카드 상태 출력 기능 구현 * feat: ace를 고려한 카드 합계 계산 로직 구현 * docs: 기능 목록에 Result 객체의 역할 추가 * feat: 플레이어들의 최종 승패 여부 계산하는 기능 구현 * feat: 최종 승패 출력 기능 구현 * feat: 더 이상 뽑을 카드가 없을 경우 카드 덱이 비어있다는 예외 발생 기능 구현 * refactor: indent 2인 메소드 indent 1로 줄이기 * refactor: 출력 줄바꿈 수정 * refactor: inline이 필요해보이는 부분 inline으로 수정 * refactor: 출력 줄바꿈 수정 * refactor: Dto 내부 변환 로직을 controller로 이동 (Dto 내부 단순화) * refactor: enum인 Number의 명칭을 Symbol로 변경 * refactor: CardDeck의 create메소드에 flatMap 적용 * refactor: 승패 판단 로직을 Dealer로 이동 * refactor: 카드 합 계산 로직 수정 * refactor: instanceof 대신 abstract method 사용 * refactor: 도메인에 의존하는 뷰 속 매직넘버 처리 * refactor: 도메인에 의존하는 문자열 처리 * refactor: 테스트 추가 및 수정 * refactor: CardDeck을 List에서 Deque으로 변경

…nto step2

greeng00se

오잉의 블랙잭 구경했도다.

greeng00se · 2023-03-11T14:35:07Z

src/main/java/blackjack/controller/BlackJackController.java

+        List<Challenger> challengers = players.getChallengers();
+        List<Money> betAmounts = new ArrayList<>();
+        challengers.stream()
+                .forEach(challenger -> betEachPlayer(challenger, betAmounts));


stream보다 for-each가 가독성이 더 좋을 것 같은데 어떻게 생각하시나요?

for (Challenger challenger: players.getChallengers()) { betEachPlayer(challenger, betAmounts)); }

리뷰어께서 stream 연습삼아 만들어보라고 하셔서 수정했던 부분임미다 ,, 🦹‍♂️
근데 저도 개인적으로 for-each 가 더 가독성이 좋을 것 같다고 생각합니당

greeng00se · 2023-03-11T15:39:05Z

src/main/java/blackjack/domain/game/Money.java

+    public static final Money ZERO = new Money(0);
+    private final int value;


상수와 필드는 한 칸 부탁드립니다

추가로 static 변수의 경우에도 public 먼저 작성하는게 우테코 컨벤션입니다

궁금한 점
public static final Money ZERO = new Money(0); 얘도 상수라고 보나요??

상수라고 말했지만, static 필드와 인스턴스 필드를 한 칸 구분하면 될 것 같습니다!

greeng00se · 2023-03-11T15:40:12Z

src/main/java/blackjack/domain/game/Money.java

+    public int getValue() {
+        return value;
+    }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o) {
+            return true;
+        }
+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        Money money = (Money) o;
+        return value == money.value;
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(value);


소소하지만 getter의 경우 equals & hashcode, toString 밑으로 위치하는게 컨벤션으로 알고 있습니다

아하 getter는 언제나 최하위에 위치해야한다는??
궁금한 점 : equals&hashcode, toString 말고 그냥 오버라이드한 함수들이어도 getter는 그 밑에 위치해야하나여??

넵 맞습니다!

greeng00se · 2023-03-11T15:42:56Z

src/main/java/blackjack/domain/player/Dealer.java

+    public static final String NAME = "딜러";
+
+    public static final int MAXIMUM_POINT = 16;


붙이는게 깔끔해보이네요

매의 눈 허브 🦅👀

greeng00se · 2023-03-11T15:44:27Z

src/main/java/blackjack/domain/player/Dealer.java

+    private Result resultWhenDealerIsBlackJack(Player challenger) {
+        if (challenger.isBust()) {
+            return Result.LOSE;
+        }
+        if (challenger.isBlackJack()) {
+            return Result.DRAW;
+        }
+        return Result.LOSE;
+    }


비교 로직이 깔끔해서 좋은 것 같습니다.

if (challenger.isBlackJack()) { return Result.DRAW; } return Result.LOSE;

로 개선할 수 있을 것 같습니다.

충격! 이걸 왜 놓쳤을까요 감사합니다

greeng00se · 2023-03-11T15:47:35Z

src/main/java/blackjack/view/InputView.java

+    }
+
+    public static int inputPlayerBetMoney(String name) {
+        System.out.println(name+ REQUEST_PLAYER_BET_MONEY);


name + REQUEST_PLAYER_BET_MONEY
IntelliJ action on save 설정해는건 어떨까요?

ㅎㅎ 감삼다

greeng00se · 2023-03-11T15:48:55Z

src/main/java/blackjack/domain/player/Player.java

+    public boolean isBust() {
+        if (status == Status.BUST) {
+            return true;
+        }
+        return false;
+    }
+
+    public boolean isBlackJack() {
+        if (status == Status.BLACKJACK) {
+            return true;
+        }
+        return false;
+    }


return status == Status.BUST

아래도 동일하게 개선할 수 있을 것 같아요 👍

감사합니다 허브좌

woo-chang

수고하셨습니다 한 수 배워갑니다 :)

woo-chang · 2023-03-12T12:07:35Z

src/main/java/blackjack/controller/BlackJackController.java

+    private PlayerStatusDto makePlayerStatusDto(Player player) {
+        String playerName = player.getName();
+        List<Card> inputCards = player.getHoldingCards().getCards();
+        int playerPoint = player.getTotalPoint();
+        return new PlayerStatusDto(playerName, makeCardInfo(inputCards), playerPoint);
+    }


dto로 해당 역할을 넘길 수 있을 것 같습니다!

woo-chang · 2023-03-12T12:16:37Z

src/main/java/blackjack/domain/player/Players.java

+                .filter(player -> !player.isDealer())
+                .map(player -> (Challenger) player)
+                .collect(Collectors.toList());
+        return new ArrayList<>(challengers);


challengers 자체로 새로운 리스트가 만들어져서 new ArrayList<>로 다시 감쌀 필요가 없지 않을까요? 🤔

ㅎㅎㅎ 제 리뷰어님이랑 똑같은 지적.. 역시 날카로운 다즐.. 감삼당
Collectors.toList()는 이미 안에서 자체적으로 방어적 복사를 해준다고 하네요!!

public static <T> Collector<T, ?, List<T>> toList() { return new CollectorImpl<>((Supplier<List<T>>) ArrayList::new, List::add, (left, right) -> { left.addAll(right); return left; }, CH_ID); }

woo-chang · 2023-03-12T12:18:01Z

src/main/java/blackjack/exception/CustomException.java

@@ -0,0 +1,10 @@
+package blackjack.exception;
+
+public class CustomException extends RuntimeException {


커스텀 예외를 잘 활용하시는 것 같아요 👍

shin-mallang

good

shin-mallang · 2023-03-13T04:45:53Z

src/main/java/blackjack/controller/BlackJackController.java

+
+    private void bet(Players players) {
+        List<Challenger> challengers = players.getChallengers();
+        List<Money> betAmounts = new ArrayList<>();


List 말고 Map으로 관리하는건 어떻게 생각하세요?

shin-mallang · 2023-03-13T04:47:29Z

src/main/java/blackjack/controller/BlackJackController.java

+
+    private PlayerStatusDto makeDealerStatus() {
+        Player dealer = blackJackGame.getDealer();
+        return makePlayerStatusDto(dealer);


여기에 makePlayerStatusDto가 있는것보다는, PlayerStatusDto.from() 가 더 좋지 않을까요?
컨트롤러에 있으면 메서드가 너무 많아져서 복잡해지는거같아요

shin-mallang · 2023-03-13T04:47:52Z

src/main/java/blackjack/controller/BlackJackController.java

+        List<Challenger> challengers = blackJackGame.getChallengers();
+        return challengers.stream()
+                .map(challenger -> makePlayerStatusDto(challenger))
+                .collect(Collectors.toUnmodifiableList());


굳이 toUnmodifiableList로 하신 이유가 있나요

shin-mallang · 2023-03-13T04:49:00Z

src/main/java/blackjack/controller/BlackJackController.java

+        }
+    }
+
+    private void inputChoice(Player player) {


입력만 받는 줄 알았는데, hit까지 하네요?
예상하지 못했는데 inputAndHit등으로 바꾸는건 어떨까요?

shin-mallang · 2023-03-13T04:49:20Z

src/main/java/blackjack/controller/BlackJackController.java

+    }
+
+    private void takeAllChallengersTurn() {
+        for (Player player : blackJackGame.getChallengers()) {


blackJackGame의 자율성을 침해하는 것 같아요

shin-mallang · 2023-03-13T04:54:46Z

src/main/java/blackjack/domain/game/BlackJackGame.java

+        this.betAmounts = betAmounts;
+    }
+
+    public static BlackJackGame from(Players players, List<Money> betAmounts) {


CardDeck을 생성자에서 받지 않고, 내부에서 생성해주는 이유가 있나요?

from인데 왜 2개?

shin-mallang · 2023-03-13T04:56:57Z

src/main/java/blackjack/domain/game/Money.java

+        }
+    }
+
+    public Money calculateRevenue(Result result) {


Money는 돈에만 관련된 역할을 하는데, 수익이라는 개념이 들어가면, 그냥 Money로는 도메인 모델을 표현하는 것이 불충분하다고 생각하는데, 어떻게 생각하시나요?

shin-mallang · 2023-03-13T04:57:46Z

src/main/java/blackjack/domain/player/Challenger.java

+    }
+
+    private void validateName(String name) {
+        if (Dealer.NAME.equals(name)) {


참가자의 이름이 딜러면 안되나요?
제 아들 이름 신딜러로 지을 생각이었는데,
벌써부터 차별당하네요?

shin-mallang · 2023-03-13T04:59:07Z

src/main/java/blackjack/domain/player/Players.java

+    }
+
+    public static Players from(List<String> names) {
+        validateDuplicatedNames(names);


동명이인은 게임에 참여할수 없나요? 왜죠???

shin-mallang · 2023-03-13T05:00:33Z

src/main/java/blackjack/exception/CustomException.java

@@ -0,0 +1,10 @@
+package blackjack.exception;
+
+public class CustomException extends RuntimeException {


이름에 Custom이 들어갈 필요가 있을까요?
차라리 BlackjackGameException이 더 좋을 것 같아요
너무 의미없는 네이밍 같습니다.

hanueleee and others added 15 commits March 8, 2023 21:13

docs: 2단계 기능목록

7f42057

feat(Player): 블랙잭인지 확인하는 기능 구현

2041f62

feat(Dealer): 도전자의 게임 결과를 판정하는 기능 구현

1c2cdf5

refactor: 패키지 이동

baa7e32

feat(GameResult): 도전자의 최종 승패 결과에 따라 수익을 계산하는 기능 구현

6dcdbc5

feat(GameResult): 딜러의 수익을 계산하는 기능 구현

9a91451

refactor(GameResult): 원시값 int를 포장한 Money 적용

d1ee2d1

feat(InputView): 카드 추가 지급 의사 입력 기능 구현

177f327

feat(OutputView): 최종 수익 출력 기능 구현

c94bf60

feat(OutputView): 최종 수익 출력 기능 구현

13aab78

refactor: unmodifiableList 대신 방어적 복사 사용

37bf000

refactor: 몇몇 for를 stream으로 수정

bae4f70

refactor: 테스트 수정

fc80dfb

Merge branch 'step2' of https://github.com/hanueleee/java-blackjack i…

12f43ec

…nto step2

greeng00se reviewed Mar 11, 2023

View reviewed changes

woo-chang reviewed Mar 12, 2023

View reviewed changes

shin-mallang reviewed Mar 13, 2023

View reviewed changes

hanueleee added 2 commits March 13, 2023 15:00

refactor: TestFixture인 CardFixture 생성 및 적용

a221486

refactor: InvalidXXX 익셉션들을 범용적인 하나의 InvalidArgumentException으로 통합

2a6944d

greeng00se self-requested a review March 13, 2023 11:13

greeng00se approved these changes Mar 13, 2023

View reviewed changes

		public static final Money ZERO = new Money(0);
		private final int value;

		public static final String NAME = "딜러";

		public static final int MAXIMUM_POINT = 16;

		@@ -0,0 +1,10 @@
		package blackjack.exception;

		public class CustomException extends RuntimeException {

오잉의 블랙잭이노라 구경하노라 #12

Are you sure you want to change the base?

오잉의 블랙잭이노라 구경하노라 #12

Conversation

hanueleee commented Mar 11, 2023

greeng00se left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanueleee Mar 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

woo-chang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanueleee Mar 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shin-mallang left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hanueleee Mar 12, 2023 •

edited

Loading

hanueleee Mar 13, 2023 •

edited

Loading