Skip to content

Conversation

chemistryx
Copy link

@chemistryx chemistryx commented Sep 17, 2025

안녕하세요 이승용 리뷰어님! 이번 자동차 경주 미션 리뷰이로 참여하게된 하수한이라고 합니다.
이번 3,4단계에서는 기존 1,2단계 리뷰 과정에서 수정-적용했던 개념들을 이어서 활용해보려고 노력했습니다.
또한 4단계 과정에서 기초적인 MVC 개념을 도입하여 비즈니스 로직과 UI를 분리하려고 시도해보았습니다..!

아래는 현재 프로젝트 구조입니다:

domain/
- Car.java : 자동차 객체 및 관련 메소드 정의
- CarRegistry.java : 자동차 목록 관리
- Game.java : 전반적인 게임 실행 담당
- RoundResult.java : 게임 결과를 담는 객체
generator/
- NumberGenerator.java : 숫자 생성기 Interface 정의
- RandomNumberGenerator.java : 랜덤 숫자 생성기
view/
- InputView.java : 사용자로부터 입력 받는 기능 수행
- ResultView.java : 사용자에게 출력 하는 기능 수행
Main.java  : Main Entrypoint

애플리케이션 실행 로직의 경우 다음과 같습니다:

  1. Main entrypoint에서 InputView를 통해 사용자로부터 자동차의 이름과 시도 횟수를 입력받습니다.
  2. 입력 받은 값을 바탕으로 Game 객체를 생성합니다. -> Game.of(count)
  3. 입력 받은 자동차 이름을 Game에 등록합니다. -> game.registerCars(names)
  4. 게임을 실행한 후(game.execute()), 반환된 결과(RoundResult)를 저장합니다.
  5. 우승자를 구한 후(game.getWinners()), 반환된 결과를 저장합니다.
  6. 앞서 저장한 결과들을 ResultView에 넘겨주어 결과를 출력합니다.

RoundResult 클래스의 경우 Car 객체 리스트를 저장한다는 점에서 CarRegistry와 유사하다고 생각되지만, 게임 결과를 게임 실행이 끝난 후 한번에 출력하기 위해 각 Round의 결과를 저장해놓을 곳이 필요하다고 판단하여 추가했습니다.

아래는 구현 과정에서 발생한 질문 사항입니다:

  1. 현재 프로젝트 구조가 Domain과 View로 구분되어 있는데, MVC 패턴의 경우 Model-View-Controller로 구성되어 있는 것으로 알고있습니다. 그렇다면 지금 Domain 구조의 경우 Model과 Controller가 합쳐있는 구조로 이해하면 되는 것일까요? 일단 구현 자체는 Model과 Controller가 혼합된 구조로 진행했습니다.
  2. 현재 입력 값 검증 테스트의 경우 하드코딩된 값(자동차 이름 5, 시도 횟수 1)을 통해 테스트를 진행하고 있는데, 추후에 해당 조건이 변경되는 경우 테스트가 실패할 것으로 예상됩니다. 구현 부분에서는 매직 넘버에 대한 처리가 되어있지만, 이를 테스트 코드에도 적용시켜야 할 것 같은데, 이에 대한 접근 방법이 궁금합니다.
  3. 테스트 코드를 작성 중이긴 하나, 현재 테스트가 필요한 부분이 생각날때 마다 수작업으로 작성하고 있습니다. 따라서 경우에 따라 테스트 코드가 누락되는 상황이 생길 수도 있을 것 같은데, 이에 대한 대비 혹은 대응 방안이 궁금합니다!

긴 글 읽어주셔서 감사드리며, 혹시 부족한 부분이나 궁금하신 사항이 있다면 말씀해주세요!

Copy link

@kokodak kokodak left a comment

Choose a reason for hiding this comment

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

안녕하세요 수한님 :) 먼저 리뷰가 늦어져서 죄송해요. 리뷰를 오랜만에 하니까 되게 어렵기도 하고, 어떻게 해야 메시지를 잘 전달드릴까 고민하다가 더 늦어졌네요. 이후의 티키타카는 최대한 빠르게 답신하도록 노력할게요. 그리디에서의 첫 미션 고생 많으셨습니다.

좋은 질문들을 주셨는데, 모두 코드리뷰 내용 안에 남겨두었습니다. 리뷰 내용 or 리뷰에 없는 내용 중 궁금한 점이 있다면 언제든 디스코드나 깃허브로 알려주세요!

@@ -0,0 +1,51 @@
package io.suhan.racingcar.domain;
Copy link

Choose a reason for hiding this comment

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

현재 프로젝트 구조가 Domain과 View로 구분되어 있는데, MVC 패턴의 경우 Model-View-Controller로 구성되어 있는 것으로 알고있습니다. 그렇다면 지금 Domain 구조의 경우 Model과 Controller가 합쳐있는 구조로 이해하면 되는 것일까요? 일단 구현 자체는 Model과 Controller가 혼합된 구조로 진행했습니다.

이 질문은 여기서 대화해보면 좋을 것 같습니다.

일단 구현 자체는 Model과 Controller가 혼합된 구조로 진행했습니다.

보기에는 수한님 나름대로 Model과 Controller를 정의하고 계신 것 같네요. 그렇지 않으면 혼합된 구조라는 사실조차 인지할 수 없을테니까요. 수한님은 Model과 Controller를 어떻게 정의하고 계신가요? 이걸 알려주시면 더 대화를 나눠볼 수 있을 것 같네요.

제 생각을 짧게 남겨보자면, Domain과 Controller는 포함 관계가 성립하지 않는 서로 다른 영역이라고 생각해요. 저는 Controller를 일종의 Entrypoint라고 생각해서, Main.java가 Controller의 역할을 하고 있는 게 아닐까? 하는 생각이 드네요.

Copy link
Author

Choose a reason for hiding this comment

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

보기에는 수한님 나름대로 Model과 Controller를 정의하고 계신 것 같네요.

네 맞습니다. 제 생각엔 Model의 경우 객체?의 정의만을 담고 있는 일종의 설계도 같은 것이라고 생각합니다. 그리고 Controller의 경우 Model을 기반으로 실제 비즈니스 로직이 구현되는 곳이라고 이해하고 있습니다. 그리하여 Car.java를 예로 들었을 때, Car의 정의에 필요한 필드(name, position)만 존재하는 것이 아니라, 실제 비즈니스 로직(move(), incrementPosition())이 혼재되어 있어 혼합된 구조라고 판단하게 되었습니다.

그래서 만약에 여기서 더 나누게 된다면 앞서 언급한 비즈니스 로직들만 가지고 있는 CarController를 추가로 만들어 Controller과 Model의 역할을 확실히 구분시키는 것이 MVC 패턴에 좀 더 부합하는 구조가 아닌가 하는 생각을 하고 있습니다..!

List<RoundResult> results = game.execute();
List<Car> winners = game.getWinners();

ResultView.printExecutionResult(results);
Copy link

Choose a reason for hiding this comment

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

image

현재 프로그램은 위의 기능 요구사항을 만족하지 않는 것 같아요. RoundResult들의 각 상태를 다시 체크해보면 좋겠네요.
프로그램에서 코드를 잘 설계하는 일은 중요하지만, 그것보다 더 중요한 건 기획대로 기능이 잘 동작하는가? 인 것 같아요.

여기서 아래 질문에 대해 고민해볼 수 있을 것 같아요.

테스트 코드를 작성 중이긴 하나, 현재 테스트가 필요한 부분이 생각날때 마다 수작업으로 작성하고 있습니다. 따라서 경우에 따라 테스트 코드가 누락되는 상황이 생길 수도 있을 것 같은데, 이에 대한 대비 혹은 대응 방안이 궁금합니다!

현실적인 질문을 주신 것 같습니다. 되게 좋은 질문인데요.
말씀하신대로 테스트는 언제든 누락될 위험이 있습니다. 여기서 중요한 건 테스트가 누락됐다는 사실을 어떻게 인지할 것인가? 인 것 같아요. 저는 아래 두 가지 정도가 떠오르네요.

A. 테스트 커버리지 확인하기

테스트 커버리지는 내가 현재 작성한 테스트 코드가 비즈니스 코드의 몇 퍼센트를 커버하는가를 정량적으로 측정하는 방법이에요.
대부분의 IDE에는 Test with Coverage 라는 기능이 포함되어 있고, 이걸로 커버리지를 측정할 수 있어요. 그러면 현재 내 코드에서 어떤 부분이 테스트로부터 보호받지 못하는지 알아챌 수 있습니다.

B. 기능을 직접 사용해보며 문제가 없는지 확인하기

저는 테스트가 기능의 스펙을 잘 정의하는 일이라고 생각합니다. 한 기능이 가지고 있는 여러 스펙이 존재할텐데, 이걸 테스트로 녹여내서 그 기능이 의도대로 잘 동작하는지를 검증하는 거죠.
하지만 테스트 코드의 한계가 있다고 한다면, 그건 개발자가 예상 가능한 영역 안에서만 검증 가능하다는 점입니다. 어떤 기능에 의한 사이드 이펙트는 대부분 예상하지 못한 곳에서 발생하다보니 테스트 코드로 사전에 검증을 한다는 게 되게 어렵습니다. 이건 커버리지로도 체크하기 어려워요. 겉보기에는 코드 논리 전개에 문제가 없거든요.
여기서 수한님의 예시를 대입해보면, 논리적으로는 RoundResult에 문제가 없을 것이라 예상하셨겠지만, 자바의 동작 방식을 간과한 사례가 되겠네요.
이 한계를 극복할 수 있는 좋은 방법은 기능을 직접 사용해보며 테스트 해보는 거예요. 사용을 하다가 문제가 발생했다면, 버그를 재현하기 위한 방법과 원인을 찾고, 그 케이스에 대한 스펙을 테스트로 다시 정의하는 거죠. 예상하지 못했던 것을 예상할 수 있는 지점으로 끌고 올 수 있는 좋은 방법이라 생각합니다.

아무리 잘 만든 소프트웨어라도 사이드 이펙트는 필연적으로 존재합니다. 즉, 예상하지 못한 지점에서 장애가 날 수 있는 확률이 항상 존재한다는 얘기이기도 해요.
만약 문제가 발생했다면, 그 케이스를 커버하는 테스트를 추가해서 추후에 다시 그런 일이 발생하지 않도록 견고한 검증부를 만들어 나가는 것이 중요한 것 같습니다.

Copy link
Author

Choose a reason for hiding this comment

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

어제 오프라인 리뷰에서 테스트 코드 관련해서 승용님의 좋은 조언들을 들을 수 있어서 유익한 시간이었던 것 같습니다.
RoundResult의 경우 최초 구현에서는 정상적으로 잘 작동했으나, 리팩토링 단계에서 영향이 있었던 것으로 보입니다.. 의도치 않게 승용님께서 언급해주신 B. 기능을 직접 사용해보며 문제가 없는지 확인하기의 중요성을 다시 한번 확인하게 된 좋은 경험이 된 것 같습니다!

해당 부분의 경우 #157 (comment) 에서 언급해주신 부분을 수정하여 정상 작동함을 확인했습니다..!

Comment on lines 14 to 30
private Car(String name, NumberGenerator generator) {
this.name = name;
this.generator = generator;
this.position = 0;
}

public static Car of(String name) {
return Car.of(name, new RandomNumberGenerator());
}

public static Car of(String name, NumberGenerator generator) {
if (name.length() > CAR_NAME_MAXIMUM_LENGTH) {
throw new IllegalArgumentException("자동차의 이름은 " + CAR_NAME_MAXIMUM_LENGTH + "자 이하만 가능합니다.");
}

return new Car(name, generator);
}
Copy link

Choose a reason for hiding this comment

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

전반적으로 생성자를 private으로 감추고 정적 팩터리 메서드를 통해서 객체를 생성하도록 작성해주셨네요. 어떤 이유에서 정적 팩터리 메서드를 사용하도록 하셨는지 궁금해요.

Copy link
Author

Choose a reason for hiding this comment

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

위 케이스에서 정적 팩토리 메소드를 사용한 이유는 객체를 생성하는 과정에서 입력 값 검증 로직을 추가하기 위해서입니다!
반드시 Car.of()를 통해서만 객체를 생성할 수 있도록 구성하여 반드시 검증 로직을 통과하도록 구현하였습니다.

검증 로직이 없는 곳에서도 정적 팩토리 메소드를 통해 객체를 생성하는 부분이 있는데(e.g., RoundResult), 이는 사실 사용하지 않아도 무방하지만 일단 실습이 목적이기에 정적 팩토리 메소드로 구현을 진행했습니다.

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 40 to 43
public List<Car> getWinners() {
int bestPosition = carRegistry.getBestPosition();

return carRegistry.getRegisteredCars()
.stream()
.filter((car) -> car.getPosition() == bestPosition)
.toList();
}
Copy link

Choose a reason for hiding this comment

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

Game 객체가 일종의 슈퍼 객체가 되고 있는 것 같아요. 많은 책임을 가지고 있는 객체라고 느껴지네요.
우승자를 찾기 위한 자원들은 Game이 아닌 CarRegistry에 이미 모두 존재하는 것으로 보여요. 맡길 수 있는 책임은 최대한 맡겨보면 어떨까요? CarRegistry와 잘 협력해보면 좋을 것 같아요!

Copy link
Author

@chemistryx chemistryx Sep 22, 2025

Choose a reason for hiding this comment

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

우승자를 찾기 위한 로직 자체는 CarRegistry에 이미 모두 존재한다는 것에는 동의합니다. 하지만 굳이 getWinners()를 Game 내부에 둔 이유는 해당 메소드 이름 자체가 CarRegistry보다는 Game에 더 적합하다는 생각이 들어서 남겨두었습니다.

하지만 승용님 말씀대로 Game 객체의 크기가 점점 비대해지는 문제가 있는 것은 사실이라 기존 CarRegistry를 확장하여 우승자 계산 로직을 CarRegistry::getCarsWithBestPosition()로 옮긴 뒤 Game 객체에서 이를 단순히 호출하는 느낌으로 변경하는 것에 대해서는 어떻게 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

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

c0c9b38 커밋에 반영했습니다.

Comment on lines 53 to 58
public void registerCars(List<String> names) {
for (String name : names) {
Car car = Car.of(name);
carRegistry.register(car);
}
}
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

@chemistryx chemistryx Sep 22, 2025

Choose a reason for hiding this comment

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

이 역시도 기존 CarRegistryCarManager로 확장하여 해당 클래스 내부로 이관하도록 하겠습니다..!
네이밍의 경우 CarRegistry를 유지하는 방향으로 하고 메소드만 이관 완료했습니다.


for (int i = 0; i < rounds; i++) {
carRegistry.moveCars();
results.add(RoundResult.of(carRegistry.getRegisteredCars()));
Copy link

Choose a reason for hiding this comment

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

carRegistry.getRegisteredCars()로 가져온 List<Car>의 메모리 주소가 과연 다를까요? 🤔
주소가 같다면 어떤 일이 벌어질까요?
Deep Copy, Shallow Copy에 대해 공부해보면 좋을 것 같습니다.

Copy link
Author

@chemistryx chemistryx Sep 22, 2025

Choose a reason for hiding this comment

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

Deep Copy를 통한 복사로 수정 완료했습니다.

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 7 to 8
private static final int CAR_MOVE_THRESHOLD = 4;
private static final int CAR_NAME_MAXIMUM_LENGTH = 5;
Copy link

Choose a reason for hiding this comment

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

현재 입력 값 검증 테스트의 경우 하드코딩된 값(자동차 이름 5, 시도 횟수 1)을 통해 테스트를 진행하고 있는데, 추후에 해당 조건이 변경되는 경우 테스트가 실패할 것으로 예상됩니다. 구현 부분에서는 매직 넘버에 대한 처리가 되어있지만, 이를 테스트 코드에도 적용시켜야 할 것 같은데, 이에 대한 접근 방법이 궁금합니다.

위 질문을 제가 이해하기로는, CAR_MOVE_THRESHOLD 같은 값이 구현부에서는 상수로 빠져 있지만 테스트에서는 매직 넘버로 사용하고 있어서, 추후에 비즈니스가 바뀔 경우 테스트가 깨지는 상황을 말하신 것 같네요.

제 생각에 가장 좋은 방법은 상수의 접근 제어자를 private에서 public으로 변경하는 거예요. 이 방안은 어떠신가요?

Copy link
Author

Choose a reason for hiding this comment

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

말씀해주신 방법의 경우 가장 간단하면서 문제를 해결할 수 있는 좋은 방법이라고 생각합니다. 하지만 다시 생각해보니 구현부에서 정의된 상수의 변화에 따라 테스트 결과 또한 변화하는게 맞는 접근인지 의문이 듭니다.

만약에 누군가가 CAR_MOVE_THRESHOLD를 실수로 5로 바꾸었다고 했을 때(실제로는 4가 맞음), 테스트코드에서 상수로 처리하게 되면 이 테스트가 성공하게 되어버리는 상황이 생길 것 같은데 이렇게 된다면 테스트의 의미가 없어져버리는 것이 아닌가요..?

테스트 자체가 사용자가 의도적으로 변경한 값에 대해서는 검출할 수 없는 것이 맞다면, 이 부분은 그냥 넘어가도 될 것 같습니다..!

Copy link
Author

Choose a reason for hiding this comment

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

이 부분은 어제 오프라인 리뷰 단계에서 테스트 Scope에 따라 해당 상수가 중요한 경우도 있고, 아닌 경우도 있는 것으로 이해했습니다. 따라서 지금 case에서는 자동차의 move()부분 검증이 중요하다고 판단되기에 상수의 접근 제어자를 public으로 변경하여 테스트코드에서도 사용 가능하도록 수정 진행하겠습니다!

Copy link
Author

Choose a reason for hiding this comment

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

670c870 커밋에 반영했습니다.

@chemistryx chemistryx force-pushed the chemistryx branch 2 times, most recently from 5be48b8 to 670c870 Compare September 23, 2025 04:30
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.

2 participants