[1단계 - 자동차 경주 구현] 미아(이종미) 미션 제출합니다. #663

jongmee · 2024-02-15T06:46:55Z

안녕하세요! 6기 미아입니다 ☺️

이번 미션은 페어 메이슨과 함께 도메인 별로 로직을 분리해서 관리하고, 깔끔한 코드를 작성하는 것을 중점으로 고민하며 구현했습니다.

기본적으로 MVC 패턴을 사용했습니다.

├── Application.java
├── config
│   └── RacingConfig.java
├── view
│   ├── InputView.java
│   └── OutputView.java
├── controller
│   └── RacingController.java
├── domain
│   ├── Car.java
│   ├── Cars.java
│   ├── RandomNumber.java
│   └──  RandomNumberImpl.java
└── util
    ├── Validator.java
    └── Parser.java

미션을 진행하면서 생각났던 의문들을 여쭤보고 싶습니다!

Validator.class를 util 패키지 내에 위치시켜서 유효성 검증 관련 로직을 모아뒀는데(추후 RacingController.class에서 validation 메서드를 호출함), view 혹은 domain에서 유효성 검증을 진행하는게 더 좋을까요?
domain과 util 패키지 내 작은 메서드 단위부터 테스트하여서 RacingController.class 메서드를 테스트하는 것은 '같은 내용을 중복으로 테스트한다'고 생각하였습니다. controller는 프로그램을 제어하면서 view, domain, util 메서드들을 호출하기 때문입니다. RacingController.class 역시 테스트하는 것이 좋을까요? 그 이유는 무엇일까요?
Validator.class 내에서 자동차 이름 목록 입력을 검증하는 메서드입니다.

    public static void validateCarNames(final List<String> carNames) {
        for(String carName: carNames) {
            validateCarNamesFormat(carName);
        }
        validateDuplicatedNames(carNames);
    }

    private static void validateCarNamesFormat(final String carName) {
        if (carName.isBlank() || carName.isEmpty() || carName.length() > 5) {
            throw new IllegalArgumentException("자동차 이름은 1자 이상 5자 이하여야 합니다.");
        }
    }

    private static void validateDuplicatedNames(final List<String> carNames) {
        final Set<String> uniqueNames = Set.copyOf(carNames);
        if(uniqueNames.size() != carNames.size()) {
            throw new IllegalArgumentException("자동차 이름은 중복될 수 없습니다.");
        }
    }

단위 테스트를 하려고 보니 다른 목적(중복이름, 글자수 검사)으로 public 메서드인 validateCarNames를 여러번 테스트할 수 밖에 없었는데요 🥲 이 또한 적절한 단위 테스트라고 볼 수 있을까요? 테스트만을 위해 public 메서드로 변경하는 것은 좋지 않은 것 같아 이렇게 구현하게 되었습니다.

감사합니다!

- 모든 자동차들이 조건에 따라 움직이는지, 올바르게 단일 우승자를 선정하는지 테스트함

jamie9504

안녕하세요, 미아 😃. 리뷰어 제이미입니다.

우테코 첫 PR 축하드려요 🎉

코드를 작성할 때 미아의 의견들이 궁금하기도 하고
생각을 해보면 좋을 것 같은 부분에 대해 코멘트를 조금 남겨두었어요.

질문주신 부분은 아래에 답변 남겨둘께요.

코드쪽 코멘트에 관련 부분 달아두었어요.
RacingController.class를 테스트했을 때의 단점을 적어주었는데요. 그러면 테스트했을 때의 장점은 무엇이 있었나요?
미아가 생각한 장점과 단점 중 어떤 것이 더 의미가 있는 것 같나요?
미아가 이해하는 단위 테스트의 정의는 무엇인가요?
또한 validateCarNames()는 어떤 역할을 가지고 있을까요?

위 부분을 고민해보고 적어주시면 다음 요청 때 추가로 답변드리도록 할께요 😄

jamie9504 · 2024-02-15T10:54:02Z

README.md

 ## 우아한테크코스 코드리뷰

 - [온라인 코드 리뷰 과정](https://github.com/woowacourse/woowacourse-docs/blob/master/maincourse/README.md)
+


깔끔하게 정리를 잘 해주셨네요 👍

README.md는 미아가 관리하는 문서이므로 사용하지 않는 기존 내용은 제거하는 게 더 깔끔할 것 같아요.

핵심 구현 기능이라는 뜻이 조금 모호하다고 느껴지는데요. 아래 세 가지를 구분한 기준은 무엇일까요?
- 핵심 구현 기능, 입출력 기능, 예외 처리 기능

그렇군요! 제거해야겠어요 😊 감사합니다

입출력(UI) 관련 기능을 제외한, 요구사항을 만족시키는데에 필요한 기능들을 핵심 구현 기능이라고 표현하였는데요..! 예외 처리 기능은 예외 처리는 따로 분리해서 보는게 개발할 때 편하여 분리하였습니다. 처음엔 비즈니스 로직을 생각하며 핵심 구현 기능이라고 이름 지었는데, 비즈니스 로직이 더 직관적일까요? 😂

기능이라는 분류 대신 요구 사항 분석으로 대체하는 것도 추가로 생각해보았습니다

(사람마다 이해하는 부분이 다를 수 있지만) 입출력과 예외처리의 경우엔 기능의 종류별로 구분했다면, 핵심 구현 기능은 핵심이냐 비핵심이냐 중요한 정도에 의해 기능을 구분했다고 보았습니다!

종류별 기능 구분 : 입출력, 예외처리, 비즈니스

중요도별 기능 구분 : 핵심, 비핵심

이 구분한 요구사항 분석이던, 기능이던 분류의 체계가 하나인 것이 읽을 때 직관적인 것 같아요!
이 부분은 위에서 말씀드렸듯이 사람에 따라 다르게 느껴질 수 있는데 미아는 어떤 기준으로 구분을 했던 것인지 궁금했습니다 😆

jamie9504 · 2024-02-15T10:55:58Z

src/main/java/racingcar/Application.java

+        RacingConfig config = new RacingConfig();
+        config.racingController().run();


이 부분만 보았을 때 RacingConfig의 역할은 무엇일까요?
그 역할이 Config라는 이름에 어울리는지 고민해보는 것은 어떨까요!

RacingController도 마찬가지로 어떤 역할을 하고 있는지,
run이라는 메서드와 클래스명이 잘 어울리는지 고민해볼까요?

경주 어플리케이션의 설정이라는 의미에서 Config를 생각했습니다. controller와 view 클래스들을 생성하고 의존성을 주입해 주는 설정이요..!

하지만 요 부분을 보니 설정에서 controller를 생성해서 경주를 시작한다는 게 좀 어색한 거 같네요 .. 경주 어플리케이션을 관리한다는 의미에서 RacingManager로 바꾸어보았습니다

A controller is responsible for controlling the way that a user interacts with an MVC application. (출처: MicroSoft Learn)

Controller는 어플리케이션을 제어하는 역할을 하니까 '실행하다(run)'보다 '시작하다(start)'가 더 적합하다는 생각이 드네요 🤔

이 부분은 여러 부분에서 고민을 해보면 좋을 포인트일 것 같은데요!

RacingManager는 왜 View를 생성해서 Controller에 넘겨주고 있는지에 대해 고민해보면 좋을 것 같아요!
장점이 있어서 분리한 것인지, 혹시 습관적으로 분리를 하고 있는 것은 아닌지요 😄

Application에서 생성하지 않고 RacingManager를 분리한 이유

Controller에서 View를 직접 생성하지 않고 RacingManager에서 생성하여 넘겨준 이유

Controller의 경우에는 말그대로 레이싱컨트롤러 (게임컨트롤러)의 느낌이라면 start도 괜찮다고 생각해요!
그런데 호옥시 MVC의, Spring에서 사용하던 -Controller를 사용한 것이라면 이 controller에서는 start가 어색하다고 느껴지네요 😃
미아가 RacingController의 이름을 지어줄 때 어떤 의미로 지어줬는지 한 번 생각해보면 좋을 것 같아요!

jamie9504 · 2024-02-15T10:58:38Z

src/main/java/racingcar/controller/RacingController.java

+    public RacingController(
+            final InputView inputView,
+            final OutputView outputView) {


미아는 어떤 기준으로 개행을 하고 있나요?

함수의 인자 같은 경우에는 2개 이상이라면 같은 소스 파일의 행들의 길이를 보고 상대적으로 길다 싶으면 개행하고 있습니다

jamie9504 · 2024-02-15T11:10:37Z

src/main/java/racingcar/controller/RacingController.java

+            Validator.validateNullName(carNames);
+            List<String> parsedCarNames = Parser.parseCarNames(carNames);
+            Validator.validateCarNames(parsedCarNames);


이름에 대한 고민들을 해볼까요?

Validator나 Parser의 클래스명을 보았을 때, 해당 클래스는 어떤 일을 하는 것으로 유추할 수 있을까요?
또한 ValidateNullName과 validateCarNames이라는 메서드명만 보았을 때, 이 메서드는 어떤 일을 하는 것으로 유추를 할 수 있을까요?

이 부분에 대해서 고민하다보면 Validator.class를 util 패키지 내에 위치시켜서 유효성 검증 관련 로직을 모아뒀는데(추후 RacingController.class에서 validation 메서드를 호출함), view 혹은 domain에서 유효성 검증을 진행하는게 더 좋을까요? 질문에 대한 미아만의 답변이 될 것 같아요!

유효성 검증(Validator)과 분석(Parser)하는 클래스로 유추될 거 같아요.
validateNullName 메서드는 null로 입력된 무언가의 이름을, validateCarNames 메서드는 자동차의 이름을 검증하는 메서드로 유추할 것 같습니다.

정확하고 구체적으로 하는 일을 유추하기 어렵네요.. 검증하고 분석하는 대상을 쉽게 찾을 수 있으려면, view나 domain으로 메서드 위치를 옮기는게 좋을 거 같네요!

예를 들어 CarNames의 검증 역할이 Controller에 있다면, Cars라는 도메인은 자신의 이름을 검증할 책임이 없게 됩니다.
즉, 다른 사람들이 해당 Cars를 사용할 때는, validate가 되지 않은 이름을 사용해도 무관하게 되는거죠!

반대로 도메인에 있다면, Cars라는 도메인은 자신의 이름을 검증할 책임이 있게 됩니다.
즉, 다른 사람들은 해당 Cars를 사용할 때, 항상 validate가 된 이름을 사용해야하는 의무가 생기는거죠!

이렇게 어디다가 두는 것이 옳다!가 아니라 미아의 의도에 맞춰서 도메인들을 설계해주는 것이 중요합니다.
해당 프로그램에서 과연 이 도메인들은 자신이 어떤 책임들을 가지고 있는 게 좋을지 고민해보는 것도 좋을 것 같네요 😃

(이 부분은 코드를 짜면서 익히는 것도 좋지만, 1레벨 필독서들을 읽으면 또 다른 느낌으로 와닿을 것이라 생각해요!!
심심할 때 틈틈히 읽어놓으면 좋습니다.)

jamie9504 · 2024-02-15T11:15:51Z

src/main/java/racingcar/controller/RacingController.java

+    private void moveCars(final Cars cars) {
+        cars.moveAllCars(new RandomNumberImpl());


moveCars는 몇번 실행될 수 있을까요?
메서드가 실행될 때마다 RandomNumberImpl를 생성하는 것은 의도된 동작인가요?

또한 cars.moveAllCars()는 뭔가 cars가 중복되어 쓰이는 느낌도 있네요 😃

RandomNumber, RandomNumberImpl의 클래스명만 보았을 때 해당 객체는 어떤 것을 나타내는, 어떤 책임이 있는 객체로 유추될까요?

인터페이스를 구현하면 -Impl이라는 이름을 사용하기도 하는데요.
만약 해당 접미사를 사용하지 않는다면 어떤 이름이 잘 어울릴까요?
페어나 크루들끼리 -Impl을 사용하는 네이밍에 대해 의견을 나눠보아도 좋을 것 같아요.

또한 인터페이스를 구현하도록 코드를 짠 이유는 무엇인가요?
의도가 있어서 인터페이스를 분리한 것인지 궁금하네요 😄

moveCars는 유저에게 입력 받은 횟수만큼 실행됩니다. 메서드가 실행될 때마다 보다는 한 경주 게임당 하나의 RandomNumberImpl이 생성되는게 더 좋을 것 같네요!

우선 인터페이스를 분리한 이유는
랜덤한 숫자를 moveAllCars 메서드 내부에서 생성하면 테스트하기 어려워서 인터페이스를 활용하여 테스트용 구현체를 만들고 싶어서였습니다!

RandomNumber 인터페이스는 랜덤한 숫자를 생성하는 객체이기 때문에 RandomNumberGenerator가 더 적합한 이름이 될 것 같네요. Impl 접미사는 다른 크루들과 조금 더 생각해봐야겠습니다. 적절한 이름이 떠오르지 않네요 😭

테스트용 구현체 👍

RandomNumber 인터페이스가 과연 랜덤한 숫자를 생성하는가, 생성하게 강제하고 있는가는 잘 모르겠습니다!
왜냐하면 테스트용 구현체의 경우에는 RandomNumber를 주고있지 않거든요.
물론 구현체는 랜덤하게 주고 있지만요~ 😃

jamie9504 · 2024-02-15T11:17:52Z

src/main/java/racingcar/controller/RacingController.java

+        outputView.printCarPosition(cars);
+    }
+
+    public void run() {


클래스내에 메서드 선언 순서에 대해서도 고민해보면 좋을 것 같네요!
미아는 어떤 기준으로 메서드를 선언했나요?

프로그램이 시작하는 run 메서드에서 호출되는 메서드 순으로 선언했는데, 리뷰를 보고 생각해보니 run 메서드를 상단에 두고 아래에 호출되는 메서드들을 선언하는게 나을 거 같아 수정했습니다 ☺️

jamie9504 · 2024-02-15T11:20:33Z

src/main/java/racingcar/domain/Car.java

+        if (randomNumber > 3 ) {
+            this.count += 1;
+        }


3이 넘으면 Car가 앞으로 가는 것은 자동차가 판단할 역할일까요?

정답은 없지만 이런 저런 고민을 해보는 것도 재미있겠죠 😄
만약 Car가 판단을 하지 않는다면 누가 판단하는 것이 좋을까요?

미아는 위에서 생각한 누군가와 Car 중에 누가 판단하는 것이 더 적절하다고 느껴지나요?

또한 넘겨줄 때는 condition이었는데, 받을 때는 randomNumber네요!
각각 어떤 의미를 담아 만들어주었을까요?

'3아 넘으면 전진한다'는 자동차 자체의 전진 역할일 수 있고, 경주 게임의 설정일 수 있다고 생각합니다! Car가 판단하지 않는다면 경주 게임이 판단하는게 좋을 거 같아요.

저는 조건에 따라 전진하는게 자동차의 내장 기능이라고 생각해서 Car가 판단하는게 적절해보입니다..!

요구사항을 '랜덤숫자가 주어지면 해당 숫자를 조건으로 자동차가 전진한다'로 해석하면서 두 가지를 섞어 써버렸네요 😭 랜덤 숫자는 경주 게임 어플리케이션의 요구 사항이고, 자동차는 '조건'을 보고 전진한다는 판단을 하므로 condition으로 바꾸도록 하겠습니다

미아의 의도대로 도메인을 설계하는 것 좋습니다 👍

프로그램의 의도대로 필요한 도메인에게 역할과 책임을 맡기는 것은 중요합니다.
하지만 프로그램이 알고 있다고 도메인이 모두 프로그램의 정보를 알고 있는 것은 아닙니다.
(ex. randomNumber)

어디까지가 도메인이 알고 있을 부분인지 고민해보는 것은 지금 단계에서 해보면 좋은 고민인 것 같아요.

jamie9504 · 2024-02-15T11:22:47Z

src/main/java/racingcar/domain/Car.java

+    public boolean isAlsoWinner(final Car car) {
+        return car.isSameCount(count);
+    }


자동차라는 객체가 count를 가지고 있는데, 그 count가 같으면 winner가 되는군요!
count와 winner는 이름만 보았을 땐 어떤 상관관계가 있나요?
Car는 다른 차와 비교해서 Winner를 알아야 할 책임이 있을까요?

그리고 위의 isSameCount를 여기서만 사용하고 있는데, 메서드를 꼭 분리할 필요가 있을까요?
메서드를 분리해서 얻을 수 있는 장점이 있는지 고민해보세요.

전진 횟수를 표현하고 싶어서 count를 사용했는데, movement나 position이 더 이해하기 쉬울 거 같습니다. 피드백 감사합니다..!

getter를 사용하기 보다 get한 값을 사용하는 목적을 밝히고 싶어서 isAlsoWinner와 isSameCount를 사용했는데 Car의 책임을 고려하면서 메서드를 만드는게 좋다는 생각을 하지 못했습니다. 객체지향 생활 체조에 getter 지양에 대한 내용이 있는데, 제이미는 이렇게 책임이 모호하다면 getter를 사용하는게 더 좋다고 생각하시나요??

객체지향 생활 체조에 getter에 대한 지양이 나와있죠!
이 부분을 조금 더 알아보는 것도 좋습니다.
키워드를 드려보자면 아래와 같은 것들이요!

getter는 과연 어느 상황에선 지양하고, 어느 상황에서 사용할 수 있는가?

getter를 지양하라고 하는 이유는 무엇일까, 어떤 효과를 불어오기 위해서일까?

또한 같은 car(즉, 같은 객체)의 경우엔 getter를 사용하지 않고 car.count로 가져올 수 있을 것 같아요 😆
이 부분 확인해보세요~!

Car는 자동차인데, 다른 차와 비교해서 Winner를 얻는다! 라는 책임이 Car에게 있는 게 맞는지 고민해보세요.
Car 입장에선 자기가 Winner인지 여부를 알고 있나요? Winner라는 정보를 알 필요가 있을까요?
메서드 시그니처에서 Car의 역할을 유추할 수 있는데, sameMovement인지 판단을 하는게 역할일지 alsoWinner인지 판단을 하는 것이 역할일지 고민해보세요! (메서드 네이밍에 대한 부분입니다.)

jamie9504 · 2024-02-15T11:28:08Z

src/main/java/racingcar/view/InputView.java

+    private static final String CAR_NAMES_INPUT_MSG = "경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분).";
+    private static final String TRY_COUNT_INPUT_MSG = "시도할 회수는 몇회인가요?";


코드에 어떤 부분은 상수로 문자열을 표시하고, 어떤 부분은 직접 문자열을 표시하고 있네요.
미아는 어떤 기준으로 상수를 따로 static으로 선언하였나요?

역할 설명이 필요하다고 생각하는 문자열은 static final 으로 선언하고자 했는데, 추가로 필요한 부분이 있는지 다시 살펴보겠습니다!

jamie9504 · 2024-02-15T11:29:35Z

src/test/java/racingcar/domain/CarTest.java

+        //when & then
+        assertTrue(car1.isAlsoWinner(car2));
+    }
+}


요 이 발생하는 이유는 마지막 줄에 개행이 없어서입니다.
왜 개행이 없으면 저런 표시가 뜰지는 궁금할 때 한 번 알아보시고 개행을 다 추가해주세요!

jongmee · 2024-02-16T14:02:52Z

안녕하세요! 좋은 리뷰 감사합니다 ☺️

리뷰를 반영하여 확실히 수정하고 싶은 부분은 수정하고, 조금 더 고민이 필요한 부분은 일단 남겨두고 코멘트로 답변과 질문을 추가로 남겨 놓았습니다! 그리고 가장 위 코멘트에 대한 답들은 아래에 적어두었습니다!

RacingController.class를 테스트했을 때의 단점을 적어주었는데요. 그러면 테스트했을 때의 장점은 무엇이 있었나요?
미아가 생각한 장점과 단점 중 어떤 것이 더 의미가 있는 것 같나요?

RacingController.class의 메서드를 테스트할 때 장점은 해당 메서드 내에서 호출하는 메서드 순서나, 생성한 예외들이 잘 잡히는지 확인하면서 올바른 동작을 테스트할 수 있을 것 같습니다. 구현해야 하는 어플리케이션의 크기가 커지면 장점이 더 의미있어질 것 같습니다..! 다른 장점도 있을까요??

미아가 이해하는 단위 테스트의 정의는 무엇인가요?
또한 validateCarNames()는 어떤 역할을 가지고 있을까요?

단위 테스트는 작은 단위를 테스트하는 것으로, 그 단위가 메서드인 것이 좋다고 생각합니다! validateCarNames()가 현재 두 개의 역할(자동차 이름 길이와 중복된 이름 검증)을 가지지만, controller에서 각 메서드를 한 번에 호출해주는게 좋다고(깔끔하다고) 생각해서 validateCarNames() 하나로 자동차 이름을 검증했습니다. validateCarNames()은 자동차 이름 목록을 검증하고 있습니다.

jamie9504

안녕하세요, 미아 😄. 리뷰어 제이미입니다.

이번에도 코멘트를 조금 남겨두었어요!

1단계는 이만 머지합니다! 2단계에서 함께 작업해주세요 😉

화이팅 💪

2번 -> RacingController를 테스트하는 경우, 말씀주셨던 장점과 처음에 말씀주셨던 단점이 있을 수 있지요.
미아는 장/단점을 비교했을 때 해당 클래스를 테스트하는 것이 좋다고 생각하시나요, 아니면 굳이 테스트를 할 필요가 없다고 생각하시나요 😃 다음 번에 의견을 적어주세요!

3번 -> 코드에 답변 달아두었습니다.

jamie9504 · 2024-02-16T14:20:46Z

README.md

 ## 우아한테크코스 코드리뷰

 - [온라인 코드 리뷰 과정](https://github.com/woowacourse/woowacourse-docs/blob/master/maincourse/README.md)
+


(사람마다 이해하는 부분이 다를 수 있지만) 입출력과 예외처리의 경우엔 기능의 종류별로 구분했다면, 핵심 구현 기능은 핵심이냐 비핵심이냐 중요한 정도에 의해 기능을 구분했다고 보았습니다!

종류별 기능 구분 : 입출력, 예외처리, 비즈니스

중요도별 기능 구분 : 핵심, 비핵심

이 구분한 요구사항 분석이던, 기능이던 분류의 체계가 하나인 것이 읽을 때 직관적인 것 같아요!
이 부분은 위에서 말씀드렸듯이 사람에 따라 다르게 느껴질 수 있는데 미아는 어떤 기준으로 구분을 했던 것인지 궁금했습니다 😆

jamie9504 · 2024-02-16T14:21:21Z

src/main/java/racingcar/Application.java

+        RacingConfig config = new RacingConfig();
+        config.racingController().run();


이 부분은 여러 부분에서 고민을 해보면 좋을 포인트일 것 같은데요!

RacingManager는 왜 View를 생성해서 Controller에 넘겨주고 있는지에 대해 고민해보면 좋을 것 같아요!
장점이 있어서 분리한 것인지, 혹시 습관적으로 분리를 하고 있는 것은 아닌지요 😄

Application에서 생성하지 않고 RacingManager를 분리한 이유

Controller에서 View를 직접 생성하지 않고 RacingManager에서 생성하여 넘겨준 이유

Controller의 경우에는 말그대로 레이싱컨트롤러 (게임컨트롤러)의 느낌이라면 start도 괜찮다고 생각해요!
그런데 호옥시 MVC의, Spring에서 사용하던 -Controller를 사용한 것이라면 이 controller에서는 start가 어색하다고 느껴지네요 😃
미아가 RacingController의 이름을 지어줄 때 어떤 의미로 지어줬는지 한 번 생각해보면 좋을 것 같아요!

jamie9504 · 2024-02-16T14:22:03Z

src/main/java/racingcar/controller/RacingController.java

+            Validator.validateNullName(carNames);
+            List<String> parsedCarNames = Parser.parseCarNames(carNames);
+            Validator.validateCarNames(parsedCarNames);


예를 들어 CarNames의 검증 역할이 Controller에 있다면, Cars라는 도메인은 자신의 이름을 검증할 책임이 없게 됩니다.
즉, 다른 사람들이 해당 Cars를 사용할 때는, validate가 되지 않은 이름을 사용해도 무관하게 되는거죠!

반대로 도메인에 있다면, Cars라는 도메인은 자신의 이름을 검증할 책임이 있게 됩니다.
즉, 다른 사람들은 해당 Cars를 사용할 때, 항상 validate가 된 이름을 사용해야하는 의무가 생기는거죠!

이렇게 어디다가 두는 것이 옳다!가 아니라 미아의 의도에 맞춰서 도메인들을 설계해주는 것이 중요합니다.
해당 프로그램에서 과연 이 도메인들은 자신이 어떤 책임들을 가지고 있는 게 좋을지 고민해보는 것도 좋을 것 같네요 😃

(이 부분은 코드를 짜면서 익히는 것도 좋지만, 1레벨 필독서들을 읽으면 또 다른 느낌으로 와닿을 것이라 생각해요!!
심심할 때 틈틈히 읽어놓으면 좋습니다.)

jamie9504 · 2024-02-16T14:22:21Z

src/main/java/racingcar/controller/RacingController.java

+    private void moveCars(final Cars cars) {
+        cars.moveAllCars(new RandomNumberImpl());


테스트용 구현체 👍

RandomNumber 인터페이스가 과연 랜덤한 숫자를 생성하는가, 생성하게 강제하고 있는가는 잘 모르겠습니다!
왜냐하면 테스트용 구현체의 경우에는 RandomNumber를 주고있지 않거든요.
물론 구현체는 랜덤하게 주고 있지만요~ 😃

jamie9504 · 2024-02-16T14:22:57Z

src/main/java/racingcar/domain/Car.java

+        if (randomNumber > 3 ) {
+            this.count += 1;
+        }


미아의 의도대로 도메인을 설계하는 것 좋습니다 👍

프로그램의 의도대로 필요한 도메인에게 역할과 책임을 맡기는 것은 중요합니다.
하지만 프로그램이 알고 있다고 도메인이 모두 프로그램의 정보를 알고 있는 것은 아닙니다.
(ex. randomNumber)

어디까지가 도메인이 알고 있을 부분인지 고민해보는 것은 지금 단계에서 해보면 좋은 고민인 것 같아요.

jamie9504 · 2024-02-16T14:23:18Z

src/main/java/racingcar/domain/Car.java

+    public boolean isAlsoWinner(final Car car) {
+        return car.isSameCount(count);
+    }


객체지향 생활 체조에 getter에 대한 지양이 나와있죠!
이 부분을 조금 더 알아보는 것도 좋습니다.
키워드를 드려보자면 아래와 같은 것들이요!

getter는 과연 어느 상황에선 지양하고, 어느 상황에서 사용할 수 있는가?

getter를 지양하라고 하는 이유는 무엇일까, 어떤 효과를 불어오기 위해서일까?

또한 같은 car(즉, 같은 객체)의 경우엔 getter를 사용하지 않고 car.count로 가져올 수 있을 것 같아요 😆
이 부분 확인해보세요~!

Car는 자동차인데, 다른 차와 비교해서 Winner를 얻는다! 라는 책임이 Car에게 있는 게 맞는지 고민해보세요.
Car 입장에선 자기가 Winner인지 여부를 알고 있나요? Winner라는 정보를 알 필요가 있을까요?
메서드 시그니처에서 Car의 역할을 유추할 수 있는데, sameMovement인지 판단을 하는게 역할일지 alsoWinner인지 판단을 하는 것이 역할일지 고민해보세요! (메서드 네이밍에 대한 부분입니다.)

jamie9504 · 2024-02-16T14:25:45Z

src/main/java/racingcar/controller/RacingController.java

+    private final InputView inputView;
+    private final OutputView outputView;
+
+    public RacingController(final InputView inputView, final OutputView outputView) {


각종 곳에 final이 붙어있는데, final을 붙였을 때 어떤 장/단점이 있을지
그에 따라 어디에 final을 붙이는 것이 가장 좋을지 고민해보는 것도 좋습니다.

이 부분은 취향이 갈리는 부분이라 크루들과 이야기를 나눠보는 것도 좋을 것 같아요.
미아만의 기준을 찾아보길 바랍니다.

jamie9504 · 2024-02-16T14:29:15Z

src/main/java/racingcar/domain/Car.java

+        }
+    }
+
+    public boolean isSameCount(final int count) {


movement로 바뀌었지만 요건 count로 남아있네요 😃

jamie9504 · 2024-02-16T14:49:20Z

src/main/java/racingcar/manager/RacingManager.java

+import racingcar.view.OutputView;
+
+public class RacingManager {
+    private InputView inputView() {


메서드명 명명 규칙을 고민해보면, 명사로 선언을 해도 괜찮을까요?
Spring이나 다른 프레임워크의 기반으로 생각하지 않고 Java 프로그램만 생각해서 명명규칙들을 맞춰줘보면 좋을 것 같아요 😃

jamie9504 · 2024-02-16T14:53:33Z

src/test/java/racingcar/util/ValidatorTest.java

+import static org.junit.jupiter.api.Assertions.*;
+
+class ValidatorTest {
+    @Test


단위테스트 - 작은 단위를 테스트하는 것으로, 그 단위가 메서드인 것이 좋다고 생각합니다 라고 보았을 때

작은 단위를 테스트하는 것이지, 메서드가 내부적으로 한 가지의 로직만 맡고 있다는 것은 아닙니다. 😃

validateCarNames의 경우에는 자동차의 이름을 검증한다!라는 하나의 역할을 맡고 있는 것은 맞습니다.
하지만 자동차의 이름을 검증하는 세부 로직이 무조건 하나로 떨어진다는 의미는 아닌 것이죠!

세부 로직이 하나 이상이라면 테스트도 당연히 여러 개가 생성되어야 해당 메서드를 잘 테스트할 수 있겠죠?
테스트를 잘 만들어주셨어요 👍

masonkimseoul and others added 30 commits February 14, 2024 16:19

docs: 구현 기능 목록 작성

eb38c39

feat: 자동차 Domain Class 생성

9b41a8d

feat: 자동차 목록을 저장하는 Domain class 생성

3bb9da8

feat: 랜덤 숫자 생성 Domain class 생성

96ed9f7

feat: 각 시도 실행 결과 출력하는 메서드 생성

bc87041

feat: 우승자 출력 및 안내 메세지 출력 메서드 생성

5641bfa

feat: 문자열 입력 및 파싱 메서드 생성

be081af

feat: 자동차 이름 길이 예외 처리 메서드 생성

5cd7dcf

feat: 중복된 자동차 이름 검증 메서드 생성

8770488

refactor: 자동차 이름 파싱 메서드 utility 패키지로 분리

4f55e91

fix: 자동차 이름 목록 검증 메서드 수정

02fd68a

feat: 시도 회수 검증 메서드 생성

66c07da

refactor: Validation 클래스 이름 변경

c25f173

feat: 컨트롤러 의존성 주입 설정

fa8cf97

feat: Cars 도메인 클래스에서 자동차들을 움직이는 메서드 구현

c12059b

feat: controller에서 입력 및 자동차 이동 기능 구현

9b1eea8

feat: 승리자 결정 메서드 생성

93d4827

feat: 우승자를 정하는 기능 구현

5038390

feat: main 메서드 생성

60f3877

refactor: controller에서 자동차 생성 기능 분리

c6abad0

refactor: outputview에서 실행 결과 출력 방식 변경

044b15a

refactor: Cars 생성자 리팩토링, controller에서 시도횟수 입력 기능 분리

acedb4e

fix: 자동차 이름 입력시 양옆 공백 제거

bb6efbe

test: Car 클래스 테스트 작성

12b2de5

test: Validator 클래스 테스트 작성

b0085b2

test: Parser 클래스 테스트 작성

e368b06

refactor: RandomNumber 클래스를 인터페이스로 변경

c9b5df6

test: Cars 클래스 테스트 작성

3afe80f

- 모든 자동차들이 조건에 따라 움직이는지, 올바르게 단일 우승자를 선정하는지 테스트함

test: Cars에서 다수의 우승자 선정 테스트 작성

4627def

chore: 코드 스타일 정리

8ae2b44

jamie9504 self-requested a review February 15, 2024 10:48

jamie9504 requested changes Feb 15, 2024

View reviewed changes

jongmee added 9 commits February 16, 2024 22:12

refactor: Config에서 Manager로 클래스명 변경, controller run메서드명 변경

b41c3b4

refactor: controller 메서드 순서 변경

0abac6e

refactor: Car 클래스 내 의미가 모호한 변수명 변경

94a373a

refactor: RandomNumber 도메인 클래스명 RandomNumberGenerator로 변경

e889b38

fix: 소스파일마다 개행 추가

0803b83

refactor: Cars 도메인 클래스 메서드명 변경

fd77b3d

docs: README 불필요한 부분 삭제

e49082a

refactor: RandomNumberGenerator를 경주 당 하나만 생성하도록 변경

13d1393

refactor: 역할 설명이 필요한 상수를 static final로 관리하기

2f6c7ef

jamie9504 approved these changes Feb 16, 2024

View reviewed changes

jamie9504 merged commit f9500ea into woowacourse:jongmee Feb 16, 2024

jamie9504 assigned jamie9504 and jongmee Mar 31, 2024

		## 우아한테크코스 코드리뷰

		- [온라인 코드 리뷰 과정](https://github.com/woowacourse/woowacourse-docs/blob/master/maincourse/README.md)

		RacingConfig config = new RacingConfig();
		config.racingController().run();

		private void moveCars(final Cars cars) {
		cars.moveAllCars(new RandomNumberImpl());

		private static final String CAR_NAMES_INPUT_MSG = "경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분).";
		private static final String TRY_COUNT_INPUT_MSG = "시도할 회수는 몇회인가요?";

[1단계 - 자동차 경주 구현] 미아(이종미) 미션 제출합니다. #663

[1단계 - 자동차 경주 구현] 미아(이종미) 미션 제출합니다. #663

Uh oh!

Conversation

jongmee commented Feb 15, 2024

Uh oh!

jamie9504 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jongmee commented Feb 16, 2024

Uh oh!

jamie9504 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!