Skip to content

Conversation

tae-wooo
Copy link

안녕하세요 조승현 리뷰어님! 코드 리뷰 잘 부탁드립니다.

저는 지난 과제를 진행할 때 기능 요구사항 구현에만 집중하다 보니, 새로운 클래스를 작성할 때마다 기존 코드를 크게 수정해야 했고 클래스 간 의존도도 높아지는 문제가 있었습니다. 이번 과제를 하면서는 이 점을 개선하기 위해, 코드를 작성하기 전에 설계 방향을 먼저 정하고 진행하는 데에 중점을 두었습니다.

제가 생각한 설계 흐름은 다음과 같습니다.

설계 흐름

사용자는 View를 통해 자동차 이름과 라운드 횟수를 입력하고, 입력값은 Controller에 전달된다.

Controller는 입력값을 Model에 넘겨 n대의 자동차를 초기화한다.

Controller는 라운드 횟수만큼 자동차 이동을 지시하고, Model은 기준값(4)에 따라 전진 또는 정지를 결정한다.

모든 라운드가 끝나면 Controller는 자동차들의 최종 위치를 확인해 우승자를 판별하고, 그 결과를 Model에 저장한다.

Controller는 Model에서 우승자 정보를 가져와 View에 전달하고, View는 이를 출력한다.

Model
자동차 (Car)
-속성: 이름, 위치
-기능: 전진 (기준값 4는 상수로 관리)

자동차 집합 (Cars)
-여러 대의 자동차 관리

-우승자 (Winner)
-최종 결과 저장

View
-입력
-자동차 이름 (쉼표 구분, 5자 이하)
-이동 횟수

출력
-라운드별 자동차 이동 결과 (자동차 이름 포함)
-최종 우승자 (한 명 이상 가능)

Controller
-n대의 자동차 초기화 (이름 전달)
-라운드 진행 및 자동차 이동 지시
-우승자 판별 후 Model에 저장
-최종 결과를 View에 전달

Utility
-0~9 범위의 랜덤 값 생성

이렇게 정리하면서, 클래스 간 의존도를 줄이고 역할을 명확히 하는 데 중점을 두었습니다.

제가 MVC 패턴을 공부하면서 느낀 점은, Controller는 View에서 받은 정보를 Model에 전달하고, Model과 Model 간의 연결을 조율하며, 다시 Model에서 처리한 결과를 View에 전달하는 역할이라는 것입니다. 그래서 GameManage를 설계할 때, 경기 진행과 우승자 판별 로직을 Controller가 담당하도록 했습니다. 그런데 이렇게 하다 보니 Controller가 맡은 책임이 많아진다는 생각이 들었습니다.
그래서 궁금한 점은, 우승자 모델을 단순히 결과 저장용으로 두는 것보다, 우승자를 판별하는 로직까지 책임지도록 하는 것이 더 적절한지입니다.

스터디 때 배운 전략 패턴을 적용해보려고 시도했지만, 변경 과정에서 어려움을 느껴 이번에는 적용하지 못했습니다. 개념을 더 공부한 뒤 다시 적용하겠습니다.

@tae-wooo tae-wooo changed the base branch from main to tae-wooo September 18, 2025 11:24
@BackFoxx
Copy link

안녕하세요 태우님! 1, 2단계 PR을 구경한 결과 아직 자바 문법이 익숙하지 않으신 것 같아, 자바 문법을 더 초점을 두고 리뷰를 드렸습니다.

제가 MVC 패턴을 공부하면서 느낀 점은, Controller는 View에서 받은 정보를 Model에 전달하고, Model과 Model 간의 연결을 조율하며, 다시 Model에서 처리한 결과를 View에 전달하는 역할이라는 것입니다. 그래서 GameManage를 설계할 때, 경기 진행과 우승자 판별 로직을 Controller가 담당하도록 했습니다. 그런데 이렇게 하다 보니 Controller가 맡은 책임이 많아진다는 생각이 들었습니다.
그래서 궁금한 점은, 우승자 모델을 단순히 결과 저장용으로 두는 것보다, 우승자를 판별하는 로직까지 책임지도록 하는 것이 더 적절한지입니다.

이 질문을 떠올리면서 리뷰를 진행하다 궁금한 점이 생겼습니다.
태우님께서는 GameManage를 Controller 역할로 정의하고 프로그램을 만들어 주셨나요?
제가 리뷰하면서 본 태우님의 코드 구조는, GameManage보다 Main 메서드가 Controller 역할을 하고 있습니다.
GameManage는 View와 Model 사이를 연결하는 역할보다는 '자동차 경주'를 담당하는 큰 단위의 또다른 Model로 느껴졌는데요,
만약 이 구조를 유도하신 거라면 나쁘지 않은 구조입니다. '자동차 경주' 라는 큰 객체의 하위 객체로 '자동차', '우승자' 객체가 있다고 보면 되니까요.

하지만 GameManage를 Controller로 두신 거라면, 아래와 같은 작업은 확실히 Controller의 역할을 초과합니다.

  • 라운드 진행
  • 우승자 판별
    이 작업은 Controller보다 Model에 들어가는 것이 더 역할에 맞아 보입니다. 우승자 판별을 우승자 객체에서 하는 것이 맞아 보이시면, 그렇게도 해보세요! Controller에 있는 것보단 역할과 책임 측면에서 더 낫다고 생각합니다.


public static int inputRound() {
System.out.println("시도할 회수는 몇회인가요?");
return scanner.nextInt();
Copy link

@BackFoxx BackFoxx Sep 18, 2025

Choose a reason for hiding this comment

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

여기서 숫자를 입력해야 하는데 한글을 입력하면 사용자에게 어떤 문구가 나타날까요?
"횟수는 숫자를 입력해주세요."와 같이 사용자 친화적인 문구를 보여주려면 어떻게 하면 좋을까요?

Copy link
Author

@tae-wooo tae-wooo Sep 21, 2025

Choose a reason for hiding this comment

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

현재 숫자가 아닌 값을 입력하면 사용자에게 별다른 안내 없이 예외 메시지가 길게 출력될 수 있습니다. 따라서 try-catch 구문을 활용해 예외 상황이 발생했을 때 ‘회수는 숫자로 입력해주세요’와 같은 안내 문구를 보여주고, 유효한 숫자가 입력될 때까지 ‘시도할 회수는 몇 회인가요?’라는 질문을 반복하도록 구현하는 것이 좋을 것 같습니다.

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

public static List<String> inputCarName() {

Choose a reason for hiding this comment

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

List라는 자료구조를 택한 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

n대의 자동차 이름을 입력받아야 하는데 문제에서 최대 개수가 정해져 있지 않았습니다. 그래서 크기를 미리 지정할 필요가 없는 List 자료구조를 사용했고, 그 결과 입력된 자동차 이름 수에 맞춰 유연하게 처리할 수 있다고 생각합니다

Copy link

@BackFoxx BackFoxx Sep 21, 2025

Choose a reason for hiding this comment

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

Set이라는 자료구조와 비교하면 어떤 자료구조가 더 나을까요?
Set도 해시테이블이 동적으로 늘어나므로 크기 지정이 필요없고,
중복 허용을 하지 않으니 동일한 자동차 이름으로 자동차를 만들지 못하게 막을 수도 있을겁니다.

Copy link
Author

Choose a reason for hiding this comment

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

Set 자료구조는 중복을 자동으로 걸러주지만, List는 중복된 이름을 개발자가 직접 처리해야 합니다.
하지만 이 문제의 입출력을 보면 입력한 순서대로 결과가 출력되고 있음을 확인할 수 있습니다.
즉, 기본 Set을 사용하면 순서가 보장되지 않으므로 List를 사용하는 편이 더 적절하다고 판단합니다.
물론 LinkedHashSet을 사용하면 순서는 보장되지만, 중복이 입력됐을 때 사용자에게 에러 메시지를 제공하지는 않습니다.
따라서 중복 입력에 대한 피드백까지 직접 구현하려면 List가 더 적합하다고 생각합니다

Choose a reason for hiding this comment

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

자바에서 제공하는 자료구조를 잘 이해하고 계시네요! 멋진 답변이었습니다.

public static List<String> inputCarName() {
System.out.println("경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분).");
String inputName = scanner.nextLine();
return Arrays.asList(inputName.split(","));

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.

쉼표 사이에 공백이 있는 경우에는 자바가 제공하는 문자열 치환 메서드를 활용하면 될 것 같습니다. replaceAll(" ", "")을 사용함으로써 입력값에서 공백을 모두 제거해 ‘강아지,고양이,얼룩말’처럼 만들 수 있습니다.

Choose a reason for hiding this comment

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

저걸 쓰면 "귀여운 강아지" 처럼 한 이름에 띄어쓰기가 있는 경우 "귀여운강아지"로 변형될 위험이 있지 않을까요?

Copy link
Author

@tae-wooo tae-wooo Sep 22, 2025

Choose a reason for hiding this comment

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

replaceAll(" ", "")을 사용하면 "귀여운 강아지"처럼 한 이름 안에 있는 공백까지 모두 제거돼 "귀여운강아지"로 변형될 위험이 있습니다.
그래서 공백 전체를 없애기보다는 replaceAll(",\s*", ",")처럼 쉼표 뒤의 공백만 없애도록 수정했습니다.
이렇게 하면 "귀여운 강아지, 고양이"라는 입력이 "귀여운 강아지,고양이"로 바뀌어 이름 내부의 공백은 유지하면서 쉼표 뒤 공백만 제거할 수 있습니다.

리뷰어님이라면 replaceAll을 사용하지 않고 어떤 방법을 쓰실지 궁금합니다.

Choose a reason for hiding this comment

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

String에서 trim()이라는 메서드를 제공합니다. 사용해보세요!

List<String> carNames = InputView.inputCarName();
int roundCount = InputView.inputRound();

Cars cars = Cars.carsCreate(carNames);

Choose a reason for hiding this comment

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

https://tecoble.techcourse.co.kr/post/2020-04-26-Method-Naming/
협업 환경에서 프로그래밍을 할 때 범용적으로 채택하는 네이밍 룰(Naming Rule)이 있습니다.
위 링크를 참고해서, 메서드 이름을 네이밍 룰에 알맞게 바꾸어보세요.

Copy link
Author

Choose a reason for hiding this comment

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

읽어본 결과, 동사+명사 형태로 메서드 이름을 작성하는 것이 맞다고 판단하여 이 형식에 맞게 변경했습니다.

}


public void setWinners(List<String> winnerNames) {

Choose a reason for hiding this comment

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

setWinners는 넘어온 인자로 winners의 원소를 모두 대체하고, getWinners는 winners를 그대로 반환합니다.
그렇다면 그냥 winners 필드를 public 접근자로 선언한 것과 차이가 있나요?

Copy link
Author

@tae-wooo tae-wooo Sep 21, 2025

Choose a reason for hiding this comment

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

처음 Winners를 설계할 때는 스터디에서 배운 캡슐화를 실습해 보고자 했습니다. 그래서 정적 팩토리 메서드를 통해 객체를 생성하고, 데이터를 외부에서 직접 변경하지 못하도록 설계했습니다. 필드를 그대로 노출하면 외부에서 임의로 접근·변경이 가능하기 때문에, Winners 내부에서 스스로 상태를 관리하도록 하고 싶었습니다.
다만 현재는 getWinners() 메서드를 통해 내부 리스트가 그대로 반환되어 외부에서 수정할 수 있는 상태입니다. 이와 관련된 부분이 한 곳 더 있어서 그 부분에 이어서 적겠습니다.

int roundCount = InputView.inputRound();

Cars cars = Cars.carsCreate(carNames);
Winners winners = Winners.winnersCreate();

Choose a reason for hiding this comment

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

게임 시작 이전에 우승자 객체가 먼저 만들어져 있습니다.
지금 이 16번째 라인에서 만들어진 Winners 객체는 생성 당시 우승자 정보를 갖고있지 않고, 시간이 지나 game.race() 작업이 끝난 후에야 우승자 정보를 갖게 됩니다.

태우님께서 만드신 Winners라는 객체는 특정한 '값'을 나타내는 '값 객체(Value Object, VO)'로써의 역할을 할 것으로 기대되었으나, 실제로 값 객체의 역할을 하지 못하고 있습니다.
객체가 완전한 상태로 생성되게 하려면, 게임이 끝났을 때 게임의 결과로써 Winners를 만들고 반환하도록 하면 더 자연스러운 구조가 되지 않을까요?

Copy link
Author

Choose a reason for hiding this comment

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

getWinners() 메서드 때문에 객체가 완전한 상태를 보장하지 못했는데, 말씀해주신 구조라면 완전한 객체로 만들 수 있다는 점에 저도 동의합니다. 그래서 게임이 끝난 뒤 우승자 목록을 받아 Winners.of(…)로 객체를 생성·반환하도록 리팩토링했고, 이 변경으로 Winners가 항상 완전한 상태로 생성되고 외부에서 수정할 수 없게 되어 처음 의도했던 설계 방향을 지킬 수 있게 되었습니다.

.collect(Collectors.toList());

winners.setWinners(winnerNames);
}

Choose a reason for hiding this comment

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

우승자를 찾아내는 코드군요. 이 방식이 가장 흔하고 직관적이지만, 자바의 stream을 활용하면 더 새로운 방법으로 우승자 그룹을 찾는 것도 가능합니다.
자바의 stream()을 활용해서 다음 방식으로 우승자를 찾아내보세요.

  • 자동차들을 Position 별로 그룹화해 보세요. Map<Integer, List>을 응답하면 됩니다. (Collectors.groupingBy 활용)
  • 위에서 만든 Map에서 Key값을 비교해 가장 높은 Position을 가진 자동차 그룹을 찾아내보세요. (max, map 활용)

stream의 유용한 메서드를 골고루 활용해볼 수 있는 방법입니다. 해보세요!

Copy link
Author

Choose a reason for hiding this comment

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

말씀해주신 방식대로 Collectors.groupingBy(Car::getCarPosition)으로 position별 Map<Integer, List>를 만들고, entrySet().stream().max(Map.Entry.comparingByKey())로 가장 높은 position의 그룹을 뽑아 우승자 이름 리스트를 생성해 보았습니다. 찾아보니 다양한 구현 방법이 있던데, 리뷰어님께서는 이런 경우 어떤 방식을 선호하시는지 궁금합니다!

Choose a reason for hiding this comment

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

this.winners = cars.stream().filter(car -> car.getCarPosition() == maxPosition()).map(Car::getCarName).collect(Collectors.toList());

이 코드는 cars 리스트를 3번 순회합니다.

  1. cars 중 최대 position를 찾을 때 (전체 cars 대상)
  2. cars 중 최대 position를 가진 car를 찾을 때 (전체 cars 대상)
  3. 2에서 찾은 car로부터 이름만 추출할 때 (2 결과값 크기만큼)

위 코드는 cars의 수가 많아짐에 따라 순회하는 리스트의 크기가 정비례로 늘어납니다. 그래서 자동차가 아주 많다면 연산속도에서 불리할 수 있습니다.

List<Car> winnersGroup = carsByPosition.entrySet().stream()
                .max(Map.Entry.comparingByKey())
                .map(Map.Entry::getValue)
                .orElse(Collections.emptyList());

List<String> winnerNames = winnersGroup.stream()
                .map(Car::getCarName)
                .collect(Collectors.toList());

이 코드도 cars 리스트를 3번 순회합니다. 단, 순회하는 대상의 크기가 다릅니다.

  1. position 별로 그룹핑할 때 (전체 cars 대상)
  2. 최대 position을 찾을 때 (position 개수만큼)
  3. 2에서 찾은 car로부터 이름만 추출할 때 (2 결과값 크기만큼)

위 코드는 cars의 수가 많아질 때, position의 다양성에 따라 2번에서 순회하는 리스트의 크기가 달라질 수 있습니다. 만약 모든 자동차가 동일한 position을 갖거나 소수의 position에 몰려있으면 순회하는 크기가 훨씬 작습니다. 반면 모든 자동차의 position이 다른 경우, 1번 코드와 동일한 연산 속도를 보일 것입니다.

그래서 만약 제가 큰 규모의 자동차 경주 프로젝트를 만든다면, 2번 코드를 사용하지 않을까 싶어요!

import util.Generater;


public class Car {

Choose a reason for hiding this comment

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

domain 패키지에도 Car가 있고 racingcar 패키지에도 Car가 있네요?

Copy link
Author

Choose a reason for hiding this comment

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

전에 racingcar 패키지에서 Car를 만들었었는데, 이번 미션을 진행하면서는 처음부터 설계 방향을 다시 잡고자 새로 구현했습니다. 그래서 이전 미션에서 생성된 Car가 깃허브에 남아 있는 것 같습니다.

@tae-wooo
Copy link
Author

안녕하세요 태우님! 1, 2단계 PR을 구경한 결과 아직 자바 문법이 익숙하지 않으신 것 같아, 자바 문법을 더 초점을 두고 리뷰를 드렸습니다.

제가 MVC 패턴을 공부하면서 느낀 점은, Controller는 View에서 받은 정보를 Model에 전달하고, Model과 Model 간의 연결을 조율하며, 다시 Model에서 처리한 결과를 View에 전달하는 역할이라는 것입니다. 그래서 GameManage를 설계할 때, 경기 진행과 우승자 판별 로직을 Controller가 담당하도록 했습니다. 그런데 이렇게 하다 보니 Controller가 맡은 책임이 많아진다는 생각이 들었습니다.
그래서 궁금한 점은, 우승자 모델을 단순히 결과 저장용으로 두는 것보다, 우승자를 판별하는 로직까지 책임지도록 하는 것이 더 적절한지입니다.

이 질문을 떠올리면서 리뷰를 진행하다 궁금한 점이 생겼습니다. 태우님께서는 GameManage를 Controller 역할로 정의하고 프로그램을 만들어 주셨나요? 제가 리뷰하면서 본 태우님의 코드 구조는, GameManage보다 Main 메서드가 Controller 역할을 하고 있습니다. GameManage는 View와 Model 사이를 연결하는 역할보다는 '자동차 경주'를 담당하는 큰 단위의 또다른 Model로 느껴졌는데요, 만약 이 구조를 유도하신 거라면 나쁘지 않은 구조입니다. '자동차 경주' 라는 큰 객체의 하위 객체로 '자동차', '우승자' 객체가 있다고 보면 되니까요.

하지만 GameManage를 Controller로 두신 거라면, 아래와 같은 작업은 확실히 Controller의 역할을 초과합니다.

  • 라운드 진행
  • 우승자 판별
    이 작업은 Controller보다 Model에 들어가는 것이 더 역할에 맞아 보입니다. 우승자 판별을 우승자 객체에서 하는 것이 맞아 보이시면, 그렇게도 해보세요! Controller에 있는 것보단 역할과 책임 측면에서 더 낫다고 생각합니다.

처음에는 Controller가 객체들 사이를 원활하게 연결하는 로직을 담당한다고 생각했습니다.
하지만 공부를 다시 해보니 핵심 로직이나 상태 관리는 Controller가 아니라 Model이 담당하는 것이 더 적절하다는 것을 알게 됐습니다.

MVC 패턴에서 Controller의 주된 역할은 사용자의 입력(요청)을 받아 적절한 Model을 호출하고, 그 결과를 View로 전달하는 중개자 역할이라고 이해하고 있습니다.
즉, Controller는 비즈니스 로직을 직접 처리하기보다는 Model과 View를 연결·조정하는 데 초점을 두어야 한다고 생각하게 됐습니다.

이 미션에서는 Main이 Controller의 역할에 더 가깝다고 판단하는데, 제 이해가 맞을까요?
GameManage를 Controller가 아닌 Model로 본다면, 우승자 판별 로직도 최종 우승자만 저장하는 모델(Winners)보다 GameManage에서 담당하는 편이 더 자연스럽다고 생각합니다.

@BackFoxx
Copy link

BackFoxx commented Sep 24, 2025

이 미션에서는 Main이 Controller의 역할에 더 가깝다고 판단하는데, 제 이해가 맞을까요?
GameManage를 Controller가 아닌 Model로 본다면, 우승자 판별 로직도 최종 우승자만 저장하는 모델(Winners)보다 GameManage에서 담당하는 편이 더 자연스럽다고 생각합니다.

판단?이라는 단어를 사용하셔서 약간 아리송합니다. 태우님 본인께서 Main이 Controller 역할을 하도록 '의도' 하고 코딩하신 건지가 중요하지요. 만들어놓고 보니 '어라, Main이 Contoller 역할을 하고 있네'가 되면 안됩니다. Main이 Controller 역할을 하고 GameManage가 Model 역할을 하도록 의도하고 코딩하셨나요?

public Winners race(int round) {
for (int i = 0; i < round; i++) {
raceOneRound();
ResultView.printRoundResult(cars.getCars());

Choose a reason for hiding this comment

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

만약 GameManage가 Model 역할을 담당하고 있다면, Model에서 View를 직접 사용하고 있는 이 코드는 다소 역할에 맞지 않습니다. 데이터와 View 사이의 연결은 Controller의 역할이니까요.

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