숫자 야구 게임 [SangBeom-Hahn] #2

SangBeom-Hahn · 2023-06-30T14:43:14Z

model 구현, 테스트 완료 / controller 구현 중

- stream()을 사용하기 위해 String이었던 랜덤 수를 List로 변환 - 테스트를 위해 private을 public으로 변환

sjiwon

필요한 부분

Optimize Import 생활화
매직넘버 및 코드 가독성에 대한 고민을 가지는 시간
파일 끝 EOF 신경써서 처리
기본적인 접근 제한자 개념

sjiwon · 2023-07-01T02:49:16Z

docs/README.md

+### 요구사항 내용
+1. 컴퓨터
+   - 1 ~ 9까지 서로 다른 임의의 수 3개를 선택한다.
+   - 사용자가 입력한 숫자에 대한 결과를 출력한다.


일상적인 관점에서는 컴퓨터가 결과를 출력하지만 이 게임상에서는 컴퓨터 & 사용자를 Player로 보고 결과를 도출하는 것은 다른 컴포넌트에게 위임하는게 좋지 않을까요?

sjiwon · 2023-07-01T02:52:36Z

src/main/java/baseball/controller/BaseballController.java

+    public void terminateGame(User user) {
+        if (isTerminate()) {
+            user.terminate();
+            System.out.println("완전 정료");


이런 출력은 요구사항에 없는데 임의로 넣으신건가요

sjiwon · 2023-07-01T02:52:41Z

src/main/java/baseball/controller/BaseballController.java

+    }
+    public boolean isTerminate() {


sjiwon · 2023-07-01T02:53:10Z

src/main/java/baseball/controller/BaseballController.java

+        validateNotStringRestartStatus(restartStatus);
+
+
+        if(restartStatus.equals("2") ){


2줄을 개행한 이유가 있나요

그리고 똑같은 검증을 2번하고 있네요

sjiwon · 2023-07-01T02:59:05Z

src/main/java/baseball/controller/BaseballController.java

+    public void validateRangeRestartStatus(final String restartStatus) {
+        if (Integer.parseInt(restartStatus) < 1 || Integer.parseInt(restartStatus) > 2) {
+            throw new IllegalArgumentException("재시작은 1, 완전 종료는 2 입니다.");
+        }
+    }
+
+    public void validateNotStringRestartStatus(final String restartStatus) {
+        if ( !(restartStatus != null && restartStatus.matches("[-+]?\\d*\\.?\\d+")) ) {
+            throw new IllegalArgumentException("재시작은 1, 완전 종료는 2인 정수입니다.");
+        }
+    }
+
+    public void validateNotDoubleRestartStatus(final String restartStatus) {
+        if (!restartStatus.chars().allMatch(Character::isDigit)) {
+            throw new IllegalArgumentException("재시작은 1, 완전 종료는 2인 정수로 소수를 입력할 수 없습니다.");
+        }
+    }
+


validateRangeRestartStatus를 가장 먼저 검증하고 있는데

Int가 아닌 값

숫자가 아닌 값

중간에 Space Separator 존재

이 경우는 어떻게 대응하실건가요?

그리고 restart에 필요한 검증을 크게보면

사용자가 숫자를 입력했냐

그 숫자가 1, 2중에 하나인가

이렇게 나올거같은데 위의 3가지 검증은 포커스가 어긋난거 같은데 어떻게 생각하시나요?

정수냐 소수냐 -> 따로 정수 판별에 대한 boolean method로 판별하면 될거같은데

sjiwon · 2023-07-01T03:05:49Z

src/main/java/baseball/model/Computer.java

+    public boolean hasDuplicateDigitInRandomNumber(Integer digit) {
+        if (randomNumber.contains(digit)) {
+            return true;
+        }
+        return false;
+    }


마찬가지로 1줄로 줄일 수 있네요

sjiwon · 2023-07-01T03:10:41Z

src/main/java/baseball/model/Computer.java

+    public List<Integer> getRandomNumber() {
+        return randomNumber;
+    }


일반적으로 getter/setter/equals와 같은 메소드들은 클래스 가장 하단에 위치하는게 관례입니다

외부에서 변경할 이유가 없다면 mutable -> immutable

sjiwon · 2023-07-01T03:11:19Z

src/main/java/baseball/model/User.java

+    public void validateNumber(final List<Integer> number) {
+        if (number.size() > 3 || number.size() < 3) {
+            throw new IllegalArgumentException("입력 숫자는 3자리입니다.");
+        }
+
+        if (!number.stream().allMatch(digit -> digit >= 1 && digit <= 9)) {
+            throw new IllegalArgumentException("입력 숫자는 3자리입니다.");
+        }
+    }


매직넘버는 위에서 말했듯이 따로 관리해서 가독성 좋게 표현해주세요

그리고 if 인자로 stream을 한줄로 배치하셨는데 가독성이 너무 안좋지 않을까요?
차라리 메소드로 빼서 가독성을 좋게 하는게 나을거같네요

Exception Message만 보면 동일한 메시지를 넘기는데 2가지 검증의 차이가 드러나게 표현해주세요

sjiwon · 2023-07-01T03:14:18Z

src/main/java/baseball/model/Computer.java

+import java.util.List;
+
+public class Computer {
+    public List<Integer> randomNumber;


이 필드는 왜 public으로 열어둔거죠?

sjiwon · 2023-07-01T03:15:45Z

src/test/java/baseball/model/ComputerTest.java

+
+        // then
+        assertThat(randomNumber.stream().allMatch(digit -> digit >= 111 && digit <= 999));
+        System.out.println(randomNumber);


테스트 코드에서 sout으로 출력을 하는 이유가 있나요

sjiwon · 2023-07-01T03:28:01Z

src/test/java/baseball/model/ComputerTest.java

+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.*;
+
+class ComputerTest {


각각의 테스트가 무엇을 위한 테스트인지는 알겠는데 private scope로 두어야 할 메소드들도 테스트를 위해서 public으로 열어둔 것처럼 보이네요

클래스의 모든 메소드를 테스트하기 위해서 public으로 열어두기 보다 private scope로 두어야할 메소드는 private scope로 두고 해당 메소드를 활용하는 의미있는 public method를 테스트하면 되지 않을까요?

https://shoulditestprivatemethods.com/

sjiwon · 2023-07-01T03:28:55Z

src/test/java/baseball/model/UserTest.java

+        User user = new User();
+
+        // when
+        ArrayList<Integer> number = new ArrayList<>(Arrays.asList(1, 2));


타입을 List로 하지않고 ArrayList로 둔 이유가 있나요?

추가적으로 new ArrayList<>(Arrays.asList(1, 2))로 구현한 이유가 있나요? 그냥 Arrays.asList(1, 2)로 해도 되지않아요?

그리고 Arrays.asList와 List.of의 차이도 뭔지 알고계시나요?

sjiwon · 2023-07-01T03:31:54Z

src/test/java/baseball/model/UserTest.java

+    @Test
+    @DisplayName("RestartStatus 테스트")
+    void UserTest() {
+        // given
+        User user = new User();
+
+        // when
+        user.terminate();
+
+        // then
+        assertThat(user.restartStatus).isEqualTo(User.RestartStatus.RESTART);
+    }


이 테스트 깨지는데요
기본적으로 PR을 생성할때는 구현한 로직에 대해서 로컬에서 모든 테스트가 통과하는지 확인하고 올려주세요

- 종료 신호 사용자 입력 메서드 분리 - BaseballControllerTest 코드 terminateTest 메서드에서 console 입력 부분 String 변수 초기화로 대체

- 테스트를 위해 컴퓨터의 생성자에 save 메서드 주석처리

- controller의 playGame 메서드에 주석은 resultTest 출력을 보기 위한 것으로 이렇게 해도 되는지 질문하고 싶습니다.

- user의 RestartStatus에 재시작 여부를 입력하던 terminateGame 메서드를 삭제 - gameOver 메서드에서 terminateGame 메서드 제거 - 게임 재시작시 컴퓨터의 랜덤수가 동일한 문제 해결

SangBeom-Hahn · 2023-07-03T03:54:46Z

일단은 제 방식대로 리팩토링했고 커멘트에 대한 답변과 리팩토링을 할 예정입니다.

SangBeom-Hahn added 9 commits June 28, 2023 16:46

docs(README): list requirements and feature specifications

d3a43d2

feat(Computer): 상대방 컴퓨터 객체 구현

79f3e7b

test(Computer): 상대방 컴퓨터 객체 테스트

bf40464

refactor(Computer): 상대방 컴퓨터 랜덤 수 문자열이었던 것 List로 리팩토링

3861edb

- stream()을 사용하기 위해 String이었던 랜덤 수를 List로 변환 - 테스트를 위해 private을 public으로 변환

feat(User): 사용자 객체 구현

ff86b26

feat(User): 숫자를 대입하는 setNumber() 기능 추가

7dd776f

test(User): 사용자 객체 테스트

6e1f6e5

feat(BaseballController): 사용자가 랜덤 수를 맞추면 재시작(1) / 종료(2) 기능 구현

b072ddf

test(BaseballController): 사용자가 랜덤 수를 맞추면 재시작(1) / 종료(2) 기능 테스트

cd05f8e

SangBeom-Hahn requested a review from sjiwon June 30, 2023 14:43

sjiwon changed the title ~~model 구현, 테스트 완료 / controller 구현 중~~ 숫자 야구 게임 [SangBeom-Hahn] Jul 1, 2023

sjiwon reviewed Jul 1, 2023

View reviewed changes

SangBeom-Hahn added 16 commits July 1, 2023 17:15

refactor(BaseballController): 사용자가 랜덤 수를 맞추면 재시작(1) / 종류(2) 기능 리팩토링

c9f6dab

- 종료 신호 사용자 입력 메서드 분리 - BaseballControllerTest 코드 terminateTest 메서드에서 console 입력 부분 String 변수 초기화로 대체

feat(BaseballController): 사용자 숫자 입력 시 결과 반환(스트라이크, 볼, 낫싱) 기능 구현

968677a

test(BaseballController): 사용자 숫자 입력 시 결과 반환(스트라이크, 볼, 낫싱) 기능 테스트

8b975fc

- 테스트를 위해 컴퓨터의 생성자에 save 메서드 주석처리

feat(BaseballController): 게임 시작 기능 구현

d498845

feat(BaseballController): 사용자 3자리 숫자 입력 기능 구현

8ba8368

test(BaseballController): 사용자 3자리 숫자 입력 기능 테스트

3e23707

feat(BaseballController): 사용자 3자리 숫자 입력에 따른 결과 반환 기능 구현

63e284e

test(BaseballController): 사용자 3자리 숫자 입력에 따른 결과 반환 기능 테스트

6faf838

- controller의 playGame 메서드에 주석은 resultTest 출력을 보기 위한 것으로 이렇게 해도 되는지 질문하고 싶습니다.

feat(BaseballController) : 게임 종료 기능 구현

79a73e2

test(BaseballController) : 게임 종료 기능 테스트

45901c9

refactor(OutputView) : 하드코딩 했던 것 view 분리 및 상수로 리팩토링

53e2e2f

refactor(InputView) : 하드코딩 했던 것 view 분리 및 상수로 리팩토링

4c24258

refactor(BaseballController) : 하드코딩 했던 것 상수로 리팩토링

c4321d5

refactor(User) : 하드코딩 했던 것 상수로 리팩토링

4aa97f2

refactor(BaseballController) : User, Computer 지역변수에서 전역 변수로 리팩토링

c133bc3

SangBeom-Hahn added 4 commits July 3, 2023 11:27

refactor(BaseballController) : strikeCount, ballCount 지역변수에서 전역 변수로 리팩토링

787d1e7

refactor(BaseballController) : strike, ball 카운트 메서드 stream으로 리팩토링

4e39001

fix(OutputView) : strike, ball의 갯수가 0일 때 0도 출력되는 버그 수정

fbf0807

refactor : 접근 지정자 리팩토링

f4391a2

refactor : 검증 추가 리팩토링

0fe1f31

		validateNotStringRestartStatus(restartStatus);


		if(restartStatus.equals("2") ){

숫자 야구 게임 [SangBeom-Hahn] #2

Are you sure you want to change the base?

숫자 야구 게임 [SangBeom-Hahn] #2

Uh oh!

Conversation

SangBeom-Hahn commented Jun 30, 2023 • edited by sjiwon Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sjiwon left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjiwon Jul 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjiwon Jul 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjiwon Jul 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sjiwon Jul 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SangBeom-Hahn commented Jul 3, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SangBeom-Hahn commented Jun 30, 2023 •

edited by sjiwon

Loading

sjiwon left a comment •

edited

Loading

sjiwon Jul 1, 2023 •

edited

Loading

sjiwon Jul 1, 2023 •

edited

Loading

sjiwon Jul 1, 2023 •

edited

Loading

sjiwon Jul 1, 2023 •

edited

Loading