[1단계 - 블랙잭 게임 실행] 히이로(문제웅) 미션 제출합니다. #447

MoonJeWoong · 2023-03-03T08:34:33Z

안녕하세요 코즈! 히이로입니다.

일단 이번 블랙잭 미션을 페어와 진행하는데 있어 시간이 정말 많이 부족했던 것 같습니다... 보시면 아시겠지만 현재 코드는 중간 이후부터 TDD 적용도 못했고 코드도 시간에 쫓겨 너무 급하게 작성했던 것 같습니다... 죄송합니다... 😭

그럼에도 불구하고 구현하면서 궁금했던 점을 일단 남겨보고자 합니다.

인터페이스 사용

이번 미션을 진행하면서 카드 객체를 구현하는 것과 유저 객체를 구현하는 것에 신경을 많이 쏟았던 것 같습니다.
카드 객체의 경우 각 문양 별로 Enum 생성을 통해 구현하였습니다. Cards 클래스에서 52장의 카드 Enum 객체들을 한 List에서 관리하기 위해서 Card라는 interface를 정의해주었습니다. 페어와 급하게 속도를 내며 진행하다 보니 해당 구현 내용에 대해 생각을 깊게 하지 못했었는데 혹시 잘 못 사용한 부분이 있다면 알려주시면 감사하겠습니다!

추상클래스 사용
딜러와 플레이어 클래스가 상속받는 추상클래스인 User 클래스를 정의하였습니다. 이번 미션의 요구사항으로 딜러와 플레이어의 중복 코드를 제거해야 한다는 요구사항을 수행하기 위해 해당 방식으로 작성했습니다. 그러나 급하게 진행하다 보니 딜러와 플레이어의 중복 코드를 제거하는 방법이 추상클래스를 사용하는 방식이 맞는가? 에 대해서는 깊게 고민해보지 못한 것 같습니다. 혹 다른 방법이 있었다면 리뷰 달아주시면 감사하겠습니다.
추상클래스의 TDD
추상 클래스는 스스로 객체를 생성하지 못합니다. 그렇기에 User 클래스의 테스트를 PlayerTest를 통해 진행한 부분이 있는데 이렇게 테스트를 진행하는 방식이 맞는지 궁금했습니다.

코드를 작성하고 리팩토링을 거의 하지 못해 많이 더러운 것 같습니다... 리뷰 주시면 최대한 빠르게 고쳐보도록 하겠습니다 ㅠㅠ

세부사항 - isEmpty() 메서드를 private 메서드로 변경함에 따른 테스트 코드 삭제

세부사항 - 1에서 11로 변경

세부사항 - 단순 합산 점수 계산 - 에이스가 포함된 합산 점수 계산

세부사항 - List<Card>를 받는 것으로 수정

세부사항 - User 추상 클래스 정의

세부사항 - Player, Dealer 클래스 공통 기능 추출

세부사항 - drawCommand 정적 팩토리 메서드 내에서 map을 사용하도록 수정

세부사항 - user 내부에서 게임 승패 정보를 저장하는 상태 값 삭제 - 게임 승패 정보 판단 및 저장 기능 GameResult로 이관

세부사항 - 기존 inputView에서 수행하던 검증 기능을 Names 클래스로 이관

MoonJeWoong

안녕하세요 코즈! 히이로입니다.

1차 리뷰에 대한 리팩토링이 개인 사정으로 인해 많이 늦어졌습니다. 죄송합니다 😭

맨 처음 코멘트에서 질문 드린 사항들 중 2번에 대한 설명만 User 클래스 쪽에 코멘트로 달아두었습니다.

먼저 말씀드리고 싶은 부분은 처음에도 말씀드렸다시피 페어 미션 과정에서 워낙 급하게 구현하느라 이전 미션에서 배운 부분이나 이번 미션에서 주어진 요구사항들 중 지키지 못하고 놓친 부분들이 많습니다. 거기에 코즈가 1차적으로 달아주신 리뷰들을 보며 리팩토링을 어떻게 해야할지 고민하다보니 고치고 싶은 부분이 너무 많이 보이는 것 같아요.

그러나 지금 코드를 전부 엎어가며 리팩토링을 진행하기엔 저나 리뷰어인 코즈에게도 좋은 것은 아닌것처럼 느껴져서 1차적으로 리뷰 주신 부분들만 최대한 집중해서 리팩토링을 진행했습니다. 제 개인적인 의견으로는 2차 미션을 병행하면서 전반적으로 놓친 부분들을 수정하는 것이 좋아보이는데 코즈의 의견은 어떠신지 궁금합니다.

혹시 1단계에서 반드시 짚고 넘어가야할 부분이 있거나 제가 놓친 것이 있다면 알려주시면 감사하겠습니다. 🙇

MoonJeWoong · 2023-03-06T08:29:00Z

src/main/java/controller/BlackJackGameController.java

+ while (player.getStatus().equals(PlayerStatus.NORMAL) && readDrawCommand(player).equals(DrawCommand.DRAW)) {
+ blackJackGame.drawOneMoreCardForPlayer(player);
+ showDrawResult(player);
+ }
+ if (player.getStatus().equals(PlayerStatus.NORMAL)) {


넵 정신없이 구현하다 보니 객체 내부에서 수행가능한 부분도 불필요하게 getter를 사용했네요... User 클래스를 상속받는 클래스들에 대해서 전반적으로 수정했습니다!

MoonJeWoong · 2023-03-06T08:35:01Z

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

+ }
+
+ public void drawCardUntilOverSixteen() {
+ while (dealer.getStatus().equals(DealerStatus.UNDER_SEVENTEEN)) {


User 클래스를 상속받는 클래스들에 대해서 전반적으로 수정했습니다!

MoonJeWoong · 2023-03-06T08:35:11Z

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

+ if (playerStatus.equals(PlayerStatus.BUST)) {
+ dealerWin(player);
+ return;
+ }
+ if (playerStatus.equals(PlayerStatus.NORMAL) && dealer.getStatus().equals(DealerStatus.BUST)) {


User 클래스를 상속받는 클래스들에 대해서 전반적으로 수정했습니다!

MoonJeWoong · 2023-03-06T11:28:24Z

src/main/java/domain/DrawCommand.java

+ }
+
+ public static DrawCommand of(String inputCommand) {
+ return Arrays.stream(values())


추후에 command 개수가 많아지면 성능 상 values()로 순회하는 것보다는 Map을 사용한 접근이 더 빠르겠네요. 해당 부분 수정 완료했습니다!

MoonJeWoong · 2023-03-06T12:09:51Z

src/main/java/domain/card/Cards.java

+
+ public Card drawCard() {
+ if (isEmpty()) {
+ throw new IllegalArgumentException("[ERROR] 카드가 존재하지 않습니다.");


게임의 참여자 수를 5명으로 제한해뒀기에 52장의 카드가 전부 바닥나는 상황은 없을 것이라 판단했었습니다. 그러나 이후 변경사항으로 인해 카드가 52장을 전부 사용하게 되는 경우도 생각해서 넣은 예외 처리입니다.

코즈의 얘기를 듣고 보니 해당 부분은 메서드의 매개변수가 잘 못 주어진 것이 아니라 도메인의 상태가 현재 기능을 수행하기에 적절하지 않은 상태를 나타내야 하기에 IllegalStateException 예외를 처리하도록 변경하였습니다.

미션에서 사용자 입력 오류시 재입력 등과 같은 요구사항은 없었기에 따로 에러 메세지를 콘솔로 출력하는 기능은 고려하지 않았습니다. 하지만 만약 콘솔 단에서 사용자에게 에러 메세지를 출력해야 한다면 try catch문을 사용해서 exception 객체 내부 메세지를 이용해 출력하도록 할 것 같습니다.

혹 해당 부분에 대해서 추가로 수정할 부분이 있다면 알려주시면 감사하겠습니다!

MoonJeWoong · 2023-03-07T13:18:08Z

src/main/java/view/InputView.java

+
+ public List<String> readPlayersName() {
+ List<String> playersName = Arrays.asList(scanner.nextLine().split(","));
+ validatePlayersSize(playersName);


코즈 말에 동의합니다. 저는 이전 미션을 통해 입력 값 등의 검증은 view. domain에서 모두 해줘야 한다고 결론을 내린 상태였었는데 이번에는 시간에 쫓겨 미처 더 신경쓰지 못했습니다.

사용자의 이름 값들을 입력받아 Name 객체를 생성해주면서 중복 이름 값을 검증 및 예외 처리할 수 있도록 Names 클래스를 구현하여 해당 책임을 부여했습니다.

플레이어 이름 값이 5개를 초과하는지 여부는 코즈 말대로 입출력 로직 보다는 도메인 로직에 더 가까워 보입니다. 그래서 Names 객체 내에서 검증 후 Name 객체들을 생성할 수 있도록 리팩토링했습니다.

MoonJeWoong · 2023-03-07T13:43:24Z

src/main/java/domain/user/Player.java

+ }
+
+ @Override
+ public boolean equals(Object other) {


미션을 진행하면서 동명이인은 존재할 수 없는 것으로 정하고 구현했습니다. 그렇기에 사용자의 입력을 사용해 생성된 Player 객체들은 서로 다른 이름을 가져야 함이 옳습니다. 만약에라도 같은 이름을 가지는 Player가 존재하게 된다면 동일한 객체로 취급되어야 합니다.

그러나 처음 리뷰 요청을 드릴 당시에 여유가 없어서 입력 단에서 중복되는 플레이어 이름을 검증하는 기능을 미처 구현하지 못했었습니다. 이번에 리팩토링을 진행하며 해당 기능을 Names 클래스에서 수행하도록 구현하였습니다.

같이 첨부해주신 레퍼런스를 살펴보며 느낀 것은 동일성(==)을 확인하여 Player간 비교를 할 수 있도록 해야한다는 것입니다. 중복되어 입력된 이름에 대한 검증 기능이 추가된 이후 Player 객체들은 서로 다른 이름을 가질 것이고 같은 이름을 가진 Player 객체가 생성되지 않는다고 봐야 하기 때문입니다.

위와 같은 사고를 거쳐 Player 클래스 내부에서 equals, hashCode 오버라이딩을 제거하였습니다. 다만 추가로 궁금한 점은 지금과 같이 Player를 동등성(equals)이 아닌 동일성(==)으로 확인하는 경우에 이 객체가 Map 자료구조의 key값으로 사용되어도 equals & hashCode를 오버라이딩 해 줄 필요가 없다고 생각하시는지 궁금합니다!

MoonJeWoong · 2023-03-07T13:52:51Z

src/main/java/domain/user/Score.java

+import java.util.List;
+import java.util.stream.Collectors;
+
+public class Score {


리뷰 주신 부분을 고민해본 결과 말씀해주신 Cards 객체는 플레이어가 손에 들고 있는 카드를 나타내는 hands의 역할을 의미하는 것처럼 이해가 됐습니다.

제가 이해한 바가 맞다면 리뷰 요청 시 구현되어 있던 Cards 클래스는 유저가 들고있는 카드의 상황 정보를 저장하는 역할이 아니라 52장의 카드를 관리하는 Deck에 더 가까운 역할을 수행하고 있었습니다. 따로 hands 역할을 수행하는 클래스는 구현하지 않은 상황이었습니다.

리팩토링을 진행하며 기존 Cards 클래스 명을 GameDeck으로 수정했습니다. 그리고 아직 구현하진 않았지만 User 클래스를 상속받는 객체들의 상태 값인 cards를 관리할 수 있는 hands 클래스를 구현하고 이 내부에서 score를 관리할 수 있게 할 수 있지 않을까 생각을 했습니다. 다만 지금도 생각보다 너무 많은 부분을 뜯어 고치려고 하는 것 같아서 지양하고 계획만 정리해 둔 상태입니다.

위와 같은 방식으로 hands 클래스를 구현하고 내부에서 score를 관리하도록 하는 것이 코즈가 남긴 리뷰의 의도라고 저는 파악했는데 혹시 제가 잘못 이해한 부분이 있을까요?

MoonJeWoong · 2023-03-07T15:40:29Z

src/main/java/domain/user/UserStatus.java

@@ -0,0 +1,4 @@
+package domain.user;
+
+public interface UserStatus {


User 클래스를 상속하는 Player와 Dealer는 각각 PlayerStatus, DealerStatus 상태 값을 이용하여 자신이 어떤 상태인지 관리하고 있습니다.

player와 dealer가 공통적으로 자신의 상태를 외부에서 메세지를 보냄으로써 확인할 수 있도록 하려고 했습니다.

그러나 해당 기능을 User 추상 클래스에 구현하려고 보니 Player와 Dealer의 상태 값 Enum 클래스가 달라서 추상 메서드를 정의해주기가 애매했습니다.

그래서 시간 관계상 깊게 고민하지 못하고 PlayerStatus와 DealerStatus가 UserStatus라는 인터페이스를 상속받게 함으로써 이를 해결해보고자 했습니다.

그러나 막상 UserStatus 인터페이스에 명시할만한 기능은 존재하지 않아 이런 결과를 낳게 되었습니다.

어떤 방식으로 리팩토링 해야할 지 아직 감이 잘 안 잡히는데 이런 경우라면 차라리 추상 메서드 선언을 포기하고 인터페이스를 삭제하는 방향이 좋을지 고민입니다.

MoonJeWoong · 2023-03-07T16:01:33Z

src/main/java/domain/user/User.java

+import java.util.ArrayList;
+import java.util.List;
+
+public abstract class User {


맨 처음 코멘트에서 질문드린 사항 중 2번에 해당하는 질문 내용입니다. 이번에 추상 클래스를 처음 사용해보면서 인스턴스 변수와 메서드의 접근제어자가 private인 경우 구현체 클래스에서 접근을 못하는 것을 확인할 수 있었습니다. 그래서 급하게 진행하던 과정에서 컴파일 오류가 사라지는 protected 접근 제어자를 사용했었는데 해당 방식에서 잘못된 부분이 있었는지 질문 드린 것이라고 이해해주시면 좋겠습니다.

해당 부분은 제가 추상클래스에 대해 좀 더 공부해보고 다음 리팩토링때 적용해보도록 하겠습니다!

컴파일 오류를 해결하기 위해 사용했다기 보단, 상속받은 객체가 접근할 수 있도록 접근제어자를 변경했다가 맞는것 같네요 :)

kouz95

안녕하세요 히이로 :)
질문주신 부분에 답변 드렸으니 확인해주세요~!
더 수정하고 싶은 부분이 있어, 2단계에 추가적으로 수정해주시는 것으로 이해했습니다.
고생하셨습니다 🙏

kouz95 · 2023-03-10T00:51:26Z

src/main/java/domain/user/Score.java

+import java.util.List;
+import java.util.stream.Collectors;
+
+public class Score {


List 를 파라미터로 받아 점수를 계산하고 있다면, Cards 객체에서도 수행이 가능할텐데 분리한 이유가 무엇인지 질문드렸던 부분입니다 :)

kouz95 · 2023-03-10T00:52:46Z

src/main/java/domain/user/User.java

+import java.util.ArrayList;
+import java.util.List;
+
+public abstract class User {


컴파일 오류를 해결하기 위해 사용했다기 보단, 상속받은 객체가 접근할 수 있도록 접근제어자를 변경했다가 맞는것 같네요 :)

kouz95 · 2023-03-10T00:53:25Z

src/main/java/domain/user/UserStatus.java

@@ -0,0 +1,4 @@
+package domain.user;
+
+public interface UserStatus {


player와 dealer가 공통적으로 자신의 상태를 외부에서 메세지를 보냄으로써 확인할 수 있도록 하려고 했습니다.

외부로 메세지를 보내는 부분을 메서드로 추출할 순 없을까요?

yujamint added 30 commits February 28, 2023 17:12

docs: 초기 기능목록 작성

1f36604

test: 블랙잭 게임 카드 생성 시 숫자 범위 검증 테스트 작성

48dc730

feat: 숫자 Enum 생성 기능 구현

0828762

feat: 문양 Enum 생성 기능 구현

bfeb070

test: 블랙잭 게임 카드 생성 테스트 작성

856ecd7

feat: 문양 별 카드 Enum 생성 기능 구현

4d6b5e4

feat: Card 인터페이스 생성 및 구현

13e5f6a

test: Card 인터페이스 생성 및 구현에 따른 테스트 코드 수정

f332d63

test: 블랙잭 게임 전체 카드 생성 테스트 작성

cc44eba

feat: 블랙잭 게임 전체 카드 생성 기능 구현

dec478d

test: 블랙잭 게임 카드 뽑기 기능 테스트 작성

e862067

세부사항 - isEmpty() 메서드를 private 메서드로 변경함에 따른 테스트 코드 삭제

feat: 블랙잭 게임 카드 뽑기 기능 구현

61ba09b

feat: 블랙잭 게임 카드 생성 후 섞기(셔플) 기능 구현

bd51b99

refactor: 블랙잭 게임 에이스 기본 숫자값 변경

8cfb433

세부사항 - 1에서 11로 변경

test: 보유하고 있는 카드에 따른 점수 계산 기능 테스트 작성

9b30aa9

세부사항 - 단순 합산 점수 계산 - 에이스가 포함된 합산 점수 계산

feat: 보유하고 있는 카드에 따른 점수 계산 기능 구현

60bd792

세부사항 - 단순 합산 점수 계산 - 에이스가 포함된 합산 점수 계산

feat: 카드 getter 추가

13fdbbc

test: 점수 계산 기능 매개변수 수정

8a551f3

세부사항 - List<Card>를 받는 것으로 수정

feat: 점수 계산 기능 매개변수 수정

f4315ad

세부사항 - List<Card>를 받는 것으로 수정

test: 게임 시작 직후 카드 두 장씩 분배하는 기능 테스트 작성

7521926

feat: 게임 시작 직후 카드 두 장씩 분배하는 기능 구현

ed8c5e8

test: Player 초기 세팅 테스트 작성

b1bc0e4

세부사항 - User 추상 클래스 정의

feat: User 추상 클래스 정의

7cfb845

세부사항 - Player, Dealer 클래스 공통 기능 추출

feat: Player 객체 생성 기능 구현

034b4ef

feat: User 카드 정보 반환 기능 구현

fa64bec

style: 패키지 명 수정

821a5fe

test: 플레이어가 카드 한 장을 받아 패에 넣는 기능 테스트 작성

88ff32a

feat: 플레이어가 카드 한 장을 받아 패에 넣는 기능 구현

acd1057

test: 플레이어의 버스트 판단 기능 테스트 작성

1ee0180

feat: 플레이어의 버스트 판단 기능 구현

3250caf

MoonJeWoong added 27 commits March 6, 2023 17:28

refactor: user들의 상태 값을 객체 내부에서 비교할 수 있도록 수정

8c5ad9e

style: 불필요한 코드 주석 및 스타일 개선

b525b15

test: 사용자 입력값에 따른 DrawCommand 반환 테스트 코드 작성

deb265a

feat: 사용자 입력값에 따른 DrawCommand 반환 기능 수정

378f5ee

세부사항 - drawCommand 정적 팩토리 메서드 내에서 map을 사용하도록 수정

refactor: Card 객체 내 뽑을 카드가 없을시 발생 예외 종류 수정

584541b

refactor: Exception 객체 에러메세지 헤더 추가

282d7dc

test: 발생 exception 종류 변경으로 인한 테스트 코드 수정

781171f

test: GameResult 객체 생성 테스트 작성

c1fad5b

feat: GameResult 객체 생성 기능 구현

462f0a0

test: 기존 cards 클래스를 GameDeck 클래스로 리팩토링한 이후 테스트 작성

c27a0b0

refactor: 기존 cards 클래스를 GameDeck 클래스로 변경

3dd908c

feat: 전략패턴을 위한 DeckGenerator 구현

3a72ffb

refactor: Cards에서 GameDeck 클래스로 리팩토링으로 인한 코드 수정

ff22bf8

test: GameResult user 결과 계산 기능 테스트 작성

7e00ba1

feat: GameResult user 결과 계산 기능 구현

611a864

refactor: 기존 결과 계산 기능 GameResult 객체를 사용하도록 수정

c6d351b

refactor: GameResult 구현으로 인한 User 클래스 상태 값 제거

87a33e8

세부사항 - user 내부에서 게임 승패 정보를 저장하는 상태 값 삭제 - 게임 승패 정보 판단 및 저장 기능 GameResult로 이관

test: 이름 값이 하나만 입력될 수 있는 기능 테스트 코드 추가

e30f15a

feat: 이름 값이 하나만 입력될 수 있는 기능 추가

ccbab7a

feat: Name 객체 간 같은 이름은 동일한 사람으로 취급하도록 기능 추가

63d9c61

test: 입력된 이름 값이 중복될 시 예외처리 기능 테스트 추가

a8ab2ec

feat: 입력된 이름 값이 중복될 시 예외처리 기능 구현

053c264

feat: controller에서 Names를 통해 플레이어 이름 값을 입력받도록 구현

71bb845

test: 입력된 이름 값이 5개를 초과할 경우 예외처리 기능 테스트 작성

e48d756

refactor: 입력된 이름 값이 5개를 초과할 경우 예외처리 기능 구현

f5a0f31

세부사항 - 기존 inputView에서 수행하던 검증 기능을 Names 클래스로 이관

refactor: player 객체 간 동일성 비교를 위한 equals & hashCode 삭제

37864cf

refactor: 예외처리 에러 메세지 헤더 통일

9aea859

MoonJeWoong commented Mar 7, 2023

View reviewed changes

kouz95 approved these changes Mar 10, 2023

View reviewed changes

kouz95 merged commit b679262 into woowacourse:moonjewoong Mar 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 블랙잭 게임 실행] 히이로(문제웅) 미션 제출합니다. #447

[1단계 - 블랙잭 게임 실행] 히이로(문제웅) 미션 제출합니다. #447

MoonJeWoong commented Mar 3, 2023 •

edited

Loading

MoonJeWoong left a comment

MoonJeWoong Mar 6, 2023

MoonJeWoong Mar 6, 2023

MoonJeWoong Mar 6, 2023

MoonJeWoong Mar 6, 2023

MoonJeWoong Mar 6, 2023

MoonJeWoong Mar 7, 2023

MoonJeWoong Mar 7, 2023

MoonJeWoong Mar 7, 2023

MoonJeWoong Mar 7, 2023

MoonJeWoong Mar 7, 2023

kouz95 Mar 10, 2023

kouz95 left a comment

kouz95 Mar 10, 2023

kouz95 Mar 10, 2023

kouz95 Mar 10, 2023 •

edited

Loading

		@@ -0,0 +1,4 @@
		package domain.user;

		public interface UserStatus {

[1단계 - 블랙잭 게임 실행] 히이로(문제웅) 미션 제출합니다. #447

[1단계 - 블랙잭 게임 실행] 히이로(문제웅) 미션 제출합니다. #447

Conversation

MoonJeWoong commented Mar 3, 2023 • edited Loading

MoonJeWoong 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

kouz95 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kouz95 Mar 10, 2023 • edited Loading

Choose a reason for hiding this comment

MoonJeWoong commented Mar 3, 2023 •

edited

Loading

kouz95 Mar 10, 2023 •

edited

Loading