[1단계 - 블랙잭 구현] 현구막(최현구) 미션 제출합니다. #147

Hyeon9mak · 2021-03-04T09:30:23Z

미립 안녕하세요! 현구막이라고 합니다. 데드라인을 지키는게 우선인 것 같아서 우선 기능이 구현되자마자 PR을 진행하고 있습니다. 😃
(복잡한 레거시 코드를 리팩토링 하는것도 의도된 사항이 아닐까 합리화를 하면서) 미립이 주시는 피드백과 함께 열심히 리팩토링 할게요!
찰리와 함께 구현하면서 궁금한 점이 몇 가지 있었어요!

1. 카드덱 셔플을 딜러가 해야할까요?

현실세계에서 카드 게임이 진행될 땐 매 게임이 진행될 때마다 딜러가 직접 카드덱을 셔플하는데, 저희는 미리 셔플된 카드를 딜러가 가지도록 구현했어요.
객체지향이 현실세계를 완전하게 모방할 필요는 없다고 생각해서 이런 결정을 했는데, 혹시 딜러가 셔플하도록 하는게 더 나았는지, 카드덱이 미리 섞일 필요는 없었는지 고민이 돼요!

2. 캐싱은 제네레이터 클래스가 아닌, 자기 자신이 직접?

카드를 캐싱하고 덱에 채워넣는 제네레이터 클래스를 따로 구현했어요. 그런데 오늘 인비가 공유해줬던 이야기 중에 "캐싱은 캐싱되는 객체에 구현해야한다." 가 떠올라서 고민이 생겼어요. 사실 새로운 카드를 만들 때도 Card 로 먼저 접근할 생각을 하기 때문에 Card 쪽에 static 블럭을 이용해 구현하는게 나았나? 하면서 고민중이에요. 미립이 생각하시기엔 어떤지 궁금해요!

3. Enum 클래스가 각각의 출력양식을 가진 것에 대하여.

카드 번호를 표현하는 Number Enum 클래스와 게임 승패를 표현하는 GameResult Enum 클래스의 상수들이 각각 출력 양식을 가지고 있어요.
사실 이전에 Enum 클래스들이 출력 양식을 가지는 것에 대해 코니가 피드백을 주셨었는데, 결론은 Enum 클래스들이 출력 양식을 가져선 안된다 였어요.
이번에 블랙잭을 구현하면서 저랑 찰리는
"OutputView 쪽에서 Enum별로 분기를 나눠서 출력을 하는건 도메인 역할을 침범하는게 아닐까?" 라는 생각과
"분기를 OutputView 에서 나누기엔 Enum 상수 개수가 너무 많지 않나?" 라는 생각 때문에 피드백을 받고도 똑같은 결정을 했어요...
그러지 말았어야 했을까요?

4. 일급 컬렉션에 메세지를 보내라를 잘 못지켰어요.

이번 블랙잭 미션에서 각 플레이어들이 매 차례마다 카드를 받기 위해 "y", "n" 입력을 받는데, 이걸 View-Controller로부터 받아오기 위해서 Player 들을 묶어놓은 Players 일급 컬렉션에 메세지를 보내지 못하고 리스트로 가져왔어요.
'객체에 메세지를 보내라'를 지키려했다면 Players 쪽에 View를 요청하는 코드가 들어갔을 것 같은데, 다른 방법이 있는건지 아니면 View-Controller 의 입력을 받기 위해서는 어쩔 수 없는것인지 궁금해요!

감사합니다!!

Hyeon9mak · 2021-03-07T10:54:53Z

안녕하세요 미립!
BlackjackManager를 가장 중점적으로 리팩토링 해봤어요.
유틸 클래스였던 BlackjackManager를 없애는 것보다, Dto를 중점적으로 만들어내는 클래스가 되면 어떨까 생각하고 구현했어요!

추가 질문이 있습니다!

1. `BlackjackManager` 를 컨트롤러가 필드로 가지는게 나았을까요?

BlackjackController.play() 메서드의 라인을 줄이려다보니 메서드가 나누어졌고,
메서드가 나누어진 모든 메서드가 BlackjackManager를 파라미터로 들고가게 되더라구요.
필드는 최대한 줄이는것이 좋지만, 이정도까지 중복되게 파라미터로 사용되는 경우 필드로 갖는 것이 나은지,
단순히 라인수를 줄이기 위한 사용이라면 그냥 사용해도 되는지 고민이 되네요...

2. `Players.findPlayerByName()` 메서드의 인자도 포장?

Player.getName() 메서드가 출력 편의를 위해서 Name 객체가 아닌 Name.getValue()로 곧바로 String을 반환해요.
이것 때문에 아래와 같은 코드가 만들어지더라구요...

    public void drawCardPlayer(final Player player) {
        this.players
            .findPlayerByName(new Name(player.getName()))
            .addCard(this.cardDeck.draw());
    }

Player.getName() 메서드를 수정해야하는 것인지, 아니면 그냥 이런식으로 사용해도 괜찮은지 확답을 얻고 싶어요! 😅

3. Dto를 어디까지 구현해주어야 하는지...

이번에 처음으로 Dto 개념을 도입해보았어요! 그 과정에서 다소 고민되는 부분이 있었는데,

    public List<CardDto> getCards() {
        return this.cards;
    }

바로 이 메서드에요. 이 메서드가 실제 OutputView에서는

        System.out.printf(HAND_FORMAT, dealer.getName(), dealer.getCards()
            .stream()
            .map(CardDto::getName)
            .collect(joining(DELIMITER))
        );

이런식으로 사용되고 있는데,
"getCards() 메서드에서 CardDto::getName 변환 작업을 미리 해주어도 되는거 아닌가?"
"Dto의 역할을 넘어서는건가?" 하는 고민을 갖고 있어요.
Dto가 어디까지 구현해주어야 적합한것인가요?!

감사합니다!

Hyeon9mak · 2021-03-07T11:10:12Z

[JDK] Stream - 1

내용

stream API로 나열중인 데이터들을 곧바로 조합해주는 .collect(joining()) 메서드에 대해 학습 후 적용함

링크

https://codechacha.com/ko/java8-stream-collect/

[TDD] 점진적 리팩토링 - 3

내용

'우선 돌아가는 프로그램 상태를 유지하고, 조금씩 수정을 진행하라' 는 메세지를 지킴
컴파일 에러를 최대한 회피하면서 점진적 리팩토링을 진행함

링크

https://techcourse.woowahan.com/s/zmAj9jfu/ls/npw8YN4M

[OOP] 캐싱 - 2

내용

중복되어 사용되는 52장의 Card 객체를 캐싱함
최초에는 CardGenerator 객체를 생성해서 캐싱했으나, 인비와의 대화를 통해 Card 클래스에 직접 캐싱하는 것이 더 자연스럽다고 판단하여 Card 클래스 내부에 static 키워드를 통해 인스턴스 캐싱을 진행함

링크

Hyeon9mak@110209b

[OOP] Service layer, Dto - 5

내용

BlackjackManager 객체를 서비스 레이어로 생각해보면서 Dto개념을 도입함

링크

seok-2-o

피드백 반영 빠르게 잘해주셨네요 💯
질문하신 내용들중 조금 더 고민해보면 좋을 부분이 있어서 Request changes 하였습니다.
제가 남긴 피드백과 질문을 다시 한 번 고민해보고 리펙토링 진행해주세요.

수정할 부분이 없다고 생각하시면 그대로 PR 주셔도 무방합니다.

BlackjackManager 를 컨트롤러가 필드로 가지는게 나았을까요?
근본적으로 BlackjackManager 가 할 수 있는 일을 Controller가 대신 하고 있기때문에 파라미터로 계속해서 전달해야하는것 같다는 생각이드네요
BlackjackManager에서 현재 차례의 플레이어 이름 외에 다른 정보를 노출하지 않도록 리펙토링 해보는건 어떤가요? 인스턴스 필드로 가진다면
동시에 한게임 밖에 실행 할 수 없을 것 같아요. Game 과 Controller 의 경계가 모호해지네요.
Players.findPlayerByName() 메서드의 인자도 포장?
편의를 위해 다양한 인자를 받는 find 메소드를 만들 수 있을 것 같아요. String, Name, Player 모두 가능하도록요.
그런데 Player 를 이미 가지고 있는데 해당하는 이름으로 Player 를 찾는게 의미가 있을까요?
Dto를 어디까지 구현해주어야 하는지...
저는 VIEW 에 전달되기전에 DTO 로 변환되어야 한다고 생각해요
VIEW 와 컨트롤러가 통신할떄 무조건 DTO를 통해서만 통신한다고 생각하면 어떨까요?
DTO 를 왜 사용하려고하시는지 조금 더 고민해보면 좋겠네요 관련한 코멘트도 하나 남겨놓았으니 같이 확인해보면 좋을 것 같아요.

seok-2-o · 2021-03-08T02:35:54Z

src/main/java/blackjack/controller/BlackjackController.java

+
+    private void initBlackjack(final BlackjackManager blackjackManager) {
+        blackjackManager.initDrawCards();
+        ParticipantDto dealerDto = blackjackManager.createDealerDto();


개인적으로 도메인은 DTO 를 모르는 상태가 좋다고 생각해요
Dto.of(dealer) 서비스에 따라 DTO 의 형태는 여러가지로 변할 수있지만 도메인은 그렇지 않거든요

예를들어 게시글의 조회수는 관리자만 볼 수 있다면 관리자를 위한 DTO 는 조회수가 포함되고
그렇지 않은 DTO 는 조회수가 포함되지 않겠죠. 그럴때마다 도메인에 DTO 를 만드는 로직을 추가하는것은 유지보수하기
어려운 코드를 만든다고 생각해요.

감사합니다! 말씀해주신대로 다른 DTO생성 객체를 구현했습니다!

seok-2-o · 2021-03-08T02:37:34Z

src/main/java/blackjack/controller/BlackjackController.java

+        }
+    }
+
+    private void hitCardPlayer(final Player player, final BlackjackManager blackjackManager) {


플레이어를 외부로 꺼내지 말고
BlackjackManager 가 현재 플레이어를 인식하고 있는것은 어떤가요 ?

시도해봣습니다!!

delete state methods, add judge method in Player delete judge method in Dealer

Hyeon9mak · 2021-03-10T16:35:19Z

안녕하세요 미립!! 오래걸렸습니다!!

DM 드렸던대로 유한 상태 머신을 중점적으로 리팩토링 해봤어요!
이번에도 구현하면서 궁금했던 부분, 고민되었던 부분들이 있었습니다!

1. 테스트를 위한 CardDeck 생성 팩터리 메서드를 만들어버렸어요..

리팩토링 과정에서 테스트코드부터 작성했다면 문제될 일이 없었겠지만,
마음만 급해서 TDD 제대로 적용하지 못했고, 그 때문에 구현 후 유닛테스트를 작성했어요.
그러던 중 랜덤으로 셔플되어 제공되는 카드 덱이 문제가 되었고, 테스트를 위한 팩터리 메서드를 만들어버렸어요...
차라리 팩터리 메서드를 없애고 생성자를 조금 더 유연하게 만드는 것이 더 나을까요?

2. 유한 상태 머신에서 승패 비교를 플레이어가 하는게 더 유리한거같아요....???!?

딜러의 블랙잭 상태 기준 (2)
- 플레이어 블랙잭 -> 플레이어 무승부
- 플레이어 스테이/버스트 -> 플레이어 패배
딜러의 스테이 상태 기준 (3)
- 플레이어 블랙잭 -> 플레이어 승리
- 플레이어 스테이 -> 점수크기 비교 진행
- 플레이어 버스트 -> 플레이어 패배
딜러의 버스트 상태 기준 (2)
- 플레이어 블랙잭/스테이 -> 플레이어 승리
- 플레이어 버스트 -> 플레이어 패배
플레이어의 블랙잭 상태 기준 (2)
- 딜러 블랙잭 -> 플레이어 무승부
- 딜러 스테이/버스트 -> 플레이어 승리
플레이어의 스테이 상태 기준 (3)
- 딜러 블랙잭 -> 플레이어 패배
- 딜러 스테이 -> 점수크기 비교 진행
- 딜러 버스트 -> 플레이어 승리
플레이어의 버스트 상태 기준 (1)
- 딜러 블랙잭/스테이/버스트 -> 플레이어 패배

합계 7대6으로 플레이어의 상태 쪽에서 딜러의 상태를 비교하는 것이 경우의 수가 더 적어서 플레이어쪽에 구현했어요!
괜찮은 생각일까요?!

3. 유한 상태 머신에서 승패 비교를 딜러도 할 수 있어요...

승패 비교 로직을 2번처럼 구현했을 때 "딜러 입장에서도 저 코드를 사용할 수 있는거 아니에요? 그럼 결과가 잘못 나오잖아요." 라고 라이언이 의문을 제기했어요.
생각해보니 정말 그런 것 같더라구요. 그래서 라이언, 손너잘과 함께 토의해보았을 때 나온 의견으로

Finished 에 해당하는 추상 클래스를 딜러용, 플레이어용으로 나누어서 승패계산 메서드를 사용하지 못하게 만든다.
isInstanceOf(Dealer.class)를 통해 딜러임을 잡아낸다.

정도가 있었어요. 하지만 두 가지 방법 모두 오버엔지니어링이 아닌가? 하는 고민도 함께 하게 됐어요.
딜러가 사용할 수도 있게 열어두는게 상관이 없을까요?

4. 컨트롤러의 모든 메서드가 매니저를 인자로 가져가는 건 똑같아요...

컨트롤러 쪽에 Players, Player, Dealer 가 빠져나가지 않게 해봤어요! 그런데 여전히 컨트롤러의 모든 메서드들이 블랙잭 매니저를 인자로 가져가네요 😭 😭 😭
메서드를 나눈 이유는 사실 메서드별 라인, 인덴트 제한을 지키기 위한 경우가 대부분인데,
"매니저에게 추상적으로 메세지를 보낼수도 있는데, 너무 하나하나 일을 처리하도록 시키고 있나?" 하는 고민도 생겨요.
그래서 조금 더 추상적으로 메세지를 보내볼까도 했는데, Input, Output을 계속 주고 받아야해서 어려운 것 같아요.
더 좋은 방법이 있을까요?!

seok-2-o

많은 고민을 하셨군요 💯
그만큼 많은 것을 얻으셨다고 생각합니다.
이번 미션은 여기까지 하고 이어서 다음 미션 진행해주시면 될 것 같아요.

테스트를 위한 CardDeck 생성 팩터리 메서드를 만들어버렸어요..
현재 구조가 크게 나쁘다고 생각이 들진 읺지만, 아쉬움이 남는다면 리펙토링 진행해보는것도 좋을 것 같아요.
유연한 설계로 변경하는 방법은 다양하니까 고민해보면 좋을 것 같네요.
유한 상태 머신에서 승패 비교를 플레이어가 하는게 더 유리한거같아요....???!?
경우의 수로 위치를 정하는 접근법 좋네요 💯
나중에 확률이나 빈도수로 접근해봐도 좋겠네요 👍
유한 상태 머신에서 승패 비교를 딜러도 할 수 있어요...
현재 규모에서는 상태를 외부로 노출하지 않고 플레이어만 비교하는 인터페이스를 구현하고 있으니 큰 문제는 없어보여요.
컨트롤러의 모든 메서드가 매니저를 인자로 가져가는 건 똑같아요...
출력을 대화식으로 해야한다는 생각을 버리면 조금 더 간소화 가능할 것 같아요. 해당하는부분에 코멘트로 설명해놓았어요 확인 해주세요.

seok-2-o · 2021-03-11T00:10:02Z

src/main/java/blackjack/controller/BlackjackController.java

+        OutputView.printNewLine();
+    }
+
+    private void hitOrStayDealer(BlackjackManager blackjackManager) {


해당 로직이 blackjackManager 밖에 나와 있을 필요가 있나요 ?
딜러가 모든 패를 받고 출력을 해도 될 것 같아요
현재

[3 ,7] [3 ,7, 4] HIT 출력 [3, 7, 4, J] BUST 출력

변경

[3 ,7] [3 ,7, 4] HIT [3, 7, 4, J] BUST 출력 [3 ,7] [3 ,7, 4] [3, 7, 4, J] ex ) for (int i = 1; i < cards.size(); i++) { for (int j = 0; int j = i; j++) { System.out.print(cards.get(j)); } }

아... 출력 예시만 따라서 구현을 했는데, 더 유연하게 생각해볼 수 있었네요. 😮😮😮

Hyeon9mak added 30 commits March 2, 2021 17:18

feat : 차량에 따른 연료주입 추상클래스로 구현

7253882

feat : 차량에 따른 연료주입 인터페이스로 구현

f080ffc

docs: 기능 구현 목록 작성

8e9d573

docs : 기능 목록 추가

048c307

docs : 테스트 구상 목록 추가

598aa92

chore: 연료주입 미션 패키지 통합

ad90477

feat : 숫자와 문양을 가지는 블랙잭 카드 구현

6d6d18a

refactor : 카드의 필드명 중복 제거

7761340

feat : 무늬별 13장씩 총 52장의 카드를 가지는 카드덱 구현

6c9311a

refactor : 카드 덱 캐싱 제너레이터 클래스 추가

be25491

feat : 손패를 가지는 일급 컬렉션 Hand 추가

1cc6048

feat : 카드를 주거나 받는 딜러 추가

336fa85

feat : 카드는 받는 플레이어 구현

14f8a2b

feat : 손패를 반환하는 메서드 추가

ec5ea9b

feat : 플레이어가 이름을 가질 수 있게 구현

54b595c

feat : 플레이어 일급 컬렉션 구현

6d5dd92

feat : 플레이어의 이름을 받는 입력, 출력, 초기셋팅 추가

3ff5f52

docs : 리팩토링 목록 추가

410f23d

refactor : 포장된 원시값을 반환하는 메서드 생성

9d11355

refactor : 딜러와 플레이어를 매니저가 상태로 가지지 않도록 삭제

730f1c8

feat : 플레이어와 딜러의 카드 히트 여부 입출력 구현

401d5cb

feat : 딜러가 초기 카드 2장을 받는 로직 추가

109882a

feat : 모든 히트가 완료된 후 최종 손패와 점수를 보여주는 출력 추가

5bc9fd7

docs : 질문 리스트 추가

749ea1c

feat : 승리 패배 무승부 3가지를 요소로 가지는 enum 생성

8dc738c

feat : 플레이어와 딜러의 승패 그리고 무승부 결과를 반환하는 로직 구현

dfa2844

feat : 플레이어가 버스트되었을 시 히트할 수 없는 로직 구현

f6f3334

refactor : 입출력 개행 수정

83cd6f6

feat : 최대 참여 플레이어 예외처리 추가

f9e301c

refactor : 딜러와 플레이어의 중복되는 부분을 추상 메서드로 분리

7729e99

Hyeon9mak added 6 commits March 7, 2021 15:58

refactor: move the burst check logic from Player to Hand

d466c3a

refactor: add final keyword, add this keyword, fix class name

6855220

refactor: add create Dto method in BlackjackManager

db5bd0c

refactor: add burst check method in Player

3845fb1

feat: add find player by name method

15da9a6

docs: update refactor list in README.md

fd465b6

seok-2-o suggested changes Mar 8, 2021

View reviewed changes

Hyeon9mak added 17 commits March 8, 2021 17:08

docs: add refactoring list in README.md

cd17b80

refactor: wrap player and dealer score to Score class

07ea032

feat: add init draw object, add Hit state

3d8c0ef

feat: add Bust state

8110b0c

feat: add Stay state

a7bb0b9

feat: add Blackjack state

b2bd489

refactor: abstract Blackjack, Bust, Stay state to Finished

55b789d

feat: add return cards method in Finished and Hit

8cd8b02

refactor: abstract Hit, Running, Finished, AfterInitDraw

ddc902f

refactor: state interface, abstract, class refactoring

b0af6a5

feat: add compare score logic

1a7ec15

refactor: move hand class into state package

742b87d

feat: add create custom deck method for test code

813ddad

refactor: refactoring for view classes

55b0fac

refactor: move resposibility in Participant objects

08fb3b1

delete state methods, add judge method in Player delete judge method in Dealer

refactor: move resposibility from Controller to Manager class

dc56f69

style: add enter lines, add blank

c5c958a

seok-2-o approved these changes Mar 11, 2021

View reviewed changes

seok-2-o merged commit 6704007 into woowacourse:hyeon9mak Mar 11, 2021

utterances-bot mentioned this pull request Apr 9, 2021

%EC%9A%B0%EC%95%84%ED%95%9C%ED%85%8C%ED%81%AC%EC%BD%94%EC%8A%A4-%EB%B8%94%EB%9E%99%EC%9E%AD-%EB%AF%B8%EC%85%98-%ED%9A%8C%EA%B3%A0/ Hyeon9mak/Hyeon9mak.github.io#43

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 블랙잭 구현] 현구막(최현구) 미션 제출합니다. #147

[1단계 - 블랙잭 구현] 현구막(최현구) 미션 제출합니다. #147

Hyeon9mak commented Mar 4, 2021

Hyeon9mak commented Mar 7, 2021

Hyeon9mak commented Mar 7, 2021

seok-2-o left a comment

seok-2-o Mar 8, 2021

Hyeon9mak Mar 10, 2021

seok-2-o Mar 8, 2021

Hyeon9mak Mar 10, 2021

Hyeon9mak commented Mar 10, 2021

seok-2-o left a comment

seok-2-o Mar 11, 2021

Hyeon9mak Mar 11, 2021

[1단계 - 블랙잭 구현] 현구막(최현구) 미션 제출합니다. #147

[1단계 - 블랙잭 구현] 현구막(최현구) 미션 제출합니다. #147

Conversation

Hyeon9mak commented Mar 4, 2021

1. 카드덱 셔플을 딜러가 해야할까요?

2. 캐싱은 제네레이터 클래스가 아닌, 자기 자신이 직접?

3. Enum 클래스가 각각의 출력양식을 가진 것에 대하여.

4. 일급 컬렉션에 메세지를 보내라를 잘 못지켰어요.

Hyeon9mak commented Mar 7, 2021

1. BlackjackManager 를 컨트롤러가 필드로 가지는게 나았을까요?

2. Players.findPlayerByName() 메서드의 인자도 포장?

3. Dto를 어디까지 구현해주어야 하는지...

Hyeon9mak commented Mar 7, 2021

[JDK] Stream - 1

내용

링크

[TDD] 점진적 리팩토링 - 3

내용

링크

[OOP] 캐싱 - 2

내용

링크

[OOP] Service layer, Dto - 5

내용

링크

seok-2-o left a comment

Choose a reason for hiding this comment

seok-2-o Mar 8, 2021

Choose a reason for hiding this comment

Hyeon9mak Mar 10, 2021

Choose a reason for hiding this comment

seok-2-o Mar 8, 2021

Choose a reason for hiding this comment

Hyeon9mak Mar 10, 2021

Choose a reason for hiding this comment

Hyeon9mak commented Mar 10, 2021

1. 테스트를 위한 CardDeck 생성 팩터리 메서드를 만들어버렸어요..

2. 유한 상태 머신에서 승패 비교를 플레이어가 하는게 더 유리한거같아요....???!?

3. 유한 상태 머신에서 승패 비교를 딜러도 할 수 있어요...

4. 컨트롤러의 모든 메서드가 매니저를 인자로 가져가는 건 똑같아요...

seok-2-o left a comment

Choose a reason for hiding this comment

seok-2-o Mar 11, 2021

Choose a reason for hiding this comment

Hyeon9mak Mar 11, 2021

Choose a reason for hiding this comment

1. `BlackjackManager` 를 컨트롤러가 필드로 가지는게 나았을까요?

2. `Players.findPlayerByName()` 메서드의 인자도 포장?