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

[3단계]- 자동차 경주 #5980

Merged
merged 12 commits into from
Mar 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 16 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,19 @@
* 모든 피드백을 완료하면 다음 단계를 도전하고 앞의 과정을 반복한다.

## 온라인 코드 리뷰 과정
* [텍스트와 이미지로 살펴보는 온라인 코드 리뷰 과정](https://github.com/next-step/nextstep-docs/tree/master/codereview)
* [텍스트와 이미지로 살펴보는 온라인 코드 리뷰 과정](https://github.com/next-step/nextstep-docs/tree/master/codereview)

### 제약사항
- [x] 모든 로직에 단위 테스트를 구현한다.
- UI 로직은 제외한다
- UI 로직은 클래스를 분리한다.
- [x] else 예약어는 쓰지 않는다.

### 요구사항
- [x] 주어진 횟수 동안 n대의 자동차는 전진 또는 멈출 수 있다.
- [x] 몇대의 자동차로 몇 번의 이동을 할 것인지 입력할 수 있다.
- 입력받는 API는 Scanner를 이용한다.
- [x] 0 ~ 9 사이의 random 값 중 4 이상일 때 전진한다.
- 랜덤값은 Random 클래스 이용한다.
- [x] 자동차의 상태를 콘솔에 출력한다.

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

import static racing.view.InputView.getCarCount;
import static racing.view.InputView.getMoveCount;

import racing.model.Cars;
import racing.service.RaceService;
import racing.service.RandomNumberGenerator;
import racing.view.OutputView;

public class RacingGameApplication {
private static final int RAND = 10;

public static void main(String[] args) {
int carCount = getCarCount();
int moveCount = getMoveCount();

RaceService raceService = new RaceService(new RandomNumberGenerator(RAND));
Cars cars = raceService.generateCar(carCount);

OutputView.printResultLabel();
for (int i = 0; i < moveCount; i++) {
raceService.moveCar(cars);
OutputView.printResult(cars);
}
}
}
20 changes: 20 additions & 0 deletions src/main/java/racing/model/Car.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package racing.model;

public class Car {
private static final int MOVE_CONDITION = 4;
private int position;

public Car() {
this.position = 0;
}

public void move(int number) {
if (number >= MOVE_CONDITION) {
this.position++;
}
}

public int getPosition() {
return position;
}
}
34 changes: 34 additions & 0 deletions src/main/java/racing/model/Cars.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package racing.model;

import java.util.ArrayList;
import java.util.List;
import racing.service.NumberGenerator;

public class Cars {
Copy link
Member

Choose a reason for hiding this comment

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

일급컬렉션 사용해 보셨네요 👍 👍

Copy link
Author

Choose a reason for hiding this comment

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

거진 처음 써본것 같은데,
일급 컬렉션 안에서 요구사항 구현에 필요한 핵심 로직들이 한대 모여있어
응집도가 되게 높게끔 설계가 가능하다는 장점이 있던 것 같습니다!

private final List<Car> cars;
private final NumberGenerator numberGenerator;

public Cars(List<Car> cars, NumberGenerator numberGenerator) {
this.cars = cars;
this.numberGenerator = numberGenerator;
}

public void moveAll() {
for (Car car : cars) {
car.move(numberGenerator.generateNumber());
}
}

public List<Integer> getPositions() {
List<Integer> positions = new ArrayList<>();
for (Car car : cars) {
positions.add(car.getPosition());
}
return positions;
}
Comment on lines +22 to +28
Copy link
Member

Choose a reason for hiding this comment

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

자동차들의 위치만 반환하네요 😮
출력과 관련된 지엽적인 기능으로 보여요

차라리 자동차 객체 자체를 반환하면 호출하는 측에서 보다 유연하게 활용할 수 있지 않을까요?
예를 들어 위치뿐만 아니라 다른 속성들도 필요할 경우 확장성이 높아질 것 같습니다!

Copy link
Author

@ParkSeryu ParkSeryu Mar 17, 2025

Choose a reason for hiding this comment

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

말씀해주신 대로 출력을 위해 반환하는게 맞긴 하지만,
자동차 객체 자체를 반환한다면 다음과 같은 점이 우려되었는데요.

  • 액세스 포인트가 많아진다.
    • car.move() 등의 메소드를 현재처럼 일급콜렉션을 통해서만 접근할수 있는게 아니라, 객체를 가져온 어디서든지 호출 할 수 있어 유지보수성이 낮아진다.

또한 이부분도 궁금했습니다!

  • 객체지향 생활 체조 원칙 규칙 9: 게터/세터/프로퍼티를 쓰지 않는다.
    • 이 원칙을 고려해 보았을때 요구사항 구현에 있어 꼭 필요하지 않은 데이터까지 제공하는게 괜찮을까? 라는 의문도 들었습니다.
    • 물론 해당 getPositions() 도 없이 하면 더 좋겠지만, 해당 함수 없이 구현할 방도가 생각이 안났습니다. 😂

해당부분 의견 및 질문드립니다~!

Copy link
Member

Choose a reason for hiding this comment

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

액세스 포인트가 많아진다.
car.move() 등의 메소드를 현재처럼 일급콜렉션을 통해서만 접근할수 있는게 아니라, 객체를 가져온 어디서든지 호출 할 수 있어 유지보수성이 낮아진다.

그렇다면 자동차 자체를 불변으로 관리해서 안전하게 내보내는 방법도 있지 않을까요?
어디서든지 수정해도 Car는 불변이므로 Cars가 관리하는 자동차들은 변경되지 않겠네요

그리고 요구사항이 변경되어 자동차들의 속도, 이름, 부가정보 같은 정보도 필요하다면
getXXX 같은 형식의 메소드들이 필드 수와 비례하게 생성될 가능성도 높아보입니다.

이런 부분은 어떻게 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

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

불변 좋은것같습니다!
다음 PR에 반영해서 해보겠습니다


public int size() {
return cars.size();
}

}
5 changes: 5 additions & 0 deletions src/main/java/racing/service/NumberGenerator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package racing.service;

public interface NumberGenerator {
int generateNumber();
}
30 changes: 30 additions & 0 deletions src/main/java/racing/service/RaceService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package racing.service;

import java.util.ArrayList;
import java.util.List;
import racing.model.Car;
import racing.model.Cars;

public class RaceService {
private final NumberGenerator numberGenerator;

public RaceService(NumberGenerator numberGenerator) {
this.numberGenerator = numberGenerator;
}

public Cars generateCar(int carCount) {
List<Car> carList = new ArrayList<>();

for (int i = 0; i < carCount; i++) {
Car car = new Car();
carList.add(car);
}

return new Cars(carList, numberGenerator);
}

public void moveCar(Cars cars) {
cars.moveAll();
}
Comment on lines +15 to +28
Copy link
Member

Choose a reason for hiding this comment

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

// 1번
var cars = service.generateCar(5);
cars.moveAll();

// 2번
var cars = service.generateCar(5);
service.moveCar(cars);

1번과 2번을 비교해보면 서비스가 꼭 필요한 상황처럼 보이지는 않네요

generateCar(5)가 단순히 Car 객체 리스트를 생성하는 역할만 한다면
이 로직은 서비스가 아니라 만들어보신 Cars 일급 컬렉션 객체가 담당하는 것이 더 적절할 수 있겠네요 😄


}
16 changes: 16 additions & 0 deletions src/main/java/racing/service/RandomNumberGenerator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package racing.service;

import java.util.Random;

public class RandomNumberGenerator implements NumberGenerator {
private final int rand;

public RandomNumberGenerator(int rand) {
this.rand = rand;
}

@Override
public int generateNumber() {
return new Random().nextInt(rand);
}
}
18 changes: 18 additions & 0 deletions src/main/java/racing/view/InputView.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package racing.view;

import java.util.Scanner;

public class InputView {
private static final Scanner scanner = new Scanner(System.in);

public static int getCarCount() {
System.out.println("자동차 대수는 몇 대 인가요?");
return scanner.nextInt();
}

public static int getMoveCount() {
System.out.println("시도할 횟수는 몇 대 인가요?");
return scanner.nextInt();
}

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

import java.util.List;
import racing.model.Cars;

public class OutputView {

public static final String RESULT_LABEL = "실행 결과";

public static void printResultLabel() {
System.out.println(RESULT_LABEL);
}

public static void printResult(Cars cars) {
List<Integer> positions = cars.getPositions();
Comment on lines +10 to +15
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
public static void printResultLabel() {
System.out.println(RESULT_LABEL);
}
public static void printResult(Cars cars) {
List<Integer> positions = cars.getPositions();
public static void printResult(Cars cars) {
printResultLabel();
List<Integer> positions = cars.getPositions();
...
}
private static void printResultLabel() { ... }

노출되는 메소드는 하나로도 충분할 것 같아요!

Copy link
Member

Choose a reason for hiding this comment

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

@ParkSeryu
생각해보니 이 부분은 제가 잘못 리뷰드린 것 같네요
해당 리뷰는 넘어가주세요~ 🙏

for (Integer position : positions) {
System.out.println("-".repeat(position));
}
System.out.println();
}

}
16 changes: 16 additions & 0 deletions src/test/java/racing/fake/FakeNumberGenerator.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package racing.fake;

import racing.service.NumberGenerator;

public class FakeNumberGenerator implements NumberGenerator {
private final int num;

public FakeNumberGenerator(int num) {
this.num = num;
}

@Override
public int generateNumber() {
return num;
}
}
38 changes: 38 additions & 0 deletions src/test/java/racing/model/CarTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
package racing.model;

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

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;

class CarTest {
private Car car;

@BeforeEach
void setUp() {
this.car = new Car();
}

@DisplayName("자동차의 위치를 한 칸 증가시킬 수 있다.")
@ParameterizedTest
@ValueSource(ints = {4, 9})
void carMoveTest(int argument) {
car.move(argument);

assertThat(car.getPosition()).isEqualTo(1);
}

@DisplayName("파라미터로 받는 숫자가 4 미만인 경우 자동차는 멈춘다.")
@ParameterizedTest
@ValueSource(ints = {0, 3})
void carNotMoveTest(int argument) {
car.move(argument);

assertThat(car.getPosition()).isEqualTo(0);
}

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


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

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import racing.fake.FakeNumberGenerator;

class CarsTest {
private static final int MOVE_NUMBER = 4;
private static final int STOP_NUMBER = 3;
Comment on lines +15 to +16
Copy link
Member

Choose a reason for hiding this comment

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

👍 👍

private Cars cars;

@DisplayName("리스트 안의 모든 자동차를 움직일 수 있다.")
@Test
void moveAllTest() {
// given
cars = new Cars(makeDummyCars(3), new FakeNumberGenerator(MOVE_NUMBER));

// when
cars.moveAll();

// then
assertThat(cars.getPositions()).isEqualTo(Arrays.asList(1, 1, 1));
}

@DisplayName("리스트 안의 모든 자동차들의 현재 위치를 알 수 있다.")
@Test
void getPositionsTest() {
// given
cars = new Cars(makeDummyCars(7), new FakeNumberGenerator(STOP_NUMBER));

// when
List<Integer> positions = cars.getPositions();

// then
Assertions.assertAll(() -> assertThat(positions).isNotNull(),
() -> assertThat(positions).containsExactly(0, 0, 0, 0, 0, 0, 0));
}

@DisplayName("일급 콜렉션 안의 리스트의 총 개수를 알 수 있다.")
@Test
void sizeTest() {
// given
cars = new Cars(makeDummyCars(5), new FakeNumberGenerator(MOVE_NUMBER));

// when
// then
assertThat(cars.size()).isEqualTo(5);
}

private List<Car> makeDummyCars(int carCount) {
List<Car> carList = new ArrayList<>();
for (int i = 0; i < carCount; i++) {
carList.add(new Car());
}
return carList;
}
}
45 changes: 45 additions & 0 deletions src/test/java/racing/service/RaceServiceTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
package racing.service;

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

import java.util.Arrays;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;
import racing.fake.FakeNumberGenerator;
import racing.model.Cars;

class RaceServiceTest {
private RaceService raceService;

@BeforeEach
void setUp() {
raceService = new RaceService(new FakeNumberGenerator(4));
}

@DisplayName("입력 받은 n대의 자동차를 생성한다.")
@Test
void generateCarsTest() {
// given
int carCount = 3;

// when
Cars cars = raceService.generateCar(carCount);

// then
assertThat(cars.size()).isEqualTo(carCount);
}

@DisplayName("자동차를 움직일 수 있다.")
@Test
void moveCarTest() {
// given
Cars cars = raceService.generateCar(3);

// when
raceService.moveCar(cars);

// then
assertThat(cars.getPositions()).isEqualTo(Arrays.asList(1, 1, 1));
}
}