[그리디] 김예주 자동차 경주 미션 3, 4단계 제출합니다.

[yejoo0310]

안녕하세요 강대현 리뷰어님! 그리디 백엔드 5기 김예주입니다.

자동차 경주 3-4단계 미션 구현했습니다. 잘 부탁드립니다!

고민한 부분

• 처음에는 RacingGame 내부에서 Cars 를 생성해서 관리하게 했는데 이번 단계에서 전체 프로그램을 연결해보니 Cars 객체가 OutputView 에서도 사용돼서 Application 에서 생성해서 사용하도록 수정했습니다.
• 자동차 목록을 외부에서 순회할 때 Cars 의 List<Car> 를 그대로 반환하면 List 라는 구체 타입과 순회 이상의 기능이 노출된다고 생각해서, Iterable<Car> 를 반환하도록 구현했습니다.
• InputView 에서 Scanner를 직접 생성했는데, 그러면 의존성이 숨겨지는 것 같아서 외부에서 주입하도록 수정했습니다. 그래서 입력값 분리와 공백 제거를 테스트할 때 테스트용 문자열로 검증할 수 있었습니다.

리뷰 받고 싶은 부분

• Application 에서 역할별로 메서드를 분리했는데, 이 정도 역할은 Application 에 둬도 괜찮은지, 아니면 별도의 controller 객체로 분리하는 게 더 적절한지 궁금합니다.
• 현재 RacingGame 은 한 라운드 진행을 맡으면서 Cars.move() 를 호출하는 역할이 대부분인데, 이 정도의 역할만 가진 상태에서도 독립적인 도메인 객체로 유지하는 것이 의미가 있는지 궁금합니다.
• Application 에서 라운드와 결과 출력이 반복되는 흐름으로 구성했는데 이러한 책임 분리가 적절한지 궁금합니다.

궁금한 점

• 요구사항에 명시되지 않은 입력 예외를 어느 정도까지 처리하는게 적절한지 궁금합니다.
  • 예를 들어 자동차 이름은 5자 이하여야 한다는 조건만 있는데, 빈 문자열처럼 별도로 언급되지 않은 경우까지 검증하는 것이 좋은지 궁금합니다.

[Kdahyn]

예주님 3, 4단계 잘 구현해주셨습니다! 👍

Cars, NumberGenerator 등의 구조를 그대로 이어가면서 이번 단계에서는 InputView, OutputView를 분리하고 전체 프로그램이 실제로 실행될 수 있도록 잘 확장해주신 것 같아요.

특히 자동차 이름에 대한 규칙을 Car 내부에서 검증한 부분이나, 입력값의 공백까지 고려해서 InputView 테스트를 작성한 부분에서 이번 미션의 학습 목표를 의식하면서 구현하신 게 느껴졌습니다.

이번 리뷰에서는 기능적인 부분 하나와 함께 MVC를 적용하면서 생기는 책임 분리에 조금 더 집중해서 코멘트를 남겨봤어요.

현재 규모에서는 Application이 실행 흐름까지 담당해도 충분히 이해할 수 있는 구조이지만, 이번 미션에서 MVC를 연습하는 만큼 Application과 Controller를 직접 한번 분리해보고 각각 어떤 역할을 가져야 하는지 경험해보면 좋을 것 같습니다.

또 Cars → OutputView로 데이터를 전달하는 과정에서 컬렉션을 보호하는 것과 내부 객체의 상태까지 보호하는 것은 어떻게 다른지, View가 Domain 객체를 어디까지 알아야 하는지도 같이 고민해보면 좋을 것 같아요.

PR에 남겨주신 RacingGame의 역할과 예외 처리에 대한 고민도 좋은 질문이라고 생각합니다. 이번에는 정답을 정하기보다는 “이 객체는 어떤 책임을 가지는가?”, “이 검증은 입력의 책임인가 Domain의 책임인가?”를 기준으로 한번 결정해보시면 좋겠습니다.

세부 코멘트 확인해보시고 예주님이 생각하신 기준이나 다른 의견이 있다면 편하게 남겨주세요! 같이 이야기해보면 좋을 것 같습니다. 😊

[Kdahyn]

반복문 안에서 각 라운드의 결과를 이미 출력하고 있는데, 반복문이 끝난 뒤 printCars()를 한 번 더 호출하고 있어서 마지막 라운드 결과가 두 번 출력되고 있어요!

요구사항에서는 각 라운드의 결과를 한 번씩 출력한 뒤 최종 우승자를 출력하도록 되어 있으니, 실제 실행 결과도 한번 확인해보면 좋을 것 같습니다. 👀

[yejoo0310]

요구사항엔 없지만 실행 결과 예시에서 각 라운드마다 결과를 출력하고, 마지막에 최종 결과를 출력한 것 같아서 한 번 더 출력하게 했습니다.
그런데 최종 결과와 마지막 라운드의 결과가 같으니 각 라운드의 결과만 출력하는게 보기에 더 좋은 것 같아서 말씀 주신대로 수정했습니다!

수정 커밋 - 8835943

[Kdahyn]

현재 Application이 객체를 생성하는 역할뿐 아니라 입력 → 경주 진행 → 각 라운드 출력 → 우승자 출력까지 전체 흐름을 제어하고 있네요.

지금처럼 작은 프로그램에서는 현재 구조도 충분히 이해하기 쉽다고 생각합니다. 다만 이번 미션의 학습 목표가 MVC인 만큼, 연습 삼아 Application과 Controller의 역할을 한 번 나눠보는 것도 좋을 것 같아요.

Application은 프로그램을 시작하고 필요한 객체들을 만들어 연결하고, Controller는 View와 Domain 사이의 실행 흐름을 담당한다고 나눈다면 현재 코드 중 어떤 부분들이 Controller로 이동하는 게 자연스러울까요?

직접 한 번 분리해보고 현재 구조와 어떤 차이가 생기는지도 비교해보면 MVC의 역할을 이해하는 데 도움이 될 것 같습니다!

[Kdahyn]

List<Car> 자체를 그대로 노출하지 않고 Iterable<Car>와 List.copyOf()를 사용해서 외부에서 컬렉션을 직접 변경하지 못하도록 한 의도가 좋네요 👍

여기서 한 단계 더 생각해보면 좋을 것 같습니다.
List.copyOf(cars)가 보호해주는 것은 컬렉션 자체일까요, 그 안에 들어 있는 Car의 상태까지일까요?

현재 OutputView에서는 결국 실제 Car 객체를 전달받고 있는데, View가 출력하기 위해 정말 Car 객체 전체를 알아야 하는지도 같이 생각해볼 수 있을 것 같아요.

물론 Domain 객체를 View에 전달하는 것 자체가 잘못된 것은 아니라고 생각합니다. 필요하다면 Car의 복사본을 전달해서 외부에서 원본 상태를 변경하지 못하게 하는 방법도 있고, View에 필요한 값만 별도의 객체(DTO 등)로 전달하는 방법도 있을 것 같아요.

각 방법이 어떤 차이가 있는지 비교해보고 지금 미션에서는 어느 정도까지 분리하는 게 적절할지 생각해보면 좋겠습니다!

[Kdahyn]

현재 RacingGame의 race()는 cars.move(numberGenerator)를 호출하는 역할만 하고 있네요.
다만 메서드가 한 줄이라는 이유만으로 해당 객체가 필요 없다고 판단할 필요는 없다고 생각해요.

만약 RacingGame을 없애고 Controller/Application에서 바로 cars.move(numberGenerator)를 호출한다면 어떤 책임이나 의미가 사라질까요?

반대로 RacingGame을 유지한다면 Cars와 구분되는 RacingGame만의 역할을 어떻게 설명할 수 있을지도 궁금합니다!

코드의 양보다는 이 객체가 어떤 역할을 표현하기 위해 존재하는가를 기준으로 한번 고민해보면 좋을 것 같아요.

[Kdahyn]

자동차 이름의 길이를 Car가 직접 검증하도록 한 점이 좋았습니다. 자동차가 가져야 하는 규칙을 자동차 스스로 보장하고 있네요 👍

이번 미션에서 예외 처리도 학습하는 만큼, 연습 삼아 요구사항에 명시되지 않은 입력도 몇 가지 생각해보면 좋을 것 같아요.

예를 들면

자동차 이름이 빈 문자열인 경우
경주 횟수에 숫자가 아닌 값이 입력된 경우
경주 횟수가 0이나 음수인 경우

등이 있을 것 같습니다.

이 예외들을 모두 같은 위치에서 검증하는 것이 좋을까요?

각 경우가 입력 형식에 대한 문제인지, Domain이 반드시 지켜야 하는 규칙인지 먼저 나눠보고, 각각 어느 객체에서 검증하고 어떤 예외를 발생시키는 것이 자연스러운지 한번 정해보면 좋을 것 같습니다!

[Kdahyn]

입력 테스트를 꽤 꼼꼼하게 작성해주셨네요!

new Scanner(" car1, car2, car3 ")

처럼 단순히 car1,car2,car3만 테스트하지 않고 앞뒤 공백까지 포함해서 실제로 trim()이 동작하는지 확인하고 있고, new Scanner(" 5 ") 역시 숫자 변환뿐 아니라 공백이 포함된 실제 입력 형태까지 함께 검증하고 있는 점이 좋았습니다.

테스트 이름에서도 어떤 입력을 어떤 형태로 변환하는지 잘 드러나서 테스트만 읽어도 InputView의 역할을 이해하기 쉬웠어요. 👍

[Kdahyn]

이름의 최대 길이를 MAX_NAME_LENGTH라는 상수로 잘 분리해주셨는데, 예외 메시지에서는 다시 5라는 값이 직접 사용되고 있네요.

private static final int MAX_NAME_LENGTH = 5;

throw new IllegalArgumentException(
        "자동차 이름은 " + MAX_NAME_LENGTH + "자 이하여야 합니다."
);

처럼 검증 기준과 메시지가 같은 값을 바라보게 하면 어떨까요?

만약 최대 길이가 변경되었을 때 검증 기준은 바뀌었는데 메시지는 그대로 남는 상황을 방지할 수 있을 것 같습니다.
단순히 숫자를 상수로 만드는 것보다, 왜 이 값이 한 곳에서 관리되어야 하는지도 같이 생각해보면 좋을 것 같아요!

[yejoo0310]

숫자를 상수로 만든 이유는 5 라는 값만 봤을 때 이게 어떤 의미를 가지는지 알기 어렵기 때문에 MAX_NAME_LENGTH 라는 이름을 통해 이름의 최대 글자수 라는 의미를 드러내 가독성을 높이기 위해서였습니다.

또한 같은 값을 여러 곳에서 직접 사용하면 이름 글자 수 요구사항이 변경됐을때 각각의 값을 모두 찾아서 수정해야 하므로, 같은 규칙에서 사용하는 값은 하나의 상수로 관리하는 것이 유지보수에도 더 안전하다는 점을 이해했습니다.

처음에 검증 로직만 생각하고 예외 메시지에서 같은 검증 기준을 표현하고 있다는 점까진 생각하지 못했던 것 같습니다. 피드백 주신 것처럼 메시지에서도 MAX_NAME_LENGTH 상수를 사용하도록 수정하겠습니다!

수정 커밋 - 8269a32

[Kdahyn]

이번 미션 요구사항에는 없었지만, 자동차 이름에 MAX_NAME_LENGTH라는 규칙이 생긴 만큼 사용자에게 입력받을 때도

경주할 자동차 이름을 입력하세요. (이름은 5자 이하, 쉼표 기준으로 구분)

처럼 제한을 미리 알려주면 사용자가 예외를 만나기 전에 올바른 값을 입력하는 데 도움이 될 것 같아요.

그런데 여기서 한 가지 고민할 점이 생길 것 같습니다.
현재 MAX_NAME_LENGTH는 Car가 가지고 있는 도메인 규칙인데, InputView에서도 동일한 5를 직접 작성한다면 나중에 규칙이 변경됐을 때 두 곳을 같이 수정해야 합니다.

그렇다고 InputView가 Car.MAX_NAME_LENGTH를 직접 참조하도록 만드는 것이 가장 좋은 방법일까요?
도메인의 검증 규칙은 한 곳에서 관리하면서 View에서도 그 규칙을 안내해야 한다면 어떻게 전달하는 게 좋을지 한번 고민해보면 좋을 것 같습니다!

이번 미션 규모에서는 꼭 복잡한 구조를 만들 필요는 없지만, MVC에서 Domain과 View의 의존성을 생각해보기 좋은 지점인 것 같아요.

[Kdahyn]

현재 메서드가 다음과 같이 배치되어 있네요.

public move()
private findMaxPosition()
public findWinners()
private addWinner()
public iterateCars()

메서드 순서는 Java 문법에서 정해주는 규칙은 아니고, Java 스타일 가이드에서도 하나의 정답이 있는 부분은 아닙니다.
다만 클래스의 메서드가 많아질수록 어떤 기준으로 메서드를 배치하는지가 코드를 읽는 데 영향을 줄 수 있어요.

예를 들어

public move()
public findWinners()
public iterateCars()

private findMaxPosition()
private addWinner()

처럼 외부에 제공하는 기능을 먼저 보여주고 내부 구현을 아래에 두는 방식도 있고,

public findWinners()
private findMaxPosition()
private addWinner()

처럼 호출되는 기능끼리 가까이 배치하는 방식도 있습니다.

현재 Cars에서는 어떤 기준으로 메서드 순서를 정하셨는지 궁금해요!
정답을 정하기보다는 본인이 읽기 쉽다고 생각하는 기준을 하나 정해서 클래스 내에서 일관되게 적용해보면 좋을 것 같습니다.

[yejoo0310]

저는 호출되는 기능끼리 가까이 배치하는 방식이 코드 흐름을 따라가기에 더 편하다고 생각해서 기능 단위로 메서드를 배치하는 것을 좀 더 선호합니다.

지금 제 코드를 보니 public , private 메서드가 섞여 있고, 어떤 기준으로 배치했는지 명확히 드러나지 않아 코드 흐름을 파악하기 힘들 것 같다는 생각이 들었습니다.

메서드 배치 순서도 코드의 가독성에 영향을 줄 수 있다는 것을 알게 되었습니다. 앞으로는 호출 관계가 있는 메서드들을 가까이 배치하는 기준을 정해서 클래스 내에서 일관되게 적용해보겠습니다!

수정 커밋 - 98d49bc