Skip to content
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
d3a43d2
docs(README): list requirements and feature specifications
SangBeom-Hahn Jun 28, 2023
79f3e7b
feat(Computer): 상대방 컴퓨터 객체 구현
SangBeom-Hahn Jun 28, 2023
bf40464
test(Computer): 상대방 컴퓨터 객체 테스트
SangBeom-Hahn Jun 29, 2023
3861edb
refactor(Computer): 상대방 컴퓨터 랜덤 수 문자열이었던 것 List로 리팩토링
SangBeom-Hahn Jun 29, 2023
ff86b26
feat(User): 사용자 객체 구현
SangBeom-Hahn Jun 29, 2023
7dd776f
feat(User): 숫자를 대입하는 setNumber() 기능 추가
SangBeom-Hahn Jun 30, 2023
6e1f6e5
test(User): 사용자 객체 테스트
SangBeom-Hahn Jun 30, 2023
b072ddf
feat(BaseballController): 사용자가 랜덤 수를 맞추면 재시작(1) / 종료(2) 기능 구현
SangBeom-Hahn Jun 30, 2023
cd05f8e
test(BaseballController): 사용자가 랜덤 수를 맞추면 재시작(1) / 종료(2) 기능 테스트
SangBeom-Hahn Jun 30, 2023
c9f6dab
refactor(BaseballController): 사용자가 랜덤 수를 맞추면 재시작(1) / 종류(2) 기능 리팩토링
SangBeom-Hahn Jul 1, 2023
968677a
feat(BaseballController): 사용자 숫자 입력 시 결과 반환(스트라이크, 볼, 낫싱) 기능 구현
SangBeom-Hahn Jul 1, 2023
8b975fc
test(BaseballController): 사용자 숫자 입력 시 결과 반환(스트라이크, 볼, 낫싱) 기능 테스트
SangBeom-Hahn Jul 1, 2023
d498845
feat(BaseballController): 게임 시작 기능 구현
SangBeom-Hahn Jul 1, 2023
8ba8368
feat(BaseballController): 사용자 3자리 숫자 입력 기능 구현
SangBeom-Hahn Jul 1, 2023
3e23707
test(BaseballController): 사용자 3자리 숫자 입력 기능 테스트
SangBeom-Hahn Jul 1, 2023
63e284e
feat(BaseballController): 사용자 3자리 숫자 입력에 따른 결과 반환 기능 구현
SangBeom-Hahn Jul 1, 2023
6faf838
test(BaseballController): 사용자 3자리 숫자 입력에 따른 결과 반환 기능 테스트
SangBeom-Hahn Jul 1, 2023
79a73e2
feat(BaseballController) : 게임 종료 기능 구현
SangBeom-Hahn Jul 1, 2023
45901c9
test(BaseballController) : 게임 종료 기능 테스트
SangBeom-Hahn Jul 1, 2023
7ccf4e9
refactor(BaseballController) : 게임 시나리오를 위한 run 메서드 리팩토링
SangBeom-Hahn Jul 1, 2023
53e2e2f
refactor(OutputView) : 하드코딩 했던 것 view 분리 및 상수로 리팩토링
SangBeom-Hahn Jul 3, 2023
4c24258
refactor(InputView) : 하드코딩 했던 것 view 분리 및 상수로 리팩토링
SangBeom-Hahn Jul 3, 2023
c4321d5
refactor(BaseballController) : 하드코딩 했던 것 상수로 리팩토링
SangBeom-Hahn Jul 3, 2023
4aa97f2
refactor(User) : 하드코딩 했던 것 상수로 리팩토링
SangBeom-Hahn Jul 3, 2023
c133bc3
refactor(BaseballController) : User, Computer 지역변수에서 전역 변수로 리팩토링
SangBeom-Hahn Jul 3, 2023
787d1e7
refactor(BaseballController) : strikeCount, ballCount 지역변수에서 전역 변수로 리팩토링
SangBeom-Hahn Jul 3, 2023
4e39001
refactor(BaseballController) : strike, ball 카운트 메서드 stream으로 리팩토링
SangBeom-Hahn Jul 3, 2023
fbf0807
fix(OutputView) : strike, ball의 갯수가 0일 때 0도 출력되는 버그 수정
SangBeom-Hahn Jul 3, 2023
f4391a2
refactor : 접근 지정자 리팩토링
SangBeom-Hahn Jul 3, 2023
0fe1f31
refactor : 검증 추가 리팩토링
SangBeom-Hahn Jul 3, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
# 요구사항 명세서
### 카테고리
1. 컴퓨터
2. 사용자
3. 시스템

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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




2. 사용자
- 서로 다른 3개의 숫자를 입력한다.
- 게임을 종료할 때 완전 종료와 다시 시작을 선택한다.


3. 시스템
- 게임 시작 문구를 출력한다.
- 사용자와 컴퓨터의 동작을 반복한다.
- 3개의 숫자를 모두 맞히면 게임을 종료한다.
게임이 종료할 때 사용자의 선택에 따라서 완전 종료하거나 다시 시작한다.
- 사용자가 잘못된 값을 입력할 경우 IllegalArgumentException을 발생시킨 후 App을 종료한다.

# 기능 목록
1. 게임 시작
- 게임 시작 문구 출력


2. 게임 진행
- 컴퓨터는 1 ~ 9까지 서로 다른 임의의 수 선택
- 사용자가 서로 다른 3개의 숫자를 입력
- 같은 수가 같은 자리에 있으면 스트라이크, 다른 자리에 있으면 볼, 전혀 없으면 낫싱 출력


3. 게임 종료
- 사용자가 컴퓨터의 수를 맞추면 게임 종료 출력
- 재시작(1) / 종료(2)
51 changes: 51 additions & 0 deletions src/main/java/baseball/controller/BaseballController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
package baseball.controller;

import baseball.model.User;
import camp.nextstep.edu.missionutils.Console;

public class BaseballController {
public void terminateGame(User user) {
if (isTerminate()) {
user.terminate();
System.out.println("완전 정료");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

} else {
user.restart();
}
}
public boolean isTerminate() {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

개행

String restartStatus = Console.readLine();

validateRangeRestartStatus(restartStatus);
validateNotStringRestartStatus(restartStatus);
validateNotStringRestartStatus(restartStatus);


if(restartStatus.equals("2") ){
Copy link
Collaborator

@sjiwon sjiwon Jul 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

return true;
} else {
return false;
}
}

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인 정수로 소수를 입력할 수 없습니다.");
}
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

  • Int가 아닌 값
  • 숫자가 아닌 값
  • 중간에 Space Separator 존재

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

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

  1. 사용자가 숫자를 입력했냐
  2. 그 숫자가 1, 2중에 하나인가

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

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

public void restartGame() {
// 게임을 끝내지 않을 것이라는 신호를 줌
}
}
48 changes: 48 additions & 0 deletions src/main/java/baseball/model/Computer.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package baseball.model;

import camp.nextstep.edu.missionutils.Randoms;

import java.util.ArrayList;
import java.util.List;

public class Computer {
public List<Integer> randomNumber;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


public Computer() {
saveRandomNumberWithGameStart();
}
Comment on lines +14 to +16
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

save를 통해서 randomNumber에 대한 init을 한다는거는 알겠는데
위의 코드보다는

Suggested change
public Computer() {
saveRandomNumberWithGameStart();
}
public Computer() {
this.randomNumber = generate~~();
}

와 같이 수정하는게 더 가독성이 좋을거 같은데 어떻게 생각하시나요?


public List<Integer> getRandomNumber() {
return randomNumber;
}
Comment on lines +18 to +20
Copy link
Collaborator

@sjiwon sjiwon Jul 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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


public void saveRandomNumberWithGameStart() {
randomNumber = new ArrayList<>();
Integer digit;

while (checkLengthSmallThanThree()) {
digit = getRandomDigit();
if (!hasDuplicateDigitInRandomNumber(digit)) {
randomNumber.add(digit);
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

테스트를 위해서 열어두신거 같은데 굳이 얘를 열어둘 필요가 있을까요?
new Computer()로 생성에 대해서 테스트 하면 같이 테스트 되는거 아닌가요?


public boolean checkLengthSmallThanThree() {
if (randomNumber.size() < 3) {
return true;
}
return false;
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1줄로 줄일 수 있을거 같네요

그리고 3이란 숫자가 물론 이 미션의 명세를 보면 뭘 의미하는지 알겠지만 코드 레벨에서 무슨 숫자인지 파악이 가능할까요?

  • 이렇게 코드 레벨에서 의미를 나타내지 않는거를 매직넘버라고 합니다

추가적으로 미션에서는 숫자는 3개만 받도록 제한을 뒀지만 만약에 5개로 늘리면 이 메소드명까지 영향을 받는거 아닌가요?


public Integer getRandomDigit() {
return Randoms.pickNumberInRange(1, 9);
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
34 changes: 34 additions & 0 deletions src/main/java/baseball/model/User.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package baseball.model;


import java.util.List;

public class User {
public enum RestartStatus { RESTART, TERMINATE }

private List<Integer> number;
public RestartStatus restartStatus;

public void setNumber(List<Integer> number) {
validateNumber(number);
this.number = number;
}

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자리입니다.");
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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


public void restart() {
this.restartStatus = RestartStatus.RESTART;
}

public void terminate() {
this.restartStatus = RestartStatus.TERMINATE;
}
}
77 changes: 77 additions & 0 deletions src/test/java/baseball/controller/BaseballControllerTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package baseball.controller;

import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import java.io.ByteArrayInputStream;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.*;

class BaseballControllerTest {
@Test
@DisplayName("재시작 검증 테스트")
void ValidateRangeRestartStatusTest() {
// given
String restartStatus = "5";
BaseballController controller = new BaseballController();

// when


// then
assertThatThrownBy(() -> controller.validateRangeRestartStatus(restartStatus))
.isInstanceOf(IllegalArgumentException.class);

}

@Test
@DisplayName("재시작 검증 테스트")
void ValidateNotStringRestartStatusTest() {
// given
String restartStatus = "*";
BaseballController controller = new BaseballController();

// when


// then
assertThatThrownBy(() -> controller.validateNotStringRestartStatus(restartStatus))
.isInstanceOf(IllegalArgumentException.class);

}

@Test
@DisplayName("재시작 검증 테스트")
void validateNotDoubleRestartStatusTest() {
// given
String restartStatus = "1.2";
BaseballController controller = new BaseballController();

// when


// then
assertThatThrownBy(() -> controller.validateNotDoubleRestartStatus(restartStatus))
.isInstanceOf(IllegalArgumentException.class);
}

@Test
@DisplayName("사용자 입력에 따른 종료 테스트")
void BaseballControllerTest() {
// given
// 콘솔 입력 시뮬레이션
String input = "1";
ByteArrayInputStream inputStream = new ByteArrayInputStream(input.getBytes());
System.setIn(inputStream);

BaseballController controller = new BaseballController();

// when

// then
assertThat(controller.isTerminate()).isFalse();
}
}
68 changes: 68 additions & 0 deletions src/test/java/baseball/model/ComputerTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package baseball.model;

import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.*;

class ComputerTest {
Copy link
Collaborator

@sjiwon sjiwon Jul 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

https://shoulditestprivatemethods.com/

@Test
@DisplayName("컴퓨터의 랜덤 수를 저장할 때 중복된 숫자를 가지지 않는지 테스트 ")
void hasDigitTest() {
// given
Computer computer = new Computer();

// when
computer.randomNumber = new ArrayList<>(Arrays.asList(1, 2, 3));

// then
assertThat(computer.hasDuplicateDigitInRandomNumber(1)).isTrue();
}

@Test
@DisplayName("한자리 랜덤 수 생성 테스트")
void getRandomDigitTest() {
// given
Computer computer = new Computer();

// when
Integer randomDigit = computer.getRandomDigit();

// then
assertThat(randomDigit >= 1 && randomDigit <= 9).isTrue();
}

@Test
@DisplayName("랜덤수가 3자리 이하인지 체크 테스트")
void checkLengthTest() {
// given
Computer computer = new Computer();

// when
computer.randomNumber = new ArrayList<>(Arrays.asList(1, 2));

// then
assertThat(computer.checkLengthSmallThanThree()).isTrue();
}

@Test
@DisplayName("랜덤 수 생성 테스트")
void saveRandomNumberTest() {
// given
Computer computer = new Computer();

// when
computer.saveRandomNumberWithGameStart();
List<Integer> randomNumber = computer.randomNumber;

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
}
55 changes: 55 additions & 0 deletions src/test/java/baseball/model/UserTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package baseball.model;

import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import java.util.ArrayList;
import java.util.Arrays;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.junit.jupiter.api.Assertions.*;

class UserTest {
@Test
@DisplayName("3자리가 아닌 입력 숫자 검증 기능 테스트")
void validateNumberLengthTest() {
// given
User user = new User();

// when
ArrayList<Integer> number = new ArrayList<>(Arrays.asList(1, 2));
Copy link
Collaborator

@sjiwon sjiwon Jul 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

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


// then
assertThatThrownBy(() -> user.validateNumber(number))
.isInstanceOf(IllegalArgumentException.class);
}

@Test
@DisplayName("3자리가 아닌 입력 숫자 검증 기능 테스트")
void validateDigitLengthTest() {
// given
User user = new User();

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

// then
assertThatThrownBy(() -> user.validateNumber(number))
.isInstanceOf(IllegalArgumentException.class);
}

@Test
@DisplayName("RestartStatus 테스트")
void UserTest() {
// given
User user = new User();

// when
user.terminate();

// then
assertThat(user.restartStatus).isEqualTo(User.RestartStatus.RESTART);
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}