Skip to content

Conversation

kimsky247-coder
Copy link

안녕하세요 정다빈 리뷰어님! 이번 미션 함께하게 된 김하늘입니다☺️

현재 코드는 MVC패턴 기반으로 리팩터링한다는 조건에 맞추어

  • model
    • Car.java: 자동차 객체 (이름, 위치 상태 및 유효성 검사)
    • MoveCar.java: 자동차 이동 전략 정의
    • Race.java: 경주 상태 및 로직 관리
  • controller
    • RacingGameController.java: 게임 전체 흐름 관리
  • view
    • InputView.java: 사용자 입력 담당
    • OutputView.java: 결과 출력 담당
      -RaceMain : 프로그램 시작점
  • PredictableRandom: 랜덤값을 테스트하기 위한 가짜 객체
    로 구성하였습니다.

MVC 패턴을 적용하며 역할을 분리하려 노력했지만, 여전히 Model과 Controller의 경계가 모호하게 느껴집니다. "Model이 View의 존재를 몰라야한다"는 규칙에 따라, 입출력과 관련된 로직을 Controller부분으로 옮겨 구분해 보았는데, 알맞게 변경된 것인지 궁금합니다. 또한, Model과 Controller를 나누는 리뷰어님만의 기준이 있다면 조언을 구하고 싶습니다.

"랜덤한 요소가 존재하는 코드는 어떻게 테스트할 수 있는지 경험한다"는 요구사항에 대해, 의존성 주입과 PredictableRandom이라는 가짜 객체를 만들어 테스트를 진행했습니다. 제가 적용한 이 방법이 요구사항의 의도를 잘 만족시킨 것인지 궁금합니다.

또한, 요구사항에 따라 Car, MoveCar, Race 등 핵심 로직을 담당하는 클래스들에 대해 단위 테스트를 작성했습니다. 제가 작성한 테스트코드에서 제가 놓친 테스트 케이스가 있는지 궁금합니다.

Copy link

@70825 70825 left a comment

Choose a reason for hiding this comment

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

안녕하세요 하늘님~ 저도 잘 부탁드립니다! 🙇

MVC 패턴을 적용하며 역할을 분리하려 노력했지만, 여전히 Model과 Controller의 경계가 모호하게 느껴집니다. "Model이 View의 존재를 몰라야한다"는 규칙에 따라, 입출력과 관련된 로직을 Controller부분으로 옮겨 구분해 보았는데, 알맞게 변경된 것인지 궁금합니다.

제가 보기엔 잘 만드셨다고 생각해요. MVC 패턴은 의존성 방향을 생각해보면 좋은데요
의존성 방향이라는건 현재 파일에 있는 import 문이 어떤 패키지를 사용하고 있는가?를 보시면서 파악하시면 좋습니다.

현재 하늘님 코드를 살펴보면 아래 그림처럼 만들어서 잘 지켜지고 있다고 생각해요

스크린샷 2025-09-18 오후 9 19 50

MVC 패턴은 스프링 미션 진행할 때 제일 많이 체감이 되실거라고 생각합니다

또한, Model과 Controller를 나누는 리뷰어님만의 기준이 있다면 조언을 구하고 싶습니다.

저 같은 경우엔 도메인은 보통 특정 객체가 스스로 할 수 있는 일이라고 생각합니다. 문장이 좀 추상적인데, 객체를 의인화한다면 혼자서 알잘딱 행동할 수 있는 상태를 말해요. 예를 들어서 Car.javamoveForward 메서드는 자동차 객체 하나가 혼자서 할 수 있는 일이고, Race.javagetWinners()는 자동차 객체 하나로는 혼자서 할 수 없는 일이죠

View는 화면을 보여주고, Domain은 혼자서 스스로 할 수 있는 일이니, Controller는 View와 Domain을 이어주는 역할도 해주고, 하늘님이 적어주신 전반적인 미션 흐름을 관리해주는 역할을 해준다고 생각합니다

그런 의미에서 만약 자바 책을 읽으신다면 객체지향의 사실과 오해라는 책을 추천드려요~

"랜덤한 요소가 존재하는 코드는 어떻게 테스트할 수 있는지 경험한다"는 요구사항에 대해, 의존성 주입과 PredictableRandom이라는 가짜 객체를 만들어 테스트를 진행했습니다. 제가 적용한 이 방법이 요구사항의 의도를 잘 만족시킨 것인지 궁금합니다.

네 가짜 객체를 만든다는 의도 자체는 정확히 만족시켜줬습니다 👍👍 다만 nextInt만을 위해 extends를 사용하는 것보다 더 좋은 방법이 있어서 이것도 리뷰 남겨놨습니다~

또한, 요구사항에 따라 Car, MoveCar, Race 등 핵심 로직을 담당하는 클래스들에 대해 단위 테스트를 작성했습니다. 제가 작성한 테스트코드에서 제가 놓친 테스트 케이스가 있는지 궁금합니다.

요거는 IntelliJ에 테스트 케이스를 파악하는 기능이 있는데요

스크린샷 2025-09-19 오전 1 28 08

테스트 코드 전체 상위 폴더에 오른쪽 마우스 > More Run/Debug > 어쩌고저쩌고 Coverage 클릭하시면 어느 곳이 테스트가 덜 됐는지 확인이 가능해요

스크린샷 2025-09-19 오전 1 29 03

현재는 Race의 runRound가 테스트하지 못했다고 나오는데, 사실 저건 테스트 해봤자 의미가 없어서 충분히 모두 테스트 했다고 생각해요

Comment on lines 9 to 13
public Car(String carName) {
validateName(carName);
this.carName = carName;
this.position = 0;
}
Copy link

Choose a reason for hiding this comment

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

여기 있는 내용은 다른 곳에서 new Car(..)를 사용하기보다 정적 팩토리 메서드에 대해서 공부하시고 적용해보면 좋아보여요~

Copy link
Author

Choose a reason for hiding this comment

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

Car 클래스의 생성자를 private으로 변경하고, Car.fromName() 정적 팩토리 메서드를 추가했습니다

private Car(String name) {
        String trimmedName = name.trim();
        validateName(trimmedName);
        this.carName = trimmedName;
        this.position = 0;
    }

    public static Car fromName(String name) {
        return new Car(name);
    }

new Car()보다 fromName()이 이름으로 객체를 생성한다는 의도를 명확히 드러나 더 좋은 코드가 되었다고 생각합니다!

Copy link

@70825 70825 Sep 19, 2025

Choose a reason for hiding this comment

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

오 잘 적용하셨습니다. 다른 부분들도 정팩메를 사용할 수 있는 부분은 바꿔보는 것도 좋을 것 같아요

Comment on lines 6 to 7
private List<Car> cars;
private MoveCar moveCar;
Copy link

Choose a reason for hiding this comment

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

  1. 코드 읽는 입장에서 자동차 종류가 2개로 보이는데요. 자동차 이동 전략이라는 뜻에 MoveCar보다는 MoveCarStrategy나 더 좋은 네이밍이 있을 것으로 보여요~

  2. 일급 컬렉션이라는 내용도 학습해서 적용해보시면 좋습니다. 간단하게 설명하면 List<Car>도 객체로 만드는 건데요. List<Car> 혼자서도 할만한 일들이 있어보여서 만들어보면 좋아보입니다

Copy link
Author

Choose a reason for hiding this comment

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

MoveCar를 MoveCarStrategy로 변경하여 '이동 전략'이라는 클래스의 역할을 훨씬 명확하게 만들었습니다. 또한, List를 Cars라는 클래스로 분리하니 Race는 경주 진행에만, Cars는 자동차 목록 관리에만 집중하게 되어 각 클래스의 책임이 뚜렷해진 것 같습니다

Copy link

Choose a reason for hiding this comment

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

👍👍

Comment on lines 24 to 46
private int getMaxPosition() {
int maxPosition = 0;
for (Car car : cars) {
maxPosition = Math.max(maxPosition, car.getPosition());
}
return maxPosition;
}

public List<String> getWinners() {
int maxPosition = getMaxPosition();
List<String> winners = new ArrayList<>();
for (Car car : cars) {
addWinner(maxPosition, car, winners);
}
return winners;
}

private void addWinner(int maxPosition, Car car, List<String> winners) {
if (car.getPosition() == maxPosition) {
winners.add(car.getCarName());
}

}
Copy link

Choose a reason for hiding this comment

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

  1. 여기 메서드들은 JAVA stream API를 사용해서 간결하게 개선이 가능해보여요~
    자바, 코틀린으로 개발하면 계속해서 사용하게될 내용이기도 하고, 내용이 많아서 모든 내용을 공부하기보다는 지금 당장 필요한 부분만 찾아보시는걸 추천드립니다

  2. 국룰?로 사용하는 자바 메서드 순서 방식이 있는데요. 여기 글 참조하시고 다른 부분도 고쳐보면 좋아보입니다. 추가적으로 현재 메서드 순서가 getMaxPosition > getWinners > addWinner로 되어 있는데요. getWinnersgetMaxPosition, addWinner를 호출하는 구조이니 흐름 순서대로 getWinners > getMaxPosition > addWinner로 두는게 제일 좋습니다

Copy link
Author

Choose a reason for hiding this comment

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

getWinners와 getMaxPosition 메서드를 Stream API를 사용하여 간결하게 수정했습니다.

 public List<String> findWinners() {
        int maxPosition = getMaxPosition();
        return cars.stream()
                .filter(car -> car.getPosition() == maxPosition)
                .map(Car::getCarName)
                .collect(Collectors.toList());
    }

    private int getMaxPosition() {
        return cars.stream()
                .mapToInt(Car::getPosition)
                .max()
                .orElse(0);
    }

반복문이 사라지니 코드가 훨씬 간결해진 것 같습니다!
또한, 메서드를 호출 순서에 따라 정렬하였습니다. 순서대로 정리하니 가독성이 향상된 것 같습니다.

Comment on lines 23 to 27
private static void printPosition(int position) {
for (int i = 0; i < position; i++) {
System.out.print("-");
}
}
Copy link

Choose a reason for hiding this comment

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

Suggested change
private static void printPosition(int position) {
for (int i = 0; i < position; i++) {
System.out.print("-");
}
}
private static void printPosition(int position) {
System.out.print("-".repeat(position));
}

참고사항으로 더 간결하게 하자면 이런 방법도 있습니다 ㅋㅋ

Copy link
Author

Choose a reason for hiding this comment

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

printPosition 메서드의 for 루프를 "-".repeat(position)을 이용해 변경했습니다. for문보다 훨씬 간결해져 좋은 것 같습니다!

Comment on lines 11 to 19
@Test
void 자동차_생성_시_이름과_초기위치가_올바르게_설정된다() {
String carName = "car";

Car car = new Car(carName);

assertEquals(carName, car.getCarName());
assertEquals(0, car.getPosition());
}
Copy link

Choose a reason for hiding this comment

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

인텔리제이에서 한글 메서드명을 사용시 노란줄이 뜰텐데요 (아니면 제 설정이 잘못됐으니 말씀 부탁)

@DisplayName("model.Car 클래스 테스트")
@SuppressWarnings("NonAsciiCharacters")
@DisplayNameGeneration(ReplaceUnderscores.class)
public class CarTest {

이런식으로 클래스 위에 두 개의 어노테이션을 붙여주시면 됩니다

Copy link
Author

Choose a reason for hiding this comment

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

테스트 클래스에 @SuppressWarnings@DisplayNameGeneration을 추가하여 경고가 뜨지 않도록 수정하였습니다!

Comment on lines 31 to 32
@Test
void 자동차_이름이_5자를_초과하면_예외가_발생한다(){
Copy link

Choose a reason for hiding this comment

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

예외 테스트까지 좋네요 👍👍

@@ -0,0 +1,14 @@
import java.util.Random;

public class PredictableRandom extends Random {
Copy link

Choose a reason for hiding this comment

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

요거는 Random에서 nextInt만을 사용하기 위해 Random 자체를 상속하는건 배보다 배꼽이 큰 상황이라고 생각하는데요 (= Random에 nextInt 말고도 많은 메서드가 존재하므로, main패키지도 마찬가지)

상속보다는 interface를 하나 두어서 main 패키지에서 사용하는 random과 test 패키지에서 사용하는 PredictableRandomnextInt 메서드 하나만 사용할 수 있게 하는게 더 좋아보입니다~

이미 PredictableRandom을 만드셨으니 금방 만드실거라고 믿어요 😁

Copy link
Author

Choose a reason for hiding this comment

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

NumberGenerator라는 인터페이스를 새로 만들고, 실제 코드에서 사용할 RandomNumberGenerator와 테스트 코드에서 사용할 PredictableNumberGenerator가 이 인터페이스를 구현하도록 수정했습니다.또한, MoveCarStrategyRandom 클래스가 아닌 NumberGenerator 인터페이스에 의존하도록 변경했습니다. MoveCarStrategy가 이제 generate()라는 기능에만 의존하게 되어 역할이 훨씬 명확해진 것 같습니다.

Copy link

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.

이 구조가 전략 패턴이군요! 좋은 키워드 알려주셔서 감사합니다. 말씀해주신 대로, 시간 날 때 꼭 다른 예시들도 찾아서 학습해보겠습니다!

Comment on lines 25 to 31
public int getPosition() {
return position;
}

public String getCarName() {
return carName;
}
Copy link

Choose a reason for hiding this comment

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

getter의 순서는 위에 적은 순서대로 getCarName, getPosition으로 하면 좋습니다

인텔리제이에 맥 기준으로 cmd + N을 입력하면 아래 창이 나오는데, Getter로도 생성할 수 있어요

스크린샷 2025-09-19 오전 2 26 35

Copy link
Author

Choose a reason for hiding this comment

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

Car 클래스의 getter 순서를 필드 선언 순서에 맞게 조정했습니다. 작은 순서가 코드의 일관성을 높여준다는 점을 배웠습니다!

Comment on lines 20 to 26
private int generateRandomNumber() {
return this.random.nextInt(10);
}

private boolean shouldMove(int randomNumber) {
return randomNumber >= MOVE_THRESHOLD;
}
Copy link

Choose a reason for hiding this comment

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

generateRandomNumber에는 매직넘버를 사용하고, shouldMove에는 MOVE_THRESHOLD 상수를 사용하는 이유가 있나요??

Copy link
Author

Choose a reason for hiding this comment

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

랜덤 값의 범위도 게임 규칙이라는 것을 놓치고 있던 것 같습니다. 매직넘버 10을 private static final int RANDOM_NUMBER_BOUND = 10; 으로 수정하였습니다!

Copy link

@70825 70825 left a comment

Choose a reason for hiding this comment

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

안녕하세요 하늘님~ 리뷰 잘 반영한거 확인했습니다~!

제가 회사일이 바빠 너무 피곤해서 이전 리뷰엔 간결하게, 말투도 간결하게 리뷰 진행했는데요 🫠 요번엔 MVC 패턴은 잘 적용하셨고, 코드도 깔끔해졌으니 하늘님이 코드 작성한 의도가 궁금한 부분 질문 남겼어요

참고로 시간 나실 때 객체지향의 사실과 오해라는 책을 읽어보시는걸 추천드립니다. 저는 이 책에서 도움을 많이 받았는데, 자바 미션 진행하시면서 더 유지보수하기 쉬운 코드, 클래스 설계에 대해 많이 얻어가실 수 있을 거라고 생각해요


image

여기 이미지에서는 제가 처음 코멘트 드린 내용과는 다르게 View → Domain 화살표가 나지 않았는데요. 도메인 데이터는 그대로 보여주면서 View가 Domain을 모르게 할 수 있을까는 이번 리뷰 반영하면서 고민해보시면 좋아보여요~ (= 코드에 적용하라는 말은 아님)

import model.RandomNumberGenerator;

public class RaceMain {
public static void main(String[] args) {
Copy link

Choose a reason for hiding this comment

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

자바 미션에 대한 내용은 아니지만, 백엔드 준비하신다면 나중에 시간 나실 때 T메모리에 대해서 찾아보시면 좋아요
(필수 아님, 개발 관련 내용보다는 CS에 가깝습니다)

Copy link
Author

Choose a reason for hiding this comment

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

T메모리라는 개념은 처음 들어보네요. 좋은 키워드 알려주셔서 감사합니다! 이번 기회에 꼭 찾아서 학습해보겠습니다.

Comment on lines +7 to +8
NumberGenerator numberGenerator = new RandomNumberGenerator();
RacingGameController controller = new RacingGameController(numberGenerator);
Copy link

Choose a reason for hiding this comment

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

이전 코드와 다르게 이제 RacingGameController 안에 NumberGenerator를 넣게 되는데요. 이러면 어떤 장점을 가지게 된다고 생각하시나요?!

Copy link
Author

Choose a reason for hiding this comment

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

이전 코드에서는 컨트롤러가 직접 Random을 생성했습니다. 이 방법은 컨트롤러와 Random이 결합되어 컨트롤러의 전체 흐름을 테스트 할 때, 실행할 때 마다 결과가 달라져 테스트가 어려웠습니다.
현재 코드는 랜덤 숫자를 반환하는 RandomNumberGenerator대신, PredictableNumberGenerator를 주입할 수 있어 전체 흐름을 예측하고 검증할 수 있어 테스트가 용이해졌다고 생각합니다.

@@ -0,0 +1,45 @@
package controller;

import model.*;
Copy link

Choose a reason for hiding this comment

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

헛 요거는 여기 글 참고하셔서 인텔리제이 옵션에 와일드카드 안나오게 설정해주시길 바래요~
설정 이후 코드 리포맷은 아래 명령어 사용하시면 됩니다

  • 맥 기준: Cmd + Shift + L
  • 윈도우 기준: Ctrl + Alt + L

Copy link
Author

Choose a reason for hiding this comment

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

말씀해주신 대로 IntelliJ 설정을 변경하였습니다! 와일드 카드를 사용하면 가독성이 저하되고 불필요한 클래스가 로딩될 수 있어서 주의해야할 것 같습니다.

Comment on lines 29 to 36
private Cars setupCars() {
String[] carNames = InputView.readCarNames();
List<Car> carList = new ArrayList<>();
for (String name : carNames) {
carList.add(Car.fromName(name));
}
return new Cars(carList);
}
Copy link

Choose a reason for hiding this comment

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

RacingGameController가 자동차 이름을 설정해서 하나하나 자동차를 생성하는건 컨트롤러가 하는 일이 아닌 것 같다는 생각이 드는데 어떻게 생각하시나요?? 하늘님이 만드신 의도가 궁금하네요

Copy link
Author

Choose a reason for hiding this comment

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

사용자 입력값을 받아 자동차 객체를 하나하나 생성하는 것은 컨트롤러의 역할이 아닌 것 같습니다. 처음에는 사용자 입력을 받아 처리하기 때문에 컨트롤러의 역할이라고 생각하고 배치했었습니다. 생각해보니, 단일 책임 원칙을 지키고, MVC 패턴에 따라 책임을 나누기 위해서는 Cars 클래스에 자동차 목록 객체를 생성하는 역할을 옮기는 것이 좋은 설계인 것 같습니다.

public static Cars fromNames(List<String> carNames) {
        List<Car> carList = carNames.stream()
                .map(Car::fromName)
                .collect(Collectors.toList());
        return new Cars(carList);
    }
    private Cars setupCars() {
        String[] carNames = InputView.readCarNames();
        return Cars.fromNames(List.of(carNames));
    }

조언에 따라 이렇게 분리하였습니다

Copy link

Choose a reason for hiding this comment

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

굿굿 좋습니다 컨트롤러 코드가 확실히 깔끔해졌네요

Comment on lines 9 to 14
private Car(String name) {
String trimmedName = name.trim();
validateName(trimmedName);
this.carName = trimmedName;
this.position = 0;
}
Copy link

Choose a reason for hiding this comment

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

        String trimmedName = name.trim();
        validateName(trimmedName);

해당 부분은 정적 팩토리 메서드 만들면서 fromName 안에 들어갈 수도 있었을텐데 여기에 둔 이유가 궁금하네요~

Copy link
Author

Choose a reason for hiding this comment

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

정적 팩토리 메서드에 미숙해서 로직을 이동할 생각을 하지 못했던 것 같습니다. 생성자에 있던 로직을 from 정적 팩토리 메서드로 이동시켰습니다.

    private Car(String name) {
        this.carName = name;
        this.position = 0;
    }

    public static Car fromName(String name) {
        String trimmedName = name.trim();
        validateName(trimmedName);
        return new Car(trimmedName);
    }

생성자는 값을 할당하는 열할에만 집중하게 되어 코드 구조가 깔끔해진 것 같습니다,

Copy link

Choose a reason for hiding this comment

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

엇 아뇨아뇨 이거는 고치는게 아니라 원래 두는게 맞아요
물어본 이유는 정팩메 만들 때 같이 옮길 수도 있을 것 같은데 그대로 둔 이유가 궁금해서요 ㅋㅋㅋ...

Comment on lines 35 to 37
public List<Car> getCars() {
return Collections.unmodifiableList(cars);
}
Copy link

Choose a reason for hiding this comment

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

여기는 unmodifiableList(...)를 사용한 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

unmodifiableList()를 사용한 이유는 캡슐화를 지키기 위해서 입니다. 원본 리스트를 반환한다면, 외부에서 자동차를 추가하거나, 삭제할 수 있다고 생각하여 Cars의 객체를 허용하지 않은 방식으로 변경할 수 없도록 사용하였습니다.

Comment on lines 3 to 6
public class MoveCarStrategy {
private static final int MOVE_THRESHOLD = 4;
private static final int RANDOM_NUMBER_BOUND = 10;
private final NumberGenerator numberGenerator;
Copy link

Choose a reason for hiding this comment

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

Suggested change
public class MoveCarStrategy {
private static final int MOVE_THRESHOLD = 4;
private static final int RANDOM_NUMBER_BOUND = 10;
private final NumberGenerator numberGenerator;
public class MoveCarStrategy {
private static final int MOVE_THRESHOLD = 4;
private static final int RANDOM_NUMBER_BOUND = 10;
private final NumberGenerator numberGenerator;

사소한 부분이긴한데 이렇게 줄바꿈으로 구역을 나눠주시면 더 좋아요
코드는 사소한 규칙을 모아서 티끌 모아 태산을 만든다고 보시면 됩니다

Copy link
Author

Choose a reason for hiding this comment

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

이 부분까지 신경쓰지 못한 것 같습니다. 줄바꿈 규칙에 대해서도 공부해보아야겠다는 생각이 드네요!

Comment on lines 9 to 12
@Override
public int generate() {
return random.nextInt(RANDOM_NUMBER_BOUND);
}
Copy link

Choose a reason for hiding this comment

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

👍

int randomNumber = numberGenerator.generate();

if (shouldMove(randomNumber)) {
car.moveForward();
Copy link

Choose a reason for hiding this comment

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

테스트 코드에 moveCarNTimes를 보고 느꼈던 부분이 있는데요
자동차 이동 전략이라는 영역이 Car에 따로 빠져나와 있는 이유는 무엇인가요?
테스트 코드를 보니까 자동차 안에 자동차 이동 전략이 있는게 맥락상 맞을 것 같아가지구요

Copy link
Author

Choose a reason for hiding this comment

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

저도 이 부분에 대해 고민했었습니다.
Car의 책임은 moveForward()처럼 이동 명령을 받았을 때, 실제로 움직이는 것이라고 생각합니다.
MoveCarStrategy은 어떤 조건으로 움직일지 판단하고 결정하고 있습니다. 이것은 자동차의 책임을 넘어선다고 생각합니다.
따라서, 조건 판별 및 이동 결정은 외부에 있는 MoveCarStrategy가 담당하는 것이 더 좋다고 생각하였습니다.

Copy link

@70825 70825 Sep 20, 2025

Choose a reason for hiding this comment

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

아하 이해 했습니다
자동차 이동 전략이 운전자(자동차 이동을 결정하므로)를 자동차로부터 분리시킨 느낌이군요

Comment on lines 21 to 31
##프로젝트 구조

- model
* Car.java: 자동차 객체 (이름, 위치 상태 및 유효성 검사)
* MoveCar.java: 자동차 이동 전략 정의
* Race.java: 경주 상태 및 로직 관리
- controller
* RacingGameController.java: 게임 전체 흐름 관리
- view
* InputView.java: 사용자 입력 담당
* OutputView.java: 결과 출력 담당
Copy link

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.

이 부분을 놓치고 있었습니다. 코드를 업데이트 할 때 문서도 업데이트 하는 습관을 들이겠습니다

Copy link

@70825 70825 left a comment

Choose a reason for hiding this comment

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

하늘님 고생하셨어요~

마지막으로 리뷰 드릴 내용은 패키지 설정하기인데요. 여기 코멘트 밑부분에 대한 답변과 아래 내용 반영해서 코멘트 주시면 머지 진행할게요~

[1] model 패키지 정리하기

model
ㄴ Car
ㄴ Cars
ㄴ MoveCarStrategy
ㄴ NumberGenerator
ㄴ Race
ㄴ RandomNumberGenerator

현재 model 패키지 안에 6개의 클래스가 있는데요. 절대적인 수로 보면 하나의 패키지 안에 적당히 클래스가 있는 것 같지만, 다른 패키지와 비교해봤을 때 controller 패키지엔 1개의 클래스만 있고, view 패키지엔 2개의 클래스가 있어서 많아보이는데요

model 안에 더 패키지를 만들어서 하늘님 생각대로 패키지 만들어서 클래스를 넣거나, model 패키지 그대로 두는걸 추천드립니다~

패키지 만든 이유가 있으면 만든 이유도 적어주시고, 안만들고 model 패키지 그대로 남긴다면 남긴 이유도 알려주시면 좋아보여요

[2] 테스트 패키지 정리하기

이거는 IntelliJ에 테스트 코드 파일 생성하는 기능이 있는데요

  • mac 기준: Cmd + Shift + T
  • window 기준: Crtl + Shift + T
    를 사용하면 편하게 생성 가능해요

테스트 코드 파일 위치는 main/java와 같은 구조로 하는게 좋은데요

[현재 구조]
test/java
ㄴ CarTest
ㄴ MoveCarStrategyTest
ㄴ PredictableNumberGenerator
ㄴ RaceTest

[main/java와 같은 구조로 변경]
test/java/model
ㄴ CarTest
ㄴ MoveCarStrategyTest
ㄴ PredictableNumberGenerator
ㄴ RaceTest

그래서 여기는 model 패키지 정리하기 진행한 이후에 거기에 맞게 정리하는걸 추천드려요
마지막 리뷰까지 화이팅입니다~!! 💪

@kimsky247-coder
Copy link
Author

1. model 및 테스트 패키지 정리
저는 model 패키지 안의 클래스들을 나누는 것을 선택했습니다. 현재는 클래스가 많지 않지만, 추후 기능을 확장을 고려했을 때, 미리 분리하는 것이 유지보수성 측면에서 유리하다고 판단했습니다
model 패키지의 클래스들을 핵심 객체와 동작 전략으로 구분하였습니다.

model/domain: Car, Cars, Race
model/strategy: MoveCarStrategy, NumberGenerator, RandomNumberGenerator

test패키지 또한 동일한 구조로 구분하였습니다!

test/java/model/domain: CarTest, RaceTest
test/java/model/strategy: MoveCarStrategyTest, PredictableNumberGenerator

2. View와 Domain의 의존성 분리
처음에는 View가 Model의 Car 객체를 직접 참조했습니다. 이것은 View와 Model의 의존성이 높다는 문제가 있었습니다.
이를 해결하기 위해, Controller가 중간에서 Model로부터 필요한 데이터만 추출하여, View에 전달하도록 수정했습니다. 수정을 통해 View는 전달받은 데이터를 화면에 그리는 책임만 갖게 되었고, Model이 View에 영향을 주지 않는 구조가 된 것 같습니다

Copy link

@70825 70825 left a comment

Choose a reason for hiding this comment

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

고생하셨어요~!!

2번 같은 경우엔 지금처럼 필요한 데이터만 넣어주는 방식도 있고 dto라는 개념도 있는데요
dto는 단순하게 Car 대신 지금의 name, position을 하나의 객체로 감싸준 CarDto라는 클래스를 만들어서 getter만 해둔 객체라고 보시면 됩니다~

다음 미션도 화이팅이에요 🚀🚀

@boorownie boorownie merged commit 0a80371 into next-step:kimsky247-coder Sep 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants