[클린코드 7기 김동진] 자동차 경주 미션 STEP 4 #318

terrydkim · 2025-02-12T12:53:37Z

안녕하세요 광열님!! 좋은 밤입니다 :>

STEP4 구현 목록

자동차 경주 횟수는 사용자에게 입력 받는다.
자동차 경주는 사용자 입력이 없다면 5회로 고정하여 진행한다.
전진하는 조건은 0에서 9 사이에서 무작위 값을 구한 후 무작위 값이 4 이상일 경우이다.

구현하면서 신경쓴 부분

관심사 분리
의존성 분리
새로운 구현보다 피드백 주신 부분들을 어떻게 고칠까 제일 많이 고민했습니다!

beforeEach 사용

beforeEach 사용 시 가독성이 괜찮은지 고민하고 있습니다. it 코드 안에서 given / when / then 보여야 가독성이 좋다고 생각합니다.

테스트 코드가 많이 길어질 때 beforeEach를 사용하면 안에 작성한 코드를 머리속에 넣고 it 테스트 코드를 읽어가는 것이 읽는 이에게 부담이 되지 않을까 생각합니다.

읽는 이의 부담과 중복 코드 둘의 단점을 보완하기 위해 describe를 작은 단위로 쪼개서 beforeEach를 돌리는 것이 좋은지, 중복이더라도 한눈에 알아볼 수 있도록 it에 넣어야 할 지 고민입니다.

피드백 답변

main이라는 이름이 다소 모호하기 때문이 아닐까요? 그리고 고민되는 부분을 좀 더 구체적인 맥락으로 알려주시면 좋을 것 같아요~!

mvc구조를 이론으로만 알고 정확히 이해하지 못하고 있었습니다. 저는 domain을 model로 view를 input, output으로 controller를 main이라고 생각하고 작성했고, 그래서 main에는 생성과 부여를 하는 역할만 할 수 있도록 코드를 구성하려고 했습니다.

재사용 상수나 변수는 constants.js 파일로 분리했습니다. ae072b3
에러 핸들링은 main과 input 에서 try - catch 추가 했습니다. input에서 에러 처리 하는 것이 view에 역할에 벗어난 건 아닌지 고민이 됩니다. fbfce55 7020410

이 값을 Car 클래스에서 가지고 있어야 하는 이유가 무엇일까요? 도메인 로직과 뷰 로직은 분리되어야 할 것 같아요!

제일 많은 고민을 했던 부분입니다!! fbfce55
1. OutputView에서 에러 메세지를 정의하고 Car 클래스에서 사용하려고 하니 의존성이 생기는 것 같아서 제외했습니다. ex. throw new Error(Output.errorMessage)
2. constants.js 파일에 에러 메세지를 작성하였습니다. 하지만 이것도 의존성이 생기는 것이 아닌가 의문이 듭니다. 이런 경우에도 의존성이 있다고 보는지 궁금합니다!

이 로직은 1) 게임을 진행하고 2) 결과를 저장하는 관심사를 모두 알고 있는 것 같네요! 어떤 식으로 하나의 관심사에 집중하게 만들 수 있을까요?

결과를 저장하는 메서드를 생성하여 분리했습니다. 7306715

커밋 링크를 걸어 놓았지만 진행하면서 변경된 부분들이 있습니다 ㅠㅠ 다음부터는 커밋 단위를 잘 나눠서 작성하겠습니다. 감사합니다!

input 사용자 입력 에러 핸들 & split 함수 분리 output 주석 제거 및 함수명 통일

- main 파일에서 try-catch 에러처리 \n-output 에러 메세지 출력 함수 추가

- Input 함수들 역할에 알맞은 이름 변경

- 자동차 경주 횟수는 입력이 없다면 5회로 고정하여 진행한다.

terrydkim · 2025-02-13T11:27:06Z

src/view/Input.js

+    try {
+      const input = await readLineAsync(
+        "경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분). \n"
+      );


비동기처리에 대한 에러 처리가 이렇게 하는 것이 옳게된 경우인지 궁금합니다!!
main에서 에러 핸들링 하기 위해서 catch에서 에러를 던졌는데 괜찮은 로직인지 아니면 바로 아래에 있는

if (!this.isValidNotSpace(carNames)) { throw new Error(ERROR_MESSAGES.NOT_SPACE_IN_NAME); }

이 부분을 main쪽이나 domain쪽으로 빼는 것도 고려를 했습니다!

별다른 처리없이 상위로 다시 던질 에러라면, 여기서는 굳이 try-catch로 감싸지 않고 실제 에러를 처리할 부분에서 에러를 잡아 처리해주면 될 것 같습니다!

igy95 · 2025-02-13T22:38:28Z

constants.js 파일에 에러 메세지를 작성하였습니다. 하지만 이것도 의존성이 생기는 것이 아닌가 의문이 듭니다. 이런 경우에도 의존성이 있다고 보는지 궁금합니다!

어떠한 코드에서 필요한 의존성을 참조해 가지는 것은 자연스러운 것 같습니다. 다만 말씀드린 맥락은 본인의 관심사가 아님에도 구체적인 맥락을 아는 것이 맞는가?에 대한 질문이었습니다 !

igy95

안녕하세요 동진님! 이번 단계부터는 핑퐁을 어느정도 이어가면 좋을 것 같아 rc 요청 드렸습니다~ 코멘트 확인 부탁드릴게요 :)

igy95 · 2025-02-13T22:19:10Z

src/domain/Car.js

+  getRandomNumber() {
+    return Math.floor(Math.random() * NUMBERS.MAX_RANGE);
+  }


랜덤한 숫자를 얻는 동작은 특정 도메인과 연관되기보다 유틸의 성격이 더 강하지 않을까요? 따로 분리하는 게 어떨까요?

igy95 · 2025-02-13T22:21:32Z

src/domain/Car.js

@@ -22,9 +23,17 @@ class Car {
    this.#location += 1;
  }

+  movingCondition() {
+    return this.getRandomNumber() >= NUMBERS.THRESHOLD;


NUMBERS라는 이름과 접근자명은 다소 포괄적으로 보이는 데요,

NUMBERS.THRESHOLD - '차가 움직일 수 있는 조건'의 역할을 가지고 있는데, 이것이 이름에 내포되어 있지 않아 사용처에서 어떻게 사용되는지로만 유추할 수 있어 보여요.
NUMBERS.MAX_RANGE - 마찬가지로 어떠한 부분에 대한 max range인지 선언부만 봐서는 알기 힘들지 않을까요?

igy95 · 2025-02-13T22:25:45Z

src/domain/Race.js

      this.moveCars(this.cars);
-
-      this.result.push({
-        round: round,
-        cars: this.cars.map((car) => ({
-          name: car.getName(),
-          location: car.getLocation(),
-        })),
-      });
+      this.recordRoundResult(round, this.cars);


moveCars, recordRoundResult에서도 this.rounds, this.cars에 접근할 수 있는데, 인자를 따로 받는 이유는 무엇일까요?

igy95 · 2025-02-13T22:29:30Z

src/main.js

+    const raceResult = race.start();
+    const raceWinners = race.getWinners();


raceResult에 winners까지 반환받을 수 있지 않을까요?

igy95 · 2025-02-13T22:30:13Z

src/utils/constants.js

@@ -0,0 +1,25 @@
+export const ERROR_MESSAGES = {
+  INVALID_CAR_NAME: "[ERROR] 자동차 이름은 1자 이상 5자 이하이어야 합니다.",


1, 5와 같은 데이터는 리터럴 내에서도 상수화 할 수 있을 것 같아요 ! 템플릿 리터럴을 확인해보면 좋을 듯 하네요

igy95 · 2025-02-13T22:30:55Z

src/utils/constants.js

+  INPUT_ASYNC_ERROR: "[ERROR] 인풋 비동기화 처리중 에러 발생.",
+};
+
+export const CAR_NAMES = {


테스트 코드에서만 필요한 상수로 보이는데, 프로덕션 코드가 아닌 테스트 코드만 위한 레이어는 __tests__ 안에 두어 용처를 구분 짓는 게 명확할 듯 합니다.

igy95 · 2025-02-13T22:32:02Z

src/utils/constants.js

+export const ROUNDS = {
+  ONE: 1,
+  FIVE: 5,
+  TEN: 10,
+};


이러한 상수는 값이 가지고 있는 의미에 비해 상수명이 주는 이점이 없기 때문에 그냥 값으로 써도 무방할 것 같습니다.

igy95 · 2025-02-13T22:36:30Z

src/view/Input.js

+    try {
+      const input = await readLineAsync(
+        "경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분). \n"
+      );


별다른 처리없이 상위로 다시 던질 에러라면, 여기서는 굳이 try-catch로 감싸지 않고 실제 에러를 처리할 부분에서 에러를 잡아 처리해주면 될 것 같습니다!

igy95 · 2025-02-13T22:48:39Z

+ beforeEach 사용 관련 답변

beforeEach 사용 시 가독성이 괜찮은지 고민하고 있습니다. it 코드 안에서 given / when / then 보여야 가독성이 좋다고 생각합니다.

그 말씀도 일리가 있는 부분이라고 생각합니다. beforeEach를 읽으러 매번 위로 올라가는 것도 어느정도 번거롭긴 하겠죠. 이 부분은 작성자의 판단에 맡길 수 있는 부분인 것 같아요. 만일 반복되는 부분이 길어져서 코드의 중복이 더 우려되는 상황이라면 beforeEach를 쓰고 그게 아니라면 그대로 사용해도 괜찮지 않을까 싶네요 !

terrydkim · 2025-02-14T07:58:09Z

광열님 감사합니다!!
남겨주신 리뷰들은 다음 미션들 중간중간 리팩토링하고 요청드리겠습니다!!

igy95 · 2025-02-18T00:38:40Z

광열님 감사합니다!! 남겨주신 리뷰들은 다음 미션들 중간중간 리팩토링하고 요청드리겠습니다!!

혹시 다음 미션을 위해 현재 PR 머지가 필요하다면 리뷰 재요청 부탁드릴게요 !

terrydkim added 14 commits February 11, 2025 23:02

Refactor: 테스트 코드 중복 변수들 전역 변수화 & 에러 메세지 상수화

ae072b3

Refactor: readline 분리 및 매직넘버 상수화 & view 로직 추가

5711c6a

input 사용자 입력 에러 핸들 & split 함수 분리 output 주석 제거 및 함수명 통일

Refactor: view 폴더 위치 변경 및 에러 처리

fbfce55

- main 파일에서 try-catch 에러처리 \n-output 에러 메세지 출력 함수 추가

Refactor: race start 함수 관심사 분리

7306715

Feat: Input 테스트 코드 추가 및 상수 분리

a3f90fa

Docs: Step4 요구사항 추가

5586943

Env: eslint no-unused-vars warn 변경

4e199a9

Feat: Input 비동기 에러 처리 추가

7020410

Feat: 자동차 경주 횟수는 사용자에게 입력 받는다

e86c2de

- Input 함수들 역할에 알맞은 이름 변경

Fix: 적합한 에러 메세지 출력 수정

39687df

Feat: 사용자 라운드 입력 trim 함수 추가 및 테스트 케이스 추가

30e9716

- 자동차 경주 횟수는 입력이 없다면 5회로 고정하여 진행한다.

Style: 상수명 변경

ecab91f

Feat: 자동차 전진 조건 추가

b62b20b

fix: 주석 제거

54cc93a

terrydkim commented Feb 13, 2025

View reviewed changes

igy95 self-requested a review February 13, 2025 22:15

igy95 requested changes Feb 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[클린코드 7기 김동진] 자동차 경주 미션 STEP 4 #318

[클린코드 7기 김동진] 자동차 경주 미션 STEP 4 #318

terrydkim commented Feb 12, 2025

terrydkim Feb 13, 2025

igy95 Feb 13, 2025

igy95 commented Feb 13, 2025

igy95 left a comment

igy95 Feb 13, 2025

igy95 Feb 13, 2025

igy95 Feb 13, 2025

igy95 Feb 13, 2025

igy95 Feb 13, 2025

igy95 Feb 13, 2025

igy95 Feb 13, 2025

igy95 Feb 13, 2025

igy95 commented Feb 13, 2025

terrydkim commented Feb 14, 2025

igy95 commented Feb 18, 2025 •

edited

Loading

		const raceResult = race.start();
		const raceWinners = race.getWinners();

		@@ -0,0 +1,25 @@
		export const ERROR_MESSAGES = {
		INVALID_CAR_NAME: "[ERROR] 자동차 이름은 1자 이상 5자 이하이어야 합니다.",

[클린코드 7기 김동진] 자동차 경주 미션 STEP 4 #318

Are you sure you want to change the base?

[클린코드 7기 김동진] 자동차 경주 미션 STEP 4 #318

Conversation

terrydkim commented Feb 12, 2025

STEP4 구현 목록

구현하면서 신경쓴 부분

beforeEach 사용

피드백 답변

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igy95 commented Feb 13, 2025

igy95 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

igy95 commented Feb 13, 2025

terrydkim commented Feb 14, 2025

igy95 commented Feb 18, 2025 • edited Loading

igy95 commented Feb 18, 2025 •

edited

Loading