[2단계 - 블랙잭 배팅] 레디(최동근) 미션 제출합니다. #725

reddevilmidzy · 2024-03-15T08:22:44Z

안녕하세요 수달!😁

step2도 잘부탁드립니다😄

아임 레디ㅣㅣ

고민했던 점

배팅 금액 입력

2단계에서는 배팅 금액이라는 새로운 친구가 생겼습니다.
현재 프로그램 흐름은 참가자 이름을 전부 입력 받고, 그 다음에 차례대로 배팅 금액을 입력받는 형태입니다.
요녀석을 어떻게 관리해야할지 고민하다가 이름과 배팅금액 입력되는 시점이 다르니 한번에 Player를 생성하기는 어렵겠구나, 배팅 금액을 따로 Map에 저장해서 repository 형태로 구현해보자! 하고 구현을 시작하였습니다.

이런 형태가 DB를 다루는 다음 체스 미션의 떡밥(?)인줄 알고 신나게 구현을 했지만, 구현하고 나니 뭔가 설명할 수 없는 찝찝함이 있었습니다.
그래서 싹 다 뒤집고 배팅 금액을 참여자의 필드로 가지고 있는 형태로 설계를 변경하였습니다.

리포지토리 형태로 구현했던 이유

현재 구조를 유지한 채 변경하고 싶었다.
이름과 배팅금액으로 한번에 참여자 객체를 생성할 수 없다.(고 생각했었다)
- 입력되는 시점이 달라 처음에 같이 생성할 수 없고, setter를 추가하거나 BetAmount 필드에 final을 제거해야 했다.

BetAmount를 필드로 가지고 있는 형태로 구현한 이유

Names라는 객체를 만들면서 Players의 이름을 임시로 가지고 있을 Names라는 객체를 만들면서 이름과 배팅금액으로 동시에 생성할 수 없었던 문제를 해결할 수 있었다.
만약 제일 처음부터 배팅 금액 요구 사항이 있었다면 Player가 배팅 금액을 가지고 있는 형태로 구현했을 거 같았다.

대박 커진 컨트롤러

구현을 하고 나니 컨트롤러가 꽤 커졌습니다. 메서드 추출 단축키(사랑해요 ctrl + alt + m)로 열심히 메서드를 분리해보았는데, 메서드의 파라미터는 모두 Dealer와 Players를 가진 형태였습니다.

private void initHands(Dealer dealer, Players players) {}

private void dealSomething(Dealer dealer, Players players) {}
...

이렇게 사용할 바에 Dealer와 Players를 필드로 가지고 있는 객체를 만들어보자! 하여 컨트롤러의 책임을 Game이라는 객체를 만들어 넘겨주었습니다.

그랬더니 컨트롤러는 조금 깔끔해진 느낌이 있었지만 Game의 파라미터들이 커졌습니다.

여기서 질문!

여기서 질문이 있습니다!

Game 객체에 view를 직접 넘겨주는 것은 이상하다고 생각하여 배운 함수형 프로그래밍을 활용해 조금 특이한 방법을 사용해보았는데, 현재 구조 괜찮을까요??? (Game에게 view를 간접적으로 넘겨주는 형태)

컨트롤러는 좀 가벼워진 느낌이 있지만 Game 객체가 깔쌈하진 않습니다..
두 형태 모두 뭔가 마음에 안드는데 새로운 방법이 있을지 궁금합니다..!

또다른 질문!!

Player의 필드가 변경되면서 이전 테스트들이 걸리적 거리더라구요. 이전 테스트들은 수정하지 않고, 생성자의 접근제한자와 필드 초기화를 해주는 방향으로 진행을 하였는데, 이런식으로 해도 괜찮을까요??

구조 변경 시 기존에 있던 테스트의 변경이 불가피한 상황이 많은데, 이런 상황이 발생하지 않게 초반에 설계를 잘 가져가야 할지, 아니면 테스트를 유연하게 잘 짜야하는건지 현업에선 요런 상황들을 어떻게 하는지 궁금합니다!

감사합니다🙇‍♂️

her0807

안녕하세요 레디 😉

수달입니다.

도전적으로 구현하고, 리팩터링해주신 점 아주 칭찬하고 시작하고 싶네요 ㅎㅎ
코드 작성하는 과정에 대해서도 꼼꼼하게 알려주셔서 코드리뷰하는데 큰 도움이 되었어요.

테스트도 엄청 꼼꼼하게 작성하셨던데 TDD 로 하셨나요??
만일 TDD 로 작성한 로직이라면, 엄청난 성장인 것 같습니다!!!

요구사항을 모두 충족한 것 같아서 머지하려고 했으나.
레디와 대화가 재밌어서 한번더 Request changes 남겨요 (ㅋㅋ)

her0807 · 2024-03-16T09:05:53Z

docs/README.md

@@ -2,15 +2,16 @@

 ## 참여자

+* [x] 다시한번 룰 숙지 필요


다시한번 룰 숙지 필요 ㅋㅋㅋㅋ

이건 지워주셔도 될것 같습니다 뤠디 😋

her0807 · 2024-03-16T09:07:00Z

docs/README.md

+  * [x] 10으로 나누어 떨어지지 않으면 예외가 발생한다.
+    * 블랙잭시 1.5배를 해주기 위함
+  * [x] 배팅 금액은 최대 100_000_000로 이를 초과시 예외가 발생한다.
+    * 건전한 플레이를 하기 위함


배팅 금액에 대한 요구사항을 더 많이 적어두셨군요 👍🏻 잘하셨습니다

her0807 · 2024-03-16T09:07:25Z

docs/README.md

+<br>
+
+* [x] 딜러의 최종 수익을 계산한다.
+  * [x] 수익을 출력시 세자리마다 쉼표를 찍는다. ex) 100,000


친절한 출력도 굿 👍🏻

히히 이번 미션을 구현할 때 클린코드와 더불어 사용자 친화적!인 애플리케이션을 만들고 싶었습니다 ㅎ.ㅎ

her0807 · 2024-03-16T09:11:49Z

docs/README.md

+* [x] 딜러의 최종 수익을 계산한다.
+  * [x] 수익을 출력시 세자리마다 쉼표를 찍는다. ex) 100,000
+* [x] 참가자의 최종 수익을 계산한다.
+* [x] 참가자만 블랙잭인 경우 배팅 금액의 1.5배를 받는다.


처음 2장이 블랙잭이면 1.5배 수익이고, 참가자 카드가 2개를 초과해서 블랙잭이 되면 1.5배를 해주면 안될 것 같아요!

요것도 처음에 헷갈렸던 건데 처음 받은 카드의 합이 21이어야만 블랙잭이라고 하고 이후 카드를 추가한 다음 21이 되면 이때는 블랙잭이라고 부르지는 않는 거 같더라구요! 그래서 참가자가 카드 2를 초과해서 21이 된 경우는 아래에 참가자가 승리한다면 배팅한 금액을 받는다에 포함될 수 있을 거 같아요!

her0807 · 2024-03-16T09:20:11Z

src/main/java/domain/participant/Player.java


 public class Player extends Participant {

-    public Player(final Name name, final Hands hands) {
+    private final BetAmount betAmount;


배팅 금액을 따로 Map에 저장해서 repository 형태로 구현해보자! 하고 구현을 시작하였습니다. 이런 형태가 DB를 다루는 다음 체스 미션의 떡밥(?)인줄 알고 신나게 구현을 했지만, 구현하고 나니 뭔가 설명할 수 없는 찝찝함이 있었습니다.

Player 에 조합형태로 BetAmount를 가지고 있는 것이 좋은 것 같다에 힘을 좀더 실어보자면! 레포지토리 형태로 구현한 것을 보니 BlackJackController 에서 배팅 금액이 관리되고 있었는데요~ 각 플레이어가 자기가 가진 돈을 모른다는게 현실 세계와 맞지 않는 것 같아요. 레포지토리에 " 레디 얼마 배팅했어? " 이렇게 물어봐야하는 것도 번거롭고요! ㅎㅎ 그래서 지금 형태로 리팩하신게 잘하신 것 같아요.

그러나 레포지토리로 구현해보신 후 이상한 점을 느끼고 개선하신 것은 굉장히 칭찬받을만한 일이라고 생각해요.
자신의 아이디어를 직접 구현해보는 연습도 충분히 되고, 이게 DB 의 역할을 하는 것 같다고도 유추해보신 것이 요구사항을 볼 때
나무를 보는게 아닌, 숲을 보는 것 같아서요~ 👍🏻

her0807 · 2024-03-16T09:23:38Z

src/main/java/domain/participant/Player.java

        super(name, hands);
        validate(name);
+        this.betAmount = new BetAmount(10);


이녀석은 배팅금액을 왜 10원으로 초기화 시켜줄까요? 🤔

최소 배팅 금액이 정해져 있다면 위와 같이 강제하는 것이 좋은데요~
플레이어를 생성할 때 배팅 금액이 없는 상태로 생성해야해서 임시 값을 넣은 것이라면,
다른 방법을 생각해보면 좋을 것 같아요.

요거 운영코드에는 안쓰이고 있는 것 같은데 안쓰면 지워도 될 것 같습니다~~!

Q. Player의 필드가 변경되면서 이전 테스트들이 걸리적 거리더라구요. 이전 테스트들은 수정하지 않고, 생성자의 접근제한자와 필드 초기화를 해주는 방향으로 진행을 하였는데, 이런식으로 해도 괜찮을까요??

리뷰를 하다보니 발견했어요 ㅋㅋㅋㅋㅋ ㅜㅜ 음 저는 테스트 코드를 위한 생성자는 있으면 안된다고 생각해요. 😱 운영에서 잘못 쓰일 여지도 있고요! 무엇보다 테스트 코드는 살아있어야하기 때문에 운영 코드가 변경되면 테스트도 변경되어야하는게 자연스러운 일이라고 느껴져요. 귀찮기는 하지만요 ㅎㅎ

그래서 이런 구조의 변화에는 어쩔 수 없이 테스트를 변경해야하지만, 간단한 메서드 명이나 필드명을 수정하거나, 함수 로직을 리팩터링 했는데, 결과가 같을 경우 등등은 테스트에 수정이 없어야한다고 생각해요.

그런 테스트를 블랙박스 테스트라고하는데요~ 테스트 함수명을 운영 메서드명와 똑같지 하지 않고,테스트하고자 하는 것으로 네이밍을 짓고, 과정을 테스트 하는게 아니라, 결과를 테스트 하는 형태 등등

이참에 유지보수하기 좋은 테스트란 무엇인가? 에 대한 고민도 레디가 해보면 좋겠네요 😊

후후 맘잡고 고쳐보겠습니다💪

테스트 수정하면서 유지보수하기 좋은 테스트가 무엇일지 고민을 해보았는데,, 유지보수하기 좋은 설계와 비슷하게 테스트도 추상화를 하여 해야하는 것인가? 했지만 아직 "테스트 추상화 그거 어떻게 하는 건데" 상태네요 🙃

머리가 나쁘면 몸이 고생한다고 하던가요, 아직까진 귀찮지만 구조 바뀔 때마다 열심히 테스트를 수정하다가 짬좀 차고 책 좀 읽으면 저만의 답을 찾을 수 있을 거 같네요 😃

her0807 · 2024-03-16T09:34:58Z

src/main/java/domain/Game.java

+        ));
+    }
+
+    public void deal(final Player player, Function<String, Answer> answerReader,


Q. 대박 커진 컨트롤러, Game 으로 책임 이관. but 이런 구조가 조금은 특이해보여요. 괜찮을까요?

이 부분 안그래도 피드백 드리려고 했는데 함수 자체를 시그니처로 보내는 것이 낯설 것 같아서 걱정했었는데
잘해주셨군요.

저희가 자주쓰는 Stream 녀석들내부 동작 구조를 살펴보면 메서드 시그니처에 함수들을 담고 있어요.
전혀 이상하지 않고, 잘하셨습니다.

조금 더 첨언하자면, controller 는 언제든지 바뀔 수 있으니 게임에 대한 실행 로직을 최대한 Game 안에 담는 것이 좋아보여요, 그런 의미에서 잘 리팩터링하신 것 같습니다.🙂

her0807 · 2024-03-16T09:44:13Z

src/test/java/domain/GameResultTest.java

+        Assertions.assertThat(sum21Size3.calculateResult(sum20Size2)).isEqualTo(GameResult.WIN);
+        Assertions.assertThat(sum20Size2.calculateResult(sum21Size3)).isEqualTo(GameResult.LOSE);


이렇게 각각의 검증문이 있을 경우는 앞에 검증문이 실패할 경우 뒤에 검증문은 실행하지 않게 되는데요~
assertions.assertall을 사용해서 묶어주면 무조건 모든 검증문이 실행되고, 어떤게 실패했는지 알려주어 편하답니다!

Assertion 문이 두개 밖에 없어 굳이 필요 없을 거라고 생각했습니다!
(핑계고 사실 귀찮아서 안한거 같네요,, 🤫)

her0807 · 2024-03-16T09:45:12Z

src/test/java/domain/participant/DealerTest.java

+        final Player blackJackPlayer = new Player(new Name("수달"), blackJack);
+        final Player noBlackJackPlayer = new Player(new Name("레디"), sum18Size2);


블랙잭 시켜줘서 고마워요 🤩

들숨에 건강을 날숨에 재력을 🤗

reddevilmidzy

안녕하세요 수달ㄹㄹ

테스트는 기본적으로 TDD로 하려고 했고 가끔 까먹으면 하이브리드(?)로 진행한 거 같네요😊

저도 수달과의 리뷰 재미있었습니다ㅏ😁
감사합니당

reddevilmidzy · 2024-03-16T10:07:48Z

src/test/java/domain/participant/DealerTest.java

+        final Player blackJackPlayer = new Player(new Name("수달"), blackJack);
+        final Player noBlackJackPlayer = new Player(new Name("레디"), sum18Size2);


들숨에 건강을 날숨에 재력을 🤗

reddevilmidzy · 2024-03-16T10:18:11Z

docs/README.md

+<br>
+
+* [x] 딜러의 최종 수익을 계산한다.
+  * [x] 수익을 출력시 세자리마다 쉼표를 찍는다. ex) 100,000


히히 이번 미션을 구현할 때 클린코드와 더불어 사용자 친화적!인 애플리케이션을 만들고 싶었습니다 ㅎ.ㅎ

reddevilmidzy · 2024-03-16T10:21:36Z

src/main/java/domain/participant/Player.java

        super(name, hands);
        validate(name);
+        this.betAmount = new BetAmount(10);


후후 맘잡고 고쳐보겠습니다💪

reddevilmidzy · 2024-03-16T10:39:15Z

src/test/java/domain/GameResultTest.java

+        Assertions.assertThat(sum21Size3.calculateResult(sum20Size2)).isEqualTo(GameResult.WIN);
+        Assertions.assertThat(sum20Size2.calculateResult(sum21Size3)).isEqualTo(GameResult.LOSE);


Assertion 문이 두개 밖에 없어 굳이 필요 없을 거라고 생각했습니다!
(핑계고 사실 귀찮아서 안한거 같네요,, 🤫)

reddevilmidzy · 2024-03-17T09:51:47Z

docs/README.md

+* [x] 딜러의 최종 수익을 계산한다.
+  * [x] 수익을 출력시 세자리마다 쉼표를 찍는다. ex) 100,000
+* [x] 참가자의 최종 수익을 계산한다.
+* [x] 참가자만 블랙잭인 경우 배팅 금액의 1.5배를 받는다.


요것도 처음에 헷갈렸던 건데 처음 받은 카드의 합이 21이어야만 블랙잭이라고 하고 이후 카드를 추가한 다음 21이 되면 이때는 블랙잭이라고 부르지는 않는 거 같더라구요! 그래서 참가자가 카드 2를 초과해서 21이 된 경우는 아래에 참가자가 승리한다면 배팅한 금액을 받는다에 포함될 수 있을 거 같아요!

reddevilmidzy · 2024-03-17T12:22:32Z

src/main/java/domain/participant/Player.java

        super(name, hands);
        validate(name);
+        this.betAmount = new BetAmount(10);


테스트 수정하면서 유지보수하기 좋은 테스트가 무엇일지 고민을 해보았는데,, 유지보수하기 좋은 설계와 비슷하게 테스트도 추상화를 하여 해야하는 것인가? 했지만 아직 "테스트 추상화 그거 어떻게 하는 건데" 상태네요 🙃

머리가 나쁘면 몸이 고생한다고 하던가요, 아직까진 귀찮지만 구조 바뀔 때마다 열심히 테스트를 수정하다가 짬좀 차고 책 좀 읽으면 저만의 답을 찾을 수 있을 거 같네요 😃

her0807

안녕하세요 레디 ㅎㅎ

이제 요구사항을 모두 충족한 것 같아 머지하겠습니다.

고생 많으셨어요!!!

reddevilmidzy added 30 commits March 13, 2024 11:12

docs: 구현 목록 작성

c7a83f1

feat: 배팅 금액이 양의 정수가 아닐 시 예외 발생 기능 구현

2206919

test: player의 canDeal 테스트 추가

dec4970

refactor: testFixture 상수 변경

ffbc5e3

feat: 카드의 갯수가 2장이고 카드의 합이 21인 경우 블랙잭이다.

ee8b66e

fix: 참여자의 이름의 공백이 포함된 경우 제거하여 중복 판단 기능

f37f288

feat: 참가자가 승패에 따라 배팅한 금액 잃거나 받는 기능 구현

285f322

feat: 각 참여자의 배팅 금액을 입력 받는 기능 구현

197f942

feat: 도메인 toString 재정의

93e5b86

feat: 참가자만 블랙잭인 경우 배팅 금액의 1.5배를 받는 기능 구현

8aba953

feat: 참가자와 딜러 모두 블랙잭인 경우 배팅한 금액 돌려 받는 기능 구현

b8b1ef9

feat: 딜러의 최종 수익 계산 기능 구현

d51ecdf

feat: 딜러와 참가자의 최종 수익 출력 기능 구현

4a03db2

fix: test 코드 오류 수정

874cd41

refactor: 딜러가 블랙잭인 경우 출력 메서드 view 로 이동

52a9b0f

fix: 무승부 조건 변경

f5c0b4d

refactor: 배팅 금액을 최종 결과 금액을 변경하는 로직 함수형 프로그래밍 이용

87d8f09

feat: 10으로 나누어 떨어지지 않으면 예외 발생 기능 구현

2988947

refactor: Participant equals & hashCode 에서 Hands 제거

24e020d

style: 코드 리포매팅

2fb529f

refactor: Player의 필드에 BetAmount 포함하는 형태로 구현

09ea27a

refactor: repository 삭제 및 테스트 코드 위치 변경

02f44e0

refactor: Player equals && hashCode 재정의

6b5c8e6

style: 테스트 코드 디스플레이네임 수정

e166122

feat: 배팅 금액이 1억을 초과할 시 예외 발생 기능 구현

e580c16

refactor: controller 변수 명 수정 및 메서드명 수정

930c385

refactor: Result 이름 GameResult로 변경

c401e21

refactor: Answer, Amount, BetAmount package 변경

e5cc892

refactor: 테스트 변수에 final 추가

f22ec42

refactor: Player 의 테스트에서 사용하는 생성자 접근제한자 디폴트로 변경

353e923

reddevilmidzy added 9 commits March 15, 2024 13:34

refactor: betAmount 필드 이름 변경

2fb5001

refactor: 카드의 합이 21이 된 경우 출력 메시지 수정

28f5b8d

refactor: 정적 팩터리 메서드 접근제한자 protected 에서 디폴트로 변경

b9f19af

feat: 수익 출력시 세자리 마다 쉼표를 붙여 출력 기능 구현

c48d744

refactor: players 등록 메서드로 분리

8b9c0ca

refactor: 컨트롤러의 책임을 Game 객체를 만들어서 분리

6ca77d6

style: 주석 제거

4721bfa

refactor: 불필요한 테스트 삭제

7f672e4

docs: 구현 목록 수정

5aa83c3

her0807 requested changes Mar 16, 2024

View reviewed changes

reddevilmidzy added 9 commits March 16, 2024 20:16

refactor: 테스트 메서드 하나의 여러 검증 문 assertAll 로 묶음

d8e32b1

refactor: assertJ 테스트 메서드 static import 로 변경

2b28eb5

docs: 구현 기능 목록 수정

c5d95bb

refactor: amount 이름 earnAmount로 변경

c10b0fe

refactor: player의 betAmount를 가지지 않았던 이전 테스트 코드 리팩터링

293e90d

fix: 참여자 이름에 딜러가 포함됐을때 예외 발생 시점 수정

e0902a3

refactor: 테스트 코드 static import 로 변경

9443310

chore: 테스트 클래스 패키지 이동

df85832

refactor: 이름의 공백제거 null 검증 후로 변경

0630034

reddevilmidzy commented Mar 17, 2024

View reviewed changes

her0807 approved these changes Mar 17, 2024

View reviewed changes

her0807 merged commit a3c10d3 into woowacourse:reddevilmidzy Mar 17, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2단계 - 블랙잭 배팅] 레디(최동근) 미션 제출합니다. #725

[2단계 - 블랙잭 배팅] 레디(최동근) 미션 제출합니다. #725

reddevilmidzy commented Mar 15, 2024 •

edited

Loading

her0807 left a comment

her0807 Mar 16, 2024

her0807 Mar 16, 2024

her0807 Mar 16, 2024

reddevilmidzy Mar 16, 2024

her0807 Mar 16, 2024

reddevilmidzy Mar 17, 2024

her0807 Mar 16, 2024

her0807 Mar 16, 2024

her0807 Mar 16, 2024

her0807 Mar 16, 2024

reddevilmidzy Mar 16, 2024

reddevilmidzy Mar 17, 2024

her0807 Mar 16, 2024

her0807 Mar 16, 2024

reddevilmidzy Mar 16, 2024

her0807 Mar 16, 2024

reddevilmidzy Mar 16, 2024

reddevilmidzy left a comment

reddevilmidzy Mar 16, 2024

reddevilmidzy Mar 16, 2024

reddevilmidzy Mar 16, 2024

reddevilmidzy Mar 16, 2024

reddevilmidzy Mar 17, 2024

reddevilmidzy Mar 17, 2024

her0807 left a comment

		@@ -2,15 +2,16 @@

		## 참여자

		* [x] 다시한번 룰 숙지 필요

		Assertions.assertThat(sum21Size3.calculateResult(sum20Size2)).isEqualTo(GameResult.WIN);
		Assertions.assertThat(sum20Size2.calculateResult(sum21Size3)).isEqualTo(GameResult.LOSE);

		final Player blackJackPlayer = new Player(new Name("수달"), blackJack);
		final Player noBlackJackPlayer = new Player(new Name("레디"), sum18Size2);

[2단계 - 블랙잭 배팅] 레디(최동근) 미션 제출합니다. #725

[2단계 - 블랙잭 배팅] 레디(최동근) 미션 제출합니다. #725

Conversation

reddevilmidzy commented Mar 15, 2024 • edited Loading

고민했던 점

배팅 금액 입력

리포지토리 형태로 구현했던 이유

BetAmount를 필드로 가지고 있는 형태로 구현한 이유

대박 커진 컨트롤러

여기서 질문!

her0807 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

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

reddevilmidzy 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

her0807 left a comment

Choose a reason for hiding this comment

reddevilmidzy commented Mar 15, 2024 •

edited

Loading