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

ParkSeryu · 2025-03-16T10:44:41Z

안녕하세요!
3단계 미션 리뷰요청드립니다.

3단계 미션 진행 중
Cars (일급 콜렉션) 에 대한 경험이 적어서 그런지,
일급 콜렉션에 대한 테스트를 어떻게 하면 좋을지에 대한 고민이 있었는데요.

일급 콜렉션에서만 제공하는 메소드도 존재하지만,
moveAll()처럼 Car 객체의 하위 move의 동작을 한 번 더 래핑하는 메서드의
모든 케이스를 검증 할 필요가 있는지가 궁금했습니다!
(moveAll 또한 이동, 정지 등의 케이스까지 테스트코드로 검증해야 하는지)

그래서 준혁님의 생각이 궁금해서 적어봅니다.
이번 미션도 잘 부탁드립니다!

nooose · 2025-03-16T15:01:17Z

Cars (일급 콜렉션) 에 대한 경험이 적어서 그런지,
일급 콜렉션에 대한 테스트를 어떻게 하면 좋을지에 대한 고민이 있었는데요.
일급 콜렉션에서만 제공하는 메소드도 존재하지만,
moveAll()처럼 Car 객체의 하위 move의 동작을 한 번 더 래핑하는 메서드의
모든 케이스를 검증 할 필요가 있는지가 궁금했습니다!
(moveAll 또한 이동, 정지 등의 케이스까지 테스트코드로 검증해야 하는지)

모든 로직에 단위 테스트를 구현한다. 단, UI(System.out, System.in) 로직은 제외
핵심 로직을 구현하는 코드와 UI를 담당하는 로직을 구분한다.

우선, 요구사항이므로 테스트 케이스를 작성하는 것이 좋겠습니다.

그런데 고민하시는 부분이 "단순히 래핑하는 수준인데도 테스트 케이스를 작성해야 할까?" 인 것 같네요
이럴 때는 역으로 질문을 던져보면 좋겠습니다

한번 더 래핑이라면, 굳이 일급 컬렉션으로 감쌀 필요가 있었을까?
일급 컬렉션으로 래핑함으로써 얻는 장점이 명확하다면, 이는 핵심 로직에 해당하므로 테스트 케이스를 작성하는 것이 맞다고 봅니다.
반대로, 별다른 장점이 느껴지지 않는다면 이 상황에서는 일급 컬렉션을 과하게 사용한 것이 아닐까 고민해볼 필요가 있겠네요!

준형님의 의견은 어떤가요?

nooose

준형님 안녕하세요

패키지도 잘 나눠져있고, 테스트하기 어려운 부분도 잘 해내셨네요 👍
조금만 집짚고 가면 좋을 것 같아 피드백 남겼어요
확인해 보시고 재요청 부탁드립니다!

nooose · 2025-03-16T14:29:15Z

src/main/java/racing/service/NumberGenerator.java

+public interface NumberGenerator {
+    int generateRandomNumber(int num);
+}


랜덤을 다룰 수 있는 인터페이스네요 👍

그런데 좀 이해하기 어려운 부분이 있어요
generateRandomNumber라면 랜덤한 번호를 생성하는데
인터페이스만 봤을 때 num 파라미터는 무엇을 위한 값인지? 하는 생각이 드네요

만약 필요하다면 num 대신 다른 네이밍이 필요할 것 같고, 인터페이스 함수에 javadoc으로 추가 설명을 작성한다면 더 좋겠네요

Suggested change

public interface NumberGenerator {

int generateRandomNumber(int num);

}

// 예시이니 참고만 해주세요

public interface NumberGenerator {

/**

* 랜덤번호를 생성해서 반환한다.

* @param num 설명

*/

int generateRandomNumber(int num);

}

행위에 집중하다보니 간과한 부분인 것 같습니다!
(다시 보니까 왜이렇게 했지.. 싶네요. 역시 잘 안될 땐 안붙잡고 있는게 상책인것 같습니다. ㅎㅎ,,)

Suggested change

public interface NumberGenerator {

int generateRandomNumber(int num);

}

public interface NumberGenerator {

int generateNumber();

}

// 1. RandomNumberGenerator

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);

}

// 2. FakeNumberGenerator

public class FakeNumberGenerator implements NumberGenerator {

private final int num;

public FakeNumberGenerator(int num) {

this.num = num;

}

@Override

public int generateNumber() {

return num;

}

이렇게 파라미터를 안쓰고도 더 자연스럽게 할 수 있을 것 같아요!

nooose · 2025-03-16T14:30:15Z

src/main/java/racing/model/Car.java

+    }
+
+    public void move(int number) {
+        if (number >= 4) {


4 값을 상수로 치환하면 더 좋겠네요 😄

nooose · 2025-03-16T14:30:33Z

src/main/java/racing/model/Cars.java

+import java.util.List;
+import racing.service.NumberGenerator;
+
+public class Cars {


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

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

nooose · 2025-03-16T14:33:46Z

src/main/java/racing/service/RandomNumberGenerator.java

+public class RandomNumberGenerator implements NumberGenerator {
+
+    @Override
+    public int generateRandomNumber(int rand) {
+        return new Random().nextInt(rand);
+    }
+}


범위가 동적으로 변하는게 아니라면
new RandomNumberGenerator(10) 같이 최대값을 정의할 수 있게 만들어 보는 방법도 있겠네요

#5980 (comment)
와 같은 맥락이라고 생각합니다

한대 묶어서 반영해볼게요~

nooose · 2025-03-16T14:37:49Z

src/main/java/racing/model/Cars.java

+    public List<Integer> getPositions() {
+        List<Integer> positions = new ArrayList<>();
+        for (Car car : cars) {
+            positions.add(car.getPosition());
+        }
+        return positions;
+    }


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

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

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

액세스 포인트가 많아진다.

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

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

객체지향 생활 체조 원칙 규칙 9: 게터/세터/프로퍼티를 쓰지 않는다.

이 원칙을 고려해 보았을때 요구사항 구현에 있어 꼭 필요하지 않은 데이터까지 제공하는게 괜찮을까? 라는 의문도 들었습니다.

물론 해당 getPositions() 도 없이 하면 더 좋겠지만, 해당 함수 없이 구현할 방도가 생각이 안났습니다. 😂

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

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

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

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

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

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

nooose · 2025-03-16T14:43:26Z

src/main/java/racing/service/RaceService.java

+
+public class RaceService {
+
+    public Cars generateCar(int carCount, NumberGenerator numberGenerator) {


RaceService가 NumberGenerator를 의존하고 있네요

new RaceService(numberGenerator) 처럼 의존 주입을 할 수 있었을 것 같은데
다른 이유가 있었을까요??

미처 생각을 못한 부분이었던 것 같습니다!
NumberGenerator가 파라미터로 전달되어야 Testability가 높아질거라는 생각에 매몰된것 같네요 😢

말씀주신대로 생성자로 주입하는게 더 객체지향스러운 것 같습니다! 😂

nooose · 2025-03-16T14:49:15Z

src/main/java/racing/RacingGameApplication.java

+        for (int i = 0; i < moveCount; i++) {
+            raceService.moveCar(cars);
+            appendCarPosition(cars, gameStatus);
+            gameStatus.append("\n");
+        }
+
+        // 3. OUTPUT
+        OutputView outputView = new OutputView();
+        outputView.printResult(gameStatus.toString());


gameStatus는 결국 문자열이네요
이 문자열이 무엇인지 파악하기가 좀 어려울 듯 합니다.

문자열 타입에 의존하지 않고 적절한 타입을 이용해 로직을 작성해 보는 것이 어떨까요?

다양한 방법이 있을 것 같습니다.

반복마다 자동차들의 상태를 출력

OutputView outputView = new OutputView(); for (int i = 0; i < moveCount; i++) { raceService.moveCar(cars); outputView.printResult(cars); }

반복을 돌리고 자동차들의 상태를 출력

var 결과 = raceService.moveCar(cars, moveCount); outputView.printResult(결과);

말씀주신대로 여러 방법이 있을 것 같습니다.
moveCar가 상태를 반환하는거는 조금 넌센스 한것 같아서 1번 케이스로 진행해 보았습니다! aa28913

확실히 캡슐화가 되고, 깔끔해지는것같네요!
근데 이렇게 View쪽을 다루는게 오랜만이라 말씀주신 의도대로 잘 했는지는 모르겠네요 😂

nooose · 2025-03-16T14:50:30Z

src/main/java/racing/view/OutputView.java

+package racing.view;
+
+public class OutputView {
+    public void printResult(String gameStatus) {


InputView, OutputView 모두 상태를 가지고 있지 않으니 static 선언도 괜찮아보이네요 😄

ParkSeryu · 2025-03-17T09:42:44Z

우선, 요구사항이므로 테스트 케이스를 작성하는 것이 좋겠습니다.

그런데 고민하시는 부분이 "단순히 래핑하는 수준인데도 테스트 케이스를 작성해야 할까?" 인 것 같네요 이럴 때는 역으로 질문을 던져보면 좋겠습니다

한번 더 래핑이라면, 굳이 일급 컬렉션으로 감쌀 필요가 있었을까? 일급 컬렉션으로 래핑함으로써 얻는 장점이 명확하다면, 이는 핵심 로직에 해당하므로 테스트 케이스를 작성하는 것이 맞다고 봅니다. 반대로, 별다른 장점이 느껴지지 않는다면 이 상황에서는 일급 컬렉션을 과하게 사용한 것이 아닐까 고민해볼 필요가 있겠네요!

준형님의 의견은 어떤가요?

확실히 #5980 (comment) 처럼
많은 장점은 느꼈습니다. 불필요한 반복문들을 많이 제거하고, 응집도가 높아 객체가 하는 일을 쉽게 파악하기 좋은것 같아요.

또한 피드백과 말씀해주신대로 구조를 조금 더 개선해보니, 래핑하더라도 래핑클래스만의 검증요소를 찾을 수 있어 보이네요!

nooose

안녕하세요 준형님!
고민하면 좋을 내용 코멘트로 남겼어요
전반적으로 잘 작성해 주셨고 다음 단계로 넘어가도 괜찮을 것 같아 머지하겠습니다
수고하셨습니다!

nooose · 2025-03-17T11:09:42Z

src/main/java/racing/model/Cars.java

+    public List<Integer> getPositions() {
+        List<Integer> positions = new ArrayList<>();
+        for (Car car : cars) {
+            positions.add(car.getPosition());
+        }
+        return positions;
+    }


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

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

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

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

nooose · 2025-03-17T11:15:22Z

src/main/java/racing/service/RaceService.java

+    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();
+    }


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

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

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

nooose · 2025-03-17T11:17:28Z

src/test/java/racing/model/CarsTest.java

+    private static final int MOVE_NUMBER = 4;
+    private static final int STOP_NUMBER = 3;


nooose · 2025-03-17T11:25:29Z

src/main/java/racing/view/OutputView.java

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


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() { ... }

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

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

ParkSeryu added 7 commits March 16, 2025 05:13

docs: 자동차 경주게임 요구사항 정리

3fd0c24

feat: 자동차 입력 기능 구현

644e7b4

feat: 주어진 횟수동안 자동차 이동 기능 구현

08bf310

feat: random 값을 받아 4 미만인 경우 이동 하지 않도록 변경

495a2b7

feat: 자동차의 상태를 콘솔에 출력한다.

fb8dddd

docs: 제약사항 체크

87795ca

test: 제약사항 체크하면서 빠진 테스트 추가

0230054

nooose requested changes Mar 16, 2025

View reviewed changes

ParkSeryu added 4 commits March 17, 2025 17:56

refactor: 인터페이스 관련 구조 변경 진행

a50aade

refactor: 매직넘버 상수로 변경

6996a1a

refactor: RaceService가 NumberGenerator를 생성자로 DI 받도록 변경

65ebc92

refactor: InputView, OutputView Static으로 변경

554b4db

refactor: 반복마다 자동차들의 상태를 출력하도록 변경

aa28913

nooose approved these changes Mar 17, 2025

View reviewed changes

nooose merged commit c135369 into next-step:parkseryu Mar 17, 2025

ParkSeryu deleted the step3 branch March 18, 2025 02:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

ParkSeryu commented Mar 16, 2025 •

edited

Loading

nooose commented Mar 16, 2025 •

edited

Loading

nooose left a comment •

edited

Loading

nooose Mar 16, 2025

ParkSeryu Mar 17, 2025 •

edited

Loading

nooose Mar 16, 2025

nooose Mar 16, 2025

ParkSeryu Mar 17, 2025

nooose Mar 16, 2025

ParkSeryu Mar 17, 2025

nooose Mar 16, 2025

ParkSeryu Mar 17, 2025 •

edited

Loading

nooose Mar 17, 2025

ParkSeryu Mar 17, 2025

nooose Mar 16, 2025

ParkSeryu Mar 17, 2025 •

edited

Loading

nooose Mar 16, 2025

ParkSeryu Mar 17, 2025

nooose Mar 16, 2025

ParkSeryu commented Mar 17, 2025

nooose left a comment

nooose Mar 17, 2025

nooose Mar 17, 2025

nooose Mar 17, 2025

nooose Mar 17, 2025

nooose Mar 17, 2025

-public interface NumberGenerator {
-    int generateRandomNumber(int num);
-}
+public interface NumberGenerator {
+    int generateNumber();
+}
+// 1. RandomNumberGenerator
+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);
+    }
+// 2. FakeNumberGenerator
+  public class FakeNumberGenerator implements NumberGenerator {
+    private final int num;
+    public FakeNumberGenerator(int num) {
+        this.num = num;
+    }
+    @Override
+    public int generateNumber() {
+        return num;
+    }


		public class RaceService {

		public Cars generateCar(int carCount, NumberGenerator numberGenerator) {

		private static final int MOVE_NUMBER = 4;
		private static final int STOP_NUMBER = 3;

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

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

Conversation

ParkSeryu commented Mar 16, 2025 • edited Loading

nooose commented Mar 16, 2025 • edited Loading

nooose left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ParkSeryu Mar 17, 2025 • edited Loading

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

ParkSeryu Mar 17, 2025 • edited Loading

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

ParkSeryu Mar 17, 2025 • edited Loading

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

ParkSeryu commented Mar 17, 2025

nooose 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

ParkSeryu commented Mar 16, 2025 •

edited

Loading

nooose commented Mar 16, 2025 •

edited

Loading

nooose left a comment •

edited

Loading

ParkSeryu Mar 17, 2025 •

edited

Loading

ParkSeryu Mar 17, 2025 •

edited

Loading

ParkSeryu Mar 17, 2025 •

edited

Loading