Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[자동차 경주 1단계] 카피 pr 요청 드립니다 #687

Merged
merged 29 commits into from
Feb 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6a49f58
docs : 기능 구현 목록 작성
Feb 14, 2024
a441e90
docs : 기능 구현 목록 추가
Feb 14, 2024
0ab8f8d
feat : 자동차 이름 입력 기능 구현
Feb 14, 2024
492af6c
feat : 자동차 게임 시도할 회수 입력 기능 구현
Feb 14, 2024
a180b01
feat : 랜덤 조건에 따라 자동차를 움직이는 기능 구현
Feb 14, 2024
c71801d
feat : 자동차 게임 실행 결과 출력하는 기능 구현
Feb 14, 2024
79f811c
게임 완료 후 우승자 출력 기능 구현
Feb 14, 2024
8265649
refactor : 사용자 입력 전, 입력 메세지 출력 기능 추가
Feb 15, 2024
d593791
feat: 자동차 게임 실행 기능 구현
Feb 15, 2024
a12bc57
refactor : moveCar 메서드에 작동 조건에 대한 주석 추가
Feb 15, 2024
fccb4fc
refactor : InputView 클래스의 getCarName() 메서드 분리 및 에러 메시지 상수 처리
Feb 15, 2024
0725d76
refactor : InputView 클래스의 getTryCount() 메서드 분리 및 에러 메시지 상수 처리
Feb 15, 2024
0256e7c
refactor : OutputView 클래스의 에러 메시지 상수 처리
Feb 15, 2024
f5681ac
refactor : GameManger 클래스 생성 및 게임 실행 기능 분리
Feb 15, 2024
a2bb927
refactor : package 분리
Feb 15, 2024
93d4553
refactor : 출력 메세지 추가 및 format 변경
Feb 15, 2024
ff58cf0
refactor : 랜덤 숫자 생성 기능 외부 클래스로 분리
Feb 15, 2024
d142726
test : moveCar() 조건에 따라 전진 혹은 정지하는지 테스트
Feb 15, 2024
e91b81a
style : 코드 스타일 정리
Feb 15, 2024
f94ac96
refactor : InputView가 OutputView를 의존하지 않도록 수정
tkdgur0906 Feb 17, 2024
537396b
refactor : 랜덤한 수를 생성하는 기능을 Car 내부에서 분리하고 파라미터로 랜덤한 수를 받도록 수정
tkdgur0906 Feb 17, 2024
954493f
refactor : RandomGenerator 클래스 삭제
tkdgur0906 Feb 17, 2024
3d6fa4b
refactor : mock을 사용하지 않고 Car가 움직이는지 검증하는 테스트 작성
tkdgur0906 Feb 17, 2024
7037759
refactor : GameManager의 makeNewCar 메서드가 리스트를 반환하도록 변경하여 클래스의 인스턴스 변수 …
tkdgur0906 Feb 17, 2024
69522c6
refactor : 매직넘버를 상수로 분리
tkdgur0906 Feb 17, 2024
1cd6a33
refactor : 메서드가 try 내부에서 얼리리턴하도록 수정
tkdgur0906 Feb 17, 2024
beb56cb
refactor : 파일 끝 개행문자 추가
tkdgur0906 Feb 17, 2024
e7703ab
test : 자동차 이름 조건을 테스트하는 코드 작성
tkdgur0906 Feb 17, 2024
39abf8b
refactor : 테스트 코드 매직넘버 상수 분리
tkdgur0906 Feb 17, 2024
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
22 changes: 19 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,22 @@

자동차 경주 미션 저장소

## 우아한테크코스 코드리뷰

- [온라인 코드 리뷰 과정](https://github.com/woowacourse/woowacourse-docs/blob/master/maincourse/README.md)
## 기능 구현 목록
- [X] 자동차 이름 입력 받는다.
Comment on lines +5 to +6

Choose a reason for hiding this comment

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

체크리스트 작성 👍🏿

- 사용자 입력을 받기 전, 입력 메세지를 출력한다.
- 이름의 길이는 1자 이상, 5자 이하이다.
- 쉼표를 기준으로 구분한다.
- 공백이 포함된 입력은 허용하지 않는다.
- 중복된 이름을 입력할 수 없다.
- 잘못된 입력 시 재입력을 받는다.
- [x] 시도할 회수를 입력 받는다.
- 사용자 입력을 받기 전, 입력 메세지를 출력한다.
- 자연수만 입력할 수 있다.
- 잘못된 입력 시 재입력을 받는다.
- [x] 조건에 따라 자동차를 움직인다.
- 0-9 사이의 random 값을 구한 후 4 이상일 경우 전진하고 3이하의 값이면 멈춘다.
- 자동차의 이동거리는 음수가 될 수 없다.
- [x] 자동차의 이름과 이동거리를 각 실행 결과로 출력한다.
- [x] 게임 완료 후 누가 우승했는지를 출력한다.
- 우승자는 한 명 이상일 수 있다.

41 changes: 41 additions & 0 deletions src/main/java/racingcar/GameManager.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package racingcar;

import racingcar.domain.Car;
import racingcar.view.InputView;
import racingcar.view.OutputView;

import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ThreadLocalRandom;

public class GameManager {

private static final int MAX_RANDOM_NUMBER = 9;
OutputView outputView = new OutputView();
InputView inputView = new InputView();

public void run() {
outputView.printCarNameInputMessage();
List<String> carNames = inputView.getCarName();
List<Car> cars = makeNewCars(carNames);
outputView.printTryCountInputMessage();
int tryCount = inputView.getTryCount();
for (int i = 0; i < tryCount; i++) {
moveCars(cars);
outputView.printTryResult(cars);
}
outputView.printWinners(cars);
}

private void moveCars(List<Car> cars) {
for (Car car : cars) {
car.moveCar(ThreadLocalRandom.current().nextInt(MAX_RANDOM_NUMBER + 1));

Choose a reason for hiding this comment

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

ThreadLocalRandom 을 사용하신 이유가 있나요?

Choose a reason for hiding this comment

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

해당 클래스는 Math.random()이나 Randdom 클래스와 어떤 차별점이 있고 뭘 위해 사용하셨나요?

Copy link
Member Author

Choose a reason for hiding this comment

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

Random은 seed를 설정하지 않으면 컴퓨터의 현재 시간을 seed로 사용합니다.
만약 멀티스레드 환경에서 동일한 시간에 여러 요청이 들어온다면 중복된 난수를 생성하지 않기 위해 스레드끼리 경합을 하게 됩니다. 이는 멀티 스레드 환경에서 성능 저하를 일으킬 수 있기 때문에 ThreadLocalRandom을 사용하였습니다.

ThreadLocalRandom은 Random처럼 하나의 인스턴스를 전역적으로 사용하는 것이 아닌, 스레드 별로 각각의 인스턴스를 할당하게 됩니다. 따라서 Random에서 발생할 수 있는 성능 저하를 해결 할 수 있다는 차별점이 있습니다.

현재 미션에서는 멀티 스레드 환경을 고려하지 않아도 무방하지만, ThreadLocalRandom 대신 Random을 사용할 이유가 없다고 생각하여 사용하였습니다!

}
}

private List<Car> makeNewCars(List<String> carNames) {
return carNames.stream()
.map(carName -> new Car(carName))
.toList();
Comment on lines +37 to +39

Choose a reason for hiding this comment

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

Suggested change
return carNames.stream()
.map(carName -> new Car(carName))
.toList();
return carNames.stream()
.map(Car::new)
.toList();

}
}
8 changes: 8 additions & 0 deletions src/main/java/racingcar/RacingcarApplication.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
package racingcar;

public class RacingcarApplication {
public static void main(String[] args) {
GameManager gameManager = new GameManager();
gameManager.run();
}
}
36 changes: 36 additions & 0 deletions src/main/java/racingcar/domain/Car.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package racingcar.domain;

public class Car {

private static final int ZERO = 0;
private static final int MINIMUM_MOVEMENT_CONDITION = 4;
private final static int MAXIMUM_NAME_LENGTH = 5;
private final static String MESSAGE_LENGTH_OF_CAR_NAME = "자동차 이름은 1자 이상 5자 이하여야 합니다.";
private String name;
private int distance;

public Car(String name) {
if(name.length() > MAXIMUM_NAME_LENGTH) {
throw new IllegalArgumentException(MESSAGE_LENGTH_OF_CAR_NAME);
}
this.name = name;
this.distance = 0;
}

public String getName() {
return name;
}

public int getDistance() {
return distance;
}

public void moveCar(int number) {
if (distance < ZERO) {
throw new IllegalArgumentException("이동 거리는 음수가 될 수 없습니다.");
}
Comment on lines +29 to +31

Choose a reason for hiding this comment

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

메소드가 시작할 때 갑자기 차의 상태가 무결한 지를 검증하고 있네요. 해당 메소드로 인해 변하는 부분에 대해서만 검증하면 어떨까요?
distance가 음수인 차는 존재하면 안될 것 같고, distance가 음수가 세팅될 수 있는 코드 부분에서 검증하는게 좋을 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

메서드를 만들 때 많은 것을 검증하고자 하여서 불필요한 검증이 포함된 것 같습니다😅
distance가 음수가 될 코드 부분이 없는 것 같아 해당 코드를 삭제하였습니다!

if (number >= MINIMUM_MOVEMENT_CONDITION) {
distance++;
}
Comment on lines +32 to +34

Choose a reason for hiding this comment

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

특정 숫자 이상이면 차가 전진한다 라는 규칙은 차가 아는게 좋을까요? 고민해보시면 좋을 것 같아요!

Copy link
Member Author

@tkdgur0906 tkdgur0906 Feb 18, 2024

Choose a reason for hiding this comment

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

자동차가 전진하는 조건에 대한 규칙을 자동차가 알 필요는 없는 것 같습니다! 자동차는 전진하는 것에 책임만 가지고 메서드를 호출하는 부분에서 특정 숫자인지 검증하는 부분을 추가하였습니다!

}
}
84 changes: 84 additions & 0 deletions src/main/java/racingcar/view/InputView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
package racingcar.view;

import java.util.List;
import java.util.Scanner;

public class InputView {

private final static String MESSAGE_NO_DUPLICATE_CAR_NAMES = "중복된 자동차 이름은 사용할 수 없습니다.";
private final static String MESSAGE_LENGTH_OF_CAR_NAME = "자동차 이름은 1자 이상 5자 이하여야 합니다.";
private final static String MESSAGE_NO_SPACE = "중복된 자동차 이름은 사용할 수 없습니다.";
private final static String MESSAGE_ONLY_NUMBER = "시도할 회수는 숫자여야만 가능합니다.";
private final static String MESSAGE_ONLY_NATURAL_NUMBER = "시도할 회수는 자연수를 입력해 주세요.";

Choose a reason for hiding this comment

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

이 검증은 View에서만 해도 괜찮을까요? View는 코어 로직보다 쉽게 교체될 수 있고, 만약 다른 형태의 View로 교체된다면, 검증 로직이 날라가겠네요..🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

View에서 직접 검증을 하지 않고 별도의 InputValidator 클래스를 생성하여 InputView의 검증을 하도록 변경하였습니다!
리팩토링을 하면서 에러 메시지도 별도의 클래스가 관리하는 것이 좋을 것 같아 enum으로 관리하도록 하였습니다!

private final static int MAXIMUM_NAME_LENGTH = 5;
private final Scanner scanner = new Scanner(System.in);


public List<String> getCarName() {
String input = scanner.nextLine();
List<String> splitCarNames;
try {
validateNoSpace(input);
splitCarNames = getSplitCarNames(input);
validateLengthOfCarNames(splitCarNames);
validateNoDuplicatedCarNames(splitCarNames);
} catch (IllegalArgumentException e) {
System.out.println(e.getMessage());
return getCarName();
}
return splitCarNames;
}

public int getTryCount() {
String input = scanner.nextLine();
int tryCount;
try {
tryCount = Integer.parseInt(input);
validateNaturalNumber(tryCount);
return tryCount;
} catch (NumberFormatException e) {
System.out.println(MESSAGE_ONLY_NUMBER);
return getTryCount();
} catch (IllegalArgumentException e) {
System.out.println(e.getMessage());
return getTryCount();
}
}

private void validateNaturalNumber(int tryCount) {
if (tryCount <= 0) {
throw new IllegalArgumentException(MESSAGE_ONLY_NATURAL_NUMBER);
}
}

private void validateNoDuplicatedCarNames(List<String> splitCarNames) {
long distinctCarCount = splitCarNames.stream()
.distinct()
.count();
if (distinctCarCount != splitCarNames.size()) {
throw new IllegalArgumentException(MESSAGE_NO_DUPLICATE_CAR_NAMES);
}
}

private void validateLengthOfCarNames(List<String> splitCarNames) {
for (String carName : splitCarNames) {
validateLengthOfCarName(carName);
}
}

private void validateLengthOfCarName(String carName) {
if (carName.isEmpty() || carName.length() > MAXIMUM_NAME_LENGTH) {
throw new IllegalArgumentException(MESSAGE_LENGTH_OF_CAR_NAME);
}
}

private List<String> getSplitCarNames(String input) {
return List.of(input.split(","));
}

private void validateNoSpace(String input) {
if (input.contains(" ")) {
throw new IllegalArgumentException(MESSAGE_NO_SPACE);
}
}
}
42 changes: 42 additions & 0 deletions src/main/java/racingcar/view/OutputView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package racingcar.view;

import racingcar.domain.Car;

import java.util.List;

public class OutputView {

private static final String MESSAGE_NOT_EXIST_CAR = "생성된 자동차가 없습니다.";

public void printTryResult(List<Car> cars) {
System.out.println();
System.out.println("실행 결과");
for (Car car : cars) {
System.out.println(car.getName() + " : " + "-".repeat(car.getDistance()));
}
}

public void printWinners(List<Car> cars) {
int longestDistance = cars.stream()
.mapToInt(Car::getDistance)
.max()
.orElseThrow(() -> new IllegalArgumentException(MESSAGE_NOT_EXIST_CAR));

String winnerNames = String.join(", ",
cars.stream()
.filter(car -> car.getDistance() == longestDistance)
.map(Car::getName)
.toList()
);

System.out.println(winnerNames + "가 최종 우승했습니다.");
}
Comment on lines +19 to +33

Choose a reason for hiding this comment

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

이 부분은 승자를 찾는 비즈니스 로직이 View에 존재하는 걸로 보이네요!

Copy link
Member Author

@tkdgur0906 tkdgur0906 Feb 18, 2024

Choose a reason for hiding this comment

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

List<Car>에 대한 일급 컬렉션을 생성하여 Car들의 최장거리 찾는 로직과 승자를 찾는 로직을 분리하였습니다.
해당 로직은 View가 아닌 Cars가 가지고 있어야 한다고 생각하여 변경하였습니다!!


public void printCarNameInputMessage() {
System.out.println("경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분).");
}

public void printTryCountInputMessage() {
System.out.println("시도할 회수는 몇회인가요?");
}
}
49 changes: 49 additions & 0 deletions src/test/java/racingcar/domain/CarTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package racingcar.domain;

import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import static org.assertj.core.api.Assertions.*;

class CarTest {

private static final int MOVE_CONDITION = 5;
private static final int STOP_CONDITION = 3;


@Test
@DisplayName("랜덤_숫자가_4_이상인_경우_전진")
void moveCarTest() {
Car car = new Car("capy");
int initDistance = car.getDistance();
car.moveCar(MOVE_CONDITION);
int movedDistance = car.getDistance();

assertThat(movedDistance).isEqualTo(initDistance + 1);
}

@Test
@DisplayName("랜덤_숫자가_3_이하인_경우_정지")
void stopCarTest() {
Car car = new Car("capy");
int initDistance = car.getDistance();
car.moveCar(STOP_CONDITION);
int movedDistance = car.getDistance();

assertThat(movedDistance).isEqualTo(initDistance);
}

@Test
@DisplayName("자동차_이름이_5자_이하인_경우_통과")
void validCarName() {
assertThatCode(() -> new Car("capy"))
.doesNotThrowAnyException();
}

@Test
@DisplayName("자동차_이름이_5자_초과인_경우_에러_발생")
void invalidCarName() {
assertThatThrownBy(() -> new Car("capyeeee"))
.isInstanceOf(IllegalArgumentException.class);
}
}