feature(step4): 자동차 경주(우승자) 기능 구현 #6056

LeeJin0527 · 2025-03-20T04:56:19Z

안녕하세요~ 오늘도 잘 부탁드립니다 😄

- 진입점에 맞추어 main 클래스 package안으로 이동 - 클래스, 인스턴스 변수 공백 추가 - private method static 제거 - 테스트 코드 분리 - util 클래스 객체 생성 방지를 위해 private생성자 추가

- Car, Position 불변 객체로 변경 - RacingGame 접근 범위 private로 제한 - 테스트 케이스 불변객체 적용 - 유틸클래스 private 생성자 추가 - InputView > InputValidator로 변경

- 1자 이상 5자 이하, 공백 및 6자 이상 예외 처리 - 에러메시지 추가 - private 생성자 추가

- CarGenerator 관련 파일 삭제하고 팩토리 메서드 패턴을 통해 이름 입력으로 차량 객체 생성하도록 수정 - 테스트 코드에 이름 받도록 수정

testrace

안녕하세요 이진님 😃
불변 객체를 잘 활용해서 잘 구현해 주셨네요 👍
몇 가지 코멘트 남겨두었는데 확인해 주시고 다시 리뷰 요청해 주세요.

testrace · 2025-03-20T08:24:03Z

src/main/java/racingcar/Main.java

+        InputView inputView = new InputView();
+
+        final List<String> names = getValidCarNames(inputView);
+        final int attemptCount = getValidAttemptCount(inputView);
+
+        RacingGame racingGame = RacingGame.createRacingGame(names);
+
+        final RacingGame finalResult = racingGame.race(attemptCount);


final 변수 👍
변수에 따라 final 이 있기도하고 없기도 한데요
어떤 기준으로 지역 변수에 final 키워드를 추가하시는지 궁금합니다 🤔

웬만하면 불변성을 나타내려고 final을 붙이는데 누락이 있는거 같습니다. 좀 더 꼼꼼히 보도록 하겠습니다!
피드백 감사합니다 ~! 😄

testrace · 2025-03-20T08:24:12Z

src/main/java/racingcar/model/Car.java


+    private final String name;


이름도 Position처럼 원시 값을 포장하면 어떨까요?
이름 유효성에 대한 책임을 위임할 수 있을 것 같아요

testrace · 2025-03-20T08:24:16Z

src/main/java/racingcar/model/Car.java

+    public Car(String name) {
+        validateName(name);
+        this.name = name;
+        this.position = new Position(0);
+    }
+
+    public Car(String name, Position initialPosition) {
+        validateName(name);
+        this.name = name;
+        this.position = initialPosition;
+    }


부 생성자는 주 생성자를 호출해 중복되는 코드를 제거하면 어떨까요?
생성자에 대한 단위 테스트도 추가되면 좋을 것 같아요

testrace · 2025-03-20T08:24:23Z

src/main/java/racingcar/model/Position.java

-    public void incrementPosition() {
-        position += 1;
+    public Position incrementPosition() {
+        return new Position(this.position + 1);


불변 객체 💯

testrace · 2025-03-20T08:24:59Z

src/main/java/racingcar/view/OutputView.java

+
+    private OutputView() {}
+
+    private static final String SEPERATOR = ", ";


Position, Car 클래스에도 출력을 위한 기호들이 선언되어 있는데요
출력에 관련된 상수는 모두 OutputView로 모아서 응집도를 높이면 어떨까요?

testrace · 2025-03-20T08:25:30Z

src/test/java/racingcar/model/CarTest.java

+    @DisplayName("자동차가 이동해도 기존 객체는 변하지 않는다.(불변 객체 테스트)")
+    @Test
+    void car_not_move_immutable_test() {
+        String carName = "car";
+        Car initialCar = new Car(carName);

-        car.move(randomValue);
-        car.move(randomValue2);
+        Car movedCar = initialCar.move(4);

-        assertThat(car.isAtPosition(new Position(expectedPosition))).isTrue();
+        assertThat(initialCar.isAtPosition(new Position(0))).isTrue();
+        assertThat(movedCar.isAtPosition(new Position(1))).isTrue();


불변에 대한 테스트 👍
isAtPosition 메서드로 값을 꺼내 비교해도 좋지만 비교해야할 값이 많다면 코드의 양도 많이 늘어날 수 있는데요
객체끼리 비교하는 방법도 고려해 보시면 좋을 것 같아요

assertThat(initialCar).isNotEqualTo(movedCar);

testrace · 2025-03-20T08:25:50Z

src/test/java/racingcar/model/PositionTest.java

+    @DisplayName("초기 위치가 5인 자동차가 2번 이동 하면 현재 위치보다 2 증가한다.")
+    @ParameterizedTest
+    @CsvSource({
+            "0,1,1,0",
+            "3,4,4,5",
+            "5,4,1,6",
+            "5,1,5,6"
+    })
+    void move_initial_move(int initialPosition, int randomValue, int randomValue2, int expectedPosition) {
+        String carName = "car";
+        Car initialCar = new Car(carName, new Position(initialPosition));
+
+        Car movedOnceCar = initialCar.move(randomValue);
+        Car movedTwiceCar = movedOnceCar.move(randomValue2);


위치 값에 알고 있는 객체는 Position입니다.
그러니 테스트 대상도 Position#incrementPosition() 가 되어야 하지 않을까요?

testrace · 2025-03-20T08:26:16Z

src/test/java/racingcar/model/RacingGameTest.java

@@ -3,16 +3,20 @@
 import org.junit.jupiter.api.DisplayName;
 import org.junit.jupiter.api.Test;

+import java.util.List;
+
 import static org.assertj.core.api.Assertions.assertThat;

 class RacingGameTest {


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

우승자 기능이 추가되었지만 관련 테스트가 누락되었네요
모든 public 메서드에 대해 누락된 테스트가 없는지 검토해 보시면 좋을 것 같아요

- 생성자 테스트 코드 작성

testrace

안녕하세요 이진님 😃
피드백 반영 잘해주셨네요 👍
소소하게 코멘트 남겨두었는데 확인해 주시고 다음 단계 진행해 주세요!

testrace · 2025-03-26T13:43:02Z

src/main/java/racingcar/model/Car.java

@@ -1,38 +1,23 @@
 package racingcar.model;

-import racingcar.message.ErrorMessage;
+import static racingcar.view.OutputView.NAME_POSITION_SEPERATOR;


MVC 패턴을 의식하셔서 model, view 패키지를 나누셨을거라 생각되는데요
이미 OutputView에서 Car 모델을 참조하고 있기에 model 패키지와 view 패키지는 순환참조하게 되었네요.
다음 단계의 주제이니 의존성 방향을 정리해 보시면 좋을 것 같아요.

testrace · 2025-03-26T13:43:11Z

src/main/java/racingcar/model/CarName.java

+public class CarName {
+
+    private final String name;


객체 분리 👍

testrace · 2025-03-26T13:43:33Z

src/test/java/racingcar/model/CarNameTest.java

+    @DisplayName("자동차명은 1자에서 5자 이하이다.")
+    @ParameterizedTest
+    @ValueSource(strings = {"a", "ab", "abc", "abcd", "abcde"})


이름 길이가 10자, 20자 제한으로 변경되면 테스트해야할 값도 많아질 것 같아요.
경계값이 되는 값으로 테스트하면 어떨까요?

testrace · 2025-03-26T13:44:21Z

src/test/java/racingcar/model/CarNameTest.java

+    @DisplayName("자동차명이 null이면 exception을 발생시킨다.")
+    @Test
+    void car_name_with_blank() {
+        assertThatThrownBy(() -> new Car(new CarName("")))


CarName 에 대한 테스트이니 new Car 는 제외하면 어떨까요?

Suggested change

assertThatThrownBy(() -> new Car(new CarName("")))

assertThatThrownBy(() -> new CarName(""))

testrace · 2025-03-26T13:45:18Z

src/test/java/racingcar/model/CarTest.java


        Car movedCar = initialCar.move(randomValue);

-        assertThat(movedCar.getName()).isEqualTo(carName);
+        assertThat(movedCar.getName().getName()).isEqualTo(carName);


자동차 객체는 비교적 단순하지만 객체 그래프가 깊어지면 get..get..get..get.. 같은 코드가 될 가능성이 높습니다.
디미터 법칙을 적용해 개선해 보시면 좋을 것 같아요.

LeeJin0527 added 6 commits March 20, 2025 08:51

refactor(step3): 코드리뷰 반영

3755dff

- 진입점에 맞추어 main 클래스 package안으로 이동 - 클래스, 인스턴스 변수 공백 추가 - private method static 제거 - 테스트 코드 분리 - util 클래스 객체 생성 방지를 위해 private생성자 추가

refactor(step3): 코드리뷰 반영

66a7abf

- Car, Position 불변 객체로 변경 - RacingGame 접근 범위 private로 제한 - 테스트 케이스 불변객체 적용 - 유틸클래스 private 생성자 추가 - InputView > InputValidator로 변경

feat(step4): 자동차 이름 입력 값 검증 추가

897b852

- 1자 이상 5자 이하, 공백 및 6자 이상 예외 처리 - 에러메시지 추가 - private 생성자 추가

feat(step4): 자동차 객체 생성 시 이름 로직 추가

de6c4b8

- CarGenerator 관련 파일 삭제하고 팩토리 메서드 패턴을 통해 이름 입력으로 차량 객체 생성하도록 수정 - 테스트 코드에 이름 받도록 수정

feat(step4): 경주 진행 시 매 회차 자동차들의 위치 출력 기능 구현

d0bacbd

feat(step4): 최종 우승자 출력기능 구현

c535ddf

LeeJin0527 changed the base branch from master to leejin0527 March 20, 2025 04:57

docs(step4): 기능 요구사항 md 완료 항목 체크

5b22668

testrace requested changes Mar 20, 2025

View reviewed changes

LeeJin0527 added 7 commits March 26, 2025 09:08

refactor(step4) : final 추가

06dbad0

refactor(step4) : 자동차명 원시 값 포장 & 테스트코드 작성

75f3c15

refactor(step4) : 부 생성자를 주 생성자를 호출하도록 수정

df07b2f

- 생성자 테스트 코드 작성

refactor(step4) : 출력과 관련된 상수 OutputView로 이동

b5e3e93

test(step4) : 객체 값 비교 테스트 추가

d8def67

test(step4) : PositionTest 위치 기반으로 수정

7c551d1

test(step4) : 우승자 출력 테스트 추가

722e61c

testrace approved these changes Mar 26, 2025

View reviewed changes

testrace merged commit dbf8d5c into next-step:leejin0527 Mar 26, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature(step4): 자동차 경주(우승자) 기능 구현 #6056

feature(step4): 자동차 경주(우승자) 기능 구현 #6056

LeeJin0527 commented Mar 20, 2025

testrace left a comment

testrace Mar 20, 2025

LeeJin0527 Mar 25, 2025

testrace Mar 20, 2025

testrace Mar 20, 2025

testrace Mar 20, 2025

testrace Mar 20, 2025

testrace Mar 20, 2025

testrace Mar 20, 2025

testrace Mar 20, 2025

testrace left a comment

testrace Mar 26, 2025

testrace Mar 26, 2025

testrace Mar 26, 2025

testrace Mar 26, 2025

testrace Mar 26, 2025


		private OutputView() {}

		private static final String SEPERATOR = ", ";

	assertThatThrownBy(() -> new Car(new CarName("")))
	assertThatThrownBy(() -> new CarName(""))

feature(step4): 자동차 경주(우승자) 기능 구현 #6056

feature(step4): 자동차 경주(우승자) 기능 구현 #6056

Conversation

LeeJin0527 commented Mar 20, 2025

testrace 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

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

testrace 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