[홍건우] 자동차 경주 미션 Step1 #6

HongGunWoo · 2024-06-24T10:12:33Z

과제 수행 중 고민했던 점

MVC 패턴을 적용하기
Controller를 구현 중 경기 정보에 대한 값 때문에 Controller도 하나의 Model처럼 구현된 것 같아서 아쉽습니다.
RaceInfo라는 또다른 모델을 만들어서 경기횟수, 가장 긴 이름 길이 등을 넣으려고 했지만 해당 모델이 다른 모델을 참조하는 것처럼 구현이 되는 것 같아 포기하고 Controller에 멤버 변수로 위치시켰던 것 같습니다.
MVC 패턴을 인턴 활동에서도 연습했지만 항상 각각의 역할에 대해 헷갈리는 것 같습니다.ㅠㅠ
Model에 대해
타입스크립트가 적용되었다고 생각하면서 Vehicle은 interface, Car는 Vehicle의 구현체처럼 생각하고 구현하였습니다. 후에 Car 뿐만 아니라 Truck이나 Bike가 추가되어도 쉽게 대응할 수 있도록 하고 싶었습니다.
Factory Pattern
Car, Truck, Bike 등 종류가 변경되어도 race 방식은 크게 변경될 부분이 없다고 생각해 RaceController에서 racer에 대한 객체를 생성할 때 팩토리 패턴을 적용해서 구현하였습니다. 경기 조건이 변경되어도(ex move에 대한 조건) 각 모델의 처리 방식은 모델이 담당하는게 맞으므로 각 모델에서 override를 통해 수정하는게 맞다고 생각하였습니다.

리뷰 받고 싶은 부분

MVC 패턴 적용 중 어떤 부분이 MVC 패턴을 지키지 못하고 있는지 궁금합니다.
controller에서 사용하고 있는 factory pattern의 사용이 적잘한지 궁금합니다.
사용자 입력에 대해 query부분은 view를 통해 출력한 후 바로 입력을 받기 때문에 view에 ask라는 함수로 입력을 받을 수 있도록 하였는데 흐름이 Controller -> View -> User로 볼 수 있습니다. 적절한 구조인지 궁금합니다.
racerType으로 type에 대한 상수값을 분리하였습니다. RacerFactory에 위치시키는 것도 좋다고 생각하는데 어떤 방법이 더 좋은 방식일지 궁금합니다.
util 함수를 특정한 곳에서만 사용하는게 아닌 정말 전역적인 util로 사용될 수 있도록 함수명에 고민이 많았던것 같습니다. 함수명이나 util함수의 구조가 적절한지 궁금합니다.

궁금한 점

Error 메시지나 결과 값 출력에 대해 I18N을 적용한다고 생각하고 constant로 변경하고 싶었지만 메시지 출력에 동적으로 변경되는 변수가 사용되어 분리가 힘들다고 생각하였습니다. 좋은 방법이 있을까요?
RaceController에 대한 test 코드로 잘못된 값이 입력된 경우 반복해서 요청을 제대로 하는지에 대한 테스트 코드를 작성하고 싶었는데 아직 vitest에 대한 공부가 부족해서인지 실패했습니다. mock을 사용하면 될 것 같은데 어떤 방식으로 접근하면 좋을지 궁금합니다.

jkea1

보고 많이 배웠습니다!! 👍👍 수고하셨어요!!

jkea1 · 2024-06-25T07:43:45Z

src/__tests__/Car.test.js

@@ -0,0 +1,37 @@
+import { describe, it, expect, vi } from 'vitest';


아래 처럼 vistest configuration 설정해주면 해당 import 코드를 생략할 수 있어요!

// vitest.config.js import { defineConfig } from 'vitest/config' export default defineConfig({ test: { globals: true, }, })

오 저런 세팅이 있었군요!! 감사합니다!

오 저도 새로 알았어요 ! 감사합니다 🙇🏻‍♀️

jkea1 · 2024-06-25T10:05:47Z

src/__tests__/Car.test.js

+  it('유효하지 않은 이름으로 Car 인스턴스를 생성하려고 하면 오류를 발생시켜야 합니다', () => {
+    expect(() => new Car({ name: 'TooLongName', position: 0 })).toThrow();
+  });


유효하지 않은 이름을 체크할 때 이름의 길이 뿐만 아니라 특수 문자 입력 시의 테스트(자동#차, 자동_차, 자동$차) 라든지, 문자 내 공백 처리에 대한 테스트(자 동차, 자동_차, 자동$차) 혹은 빈 값으로 들어온 경우에 대한 테스트(' ')를 추가해도 좋을 거 같아요.

빈값에 대한 테스트를 빼먹었네요... 감사합니다!

jkea1 · 2024-06-25T10:08:26Z

src/constant/racerType.js

+export const RACER_TYPE = {
+  CAR: 'car',
+};


racer가 꼭 자동차가 아닐 수 도 있겠군요! 확장성 측면에서 너무 좋은 것 같습니다..!

jkea1 · 2024-06-25T10:17:51Z

src/__tests__/Car.test.js

+
+describe('Car 클래스 테스트', () => {
+  it('유효한 이름과 초기 위치로 Car 인스턴스를 생성할 수 있어야 합니다', () => {
+    const car = new Car({ name: 'Test', position: 0 });


{ name: 'Test', position: 0 }; 부분이 자주 반복되는 거 같은데 const validArgs = { name: 'Test', position: 0 }; 와 같이 공통 인수로 뽑아줘도 좋을 거 같아요.

jkea1 · 2024-06-25T10:21:27Z

src/__tests__/random.test.js

+  it('지정된 최소값과 최대값 사이의 숫자를 반환해야 합니다', () => {
+    const min = 1;
+    const max = 10;
+    const randomNumber = getRandomNumber(min, max);


저는 랜덤숫자를 만드는 범위를 고정시켰는데 getRandomNumber 처럼 만들면 나중에 확장하기 편하겠군요..! 배우고 갑니다!

jkea1 · 2024-06-25T10:25:16Z

src/factory/RacerFactory.js

+export class RacerFactory {
+  static createRacer(type, args) {
+    switch (type) {
+      case RACER_TYPE.CAR:
+        return new Car(args);
+      default:
+        throw new Error('Unknown type');
+    }
+  }
+}


jkea1 · 2024-06-25T10:28:03Z

src/controller/RaceController.js

+    if (!isValidateRounds) {
+      throw new Error(
+        `시도 횟수는 ${this.#minRounds} 이상${
+          this.#maxRounds !== Infinity ? `, ${this.#maxRounds} 이하의` : '의'
+        } 정수만 입력 가능합니다.`
+      );
+    }


에러 문구 처리도 구체적으로 해주셨네요..!

jkea1 · 2024-06-25T10:30:03Z

src/util/validate.js

@@ -0,0 +1,17 @@
+export function validateName({ name, minLength, maxLength }) {
+  if (typeof name !== 'string') return false;


저도 타입에 대한 부분을 추가해야 겠네요!

jgjgill

안녕하세요 건우님..!
이렇게 코드로 다시 리뷰를 남기게 되네요.. ㅎㅎ

리뷰 받고 싶은 내용에서 제가 궁금한 부분은 Controller가 많은 역할을 하는 것 같다는 생각이 들었어요..! 원래 MVC 패턴이 Controller 이렇게 구성되는지는 잘 모르겠네요.. 🫠

제가 어렴풋이 듣거나 멘토님의 리뷰를 통해 유추했을 때 현재 Controller는 Model의 동작도 한다는 생각을 했습니다..!

수고 많으셨습니다..! 🙇‍♂️

jgjgill · 2024-06-27T09:07:39Z

src/__tests__/random.test.js

+    const mockRandom = vi
+      .spyOn(Math, 'random')
+      .mockReturnValueOnce(0.1)
+      .mockReturnValueOnce(0.9);


해당 코드가 현재 테스트에 필요한게 맞을까요?? 🤔

랜덤 값을 반환해주는 util함수에 대해 진짜 정말 랜덤 값이 반환되는지에 대한 테스트를 추가해보고 싶었는데 지금 보니 굳이? 라는 생각이 들긴 하네요;;

jgjgill · 2024-06-27T09:09:10Z

src/__tests__/random.test.js

+  it('최소값이 최대값보다 클 때 값을 교환하여 숫자를 반환해야 합니다', () => {
+    const min = 4;
+    const max = 1;
+    const randomNumber = getRandomNumber(min, max);


오오 값을 알아서 비교해주면 네이밍을 min, max로 할 필요가 없을 것 같았어요..! first, second?

지금 생각보니 교환보다는 네이밍을 min, max로 유지하고 min > max 인 값이 들어오면 오류를 반환하는게 더 맞는 것 같기도 하네요

jgjgill · 2024-06-27T09:11:52Z

src/model/Car.js

+  move() {
+    const randomNumber = getRandomNumber(0, 9);
+
+    if (randomNumber >= 4) {


상수로 구분해도 좋을 것 같아요..!

저도 해당 부분을 상수로 구분하고 싶었는데 언제나 그렇듯이 항상 네이밍이 고민되더라구요...

jgjgill · 2024-06-27T09:37:58Z

src/controller/RaceController.js

+    const winnerNames = this.#racers
+      .filter((racer) => racer.position === maxPosition)
+      .map((winner) => winner.name)
+      .join(', ');


MVC 패턴과 관련있는지는 잘 모르겠지만 이 부분과 관련해서 이야기하고 싶었습니다..!
제가 생각했을 때 this.#racers.filter((racer) => racer.position === maxPosition).map((winner) => winner.name) 여기까지 winner에 대한 데이터를 추출하는 부분은 모델에서 구현되어야 할 것 같습니다!
지금은 말씀하신 대로 RaceController가 모델이 된 느낌이네요?? Game같은 모델을 추가해도 괜찮지 않을까? 라는 생각이 들었습니다.

그리고 마지막 join 메서드는 뷰에 대한 관심사 높다고 생각했어요..! 해당 부분은 현재 콘솔 기반이기 때문에 join 메서드로 문자열로 붙여줬는데 컴포넌트였다면 달라졌을 것 같아요!

정리하면 모델을 통해 map 함수까지 만든 데이터를 컨트롤러에서 받고 컨트롤러에서 받은 데이터를 뷰에 주입하는 것이 MVC 패턴?? 이 저의 현재 생각이네요..!

저도 구현 중 controller의 역할이 점점 무너지고 있다는 생각을 했었습니다...
Game 과 같은 모델을 추가하려고 저도 RaceInfo이름을 가진 모델을 추가해서 구현하려다가 구조가 너무 복잡해지는 것 같아 뺐었는데 다시 시도해봐야할 것 같습니다.

남겨주신 내용 중에 join 메소드는 뷰가 담당하도록 하는 것이 더 맞는것 같습니다!🙇

winner 부분은 제가 RaceInfo를 만들면서 고민했던 부분입니다. winner를 알기위해서는 모델이 racer 리스트를 가지고 있어야하는데 그럼 이 과정에서 Model이 또다른 Model의 구조를 알고 정보를 꺼내야하는 방식으로 구현이 되서 MVC 패턴 중 Model의 역할이 깨지지 않을까 고민했어요. MVC 패턴을 더 공부해보고 적절한 방법을 찾아봐야겠네요.

chasj0326

안녕하세요 건우님! 🙌 MVC + 팩토리 패턴을 사용해서 좋은 구조가 만들어진 것 같아요! 고민을 많이 하신게 보입니다.
팩토리 패턴을 써서 그런지, Racer 를 생성하는 부분이 밖으로 노출되지 않아서 깔끔해요 (특히 말씀하신대로 Racer 가 확장되면 더 효과적으로 쓰일 것 같습니다)

보면서 한가지 생각난 점은, 의도하신 것 보다 컨트롤러의 역할이 커졌다는 점입니다 !
저같은 경우, MVC 패턴을 사용해야 할 때 view.print 라고 생각하기보다 view.update 라고 생각하면 좀 더 편했어요. 뭔가 print 라고 생각하면 문자열을 제대로 만들어서 보내줘야할 것 같은 느낌이 들기 때문에 .. ! 허허

그리고 입력을 반복적으로 요청하는 지에 대한 질문을 하셨는데,

inputFn = vi.fn()
      .mockImplementationOnce(() => { throw new Error(...) })
      .mockImplementationOnce(() => Promise.resolve())

이런식으로 inputFn 을 모킹하고, getValidInput 실행을 테스트하면 가능하지 않을까 ? 라는 생각이 듭니다 ! 따라서 마지막에는 setterFn 이 제대로 호출되는지를 보면 될 것 같아요

chasj0326 · 2024-06-27T12:21:12Z

src/__tests__/Car.test.js

@@ -0,0 +1,37 @@
+import { describe, it, expect, vi } from 'vitest';


오 저도 새로 알았어요 ! 감사합니다 🙇🏻‍♀️

chasj0326 · 2024-06-27T12:23:03Z

src/__tests__/Car.test.js

+  });
+
+  it('move 메서드는 randomNumber가 4 이상일 때 위치를 증가시켜야 합니다', () => {
+    const car = new Car({ name: 'Test', position: 0 });


car 를 생성하는 부분이 반복이 되는군요 !
알고 계실 수도 있지만 ... 각각의 테스트마다 실행하는 코드를 미리 작성할 수 있어요 !
beforeEach 라는 테스트 메소드에 대해 찾아보세요!

chasj0326 · 2024-06-27T12:32:05Z

src/controller/RaceController.js

+  async getValidInput(inputFn, setterFn) {
+    while (true) {
+      try {
+        const input = await inputFn();
+        setterFn(input);
+        return;
+      } catch (error) {
+        this.#raceView.printMessage(error.message);
+      }
+    }
+  }


input 을 받아서 set 하는 로직을 이렇게 묶는다는 아이디어가 좋아요 !
다양한 목적의 input 을 받아야하는 경우에 유용한 메소드라는 생각이 듭니다

chasj0326 · 2024-06-27T12:34:25Z

src/controller/RaceController.js

+
+  #printRaceStatus() {
+    this.#racers.forEach((racer) => {
+      const racerName = racer.name.padEnd(this.#maxNameLength, ' ');


오 이름의 길이를 맞춰주셨군요 ! 출력 때 좀 더 깔끔하게 나오게하기 위함인가요 ?

맞습니다 ㅎㅎ 한눈에 깔끔하게 보이면 좋겠다는 생각이 들어서요!

chasj0326 · 2024-06-27T12:36:11Z

src/controller/RaceController.js

+    for (let _ = 0; _ < this.#rounds; _++) {
+      this.#racers.forEach((racer) => racer.move());
+      this.#printRaceStatus();
+    }
+  }


오 이거는 쫌 더 리팩토링해봐도 좋을 것 같습니다 !

chasj0326 · 2024-06-27T12:37:22Z

src/util/random.js

+  if (min > max) {
+    [min, max] = [max, min];
+  }


매개변수를 이렇게 받으면 min, max 순서가 바뀌었을 때의 문제가 사라지네요 👍

이부분은 유틸함수의 역할을 벗어난건 아닌지 생각이 많아지는 코드인것 같네요ㅎㅎ...

JunilHwang and others added 10 commits June 18, 2024 18:38

test: 예제 테스트 추가

d60235c

chore: CI 추가

47dcad8

chore: 기본 테스트 파일 삭제

3c7e4a9

chore: package 파일 린트 적용

f29f87a

feat: Vehicle 모델 구현

ee9f99c

feat: view 구현

d00d0d9

feat: util 함수 구현

15d59b3

feat: Car 모델 구현

9f6df89

feat: Controller 및 factory 구현

b412239

feat: test 코드 작성

c12c303

HongGunWoo added the team2 label Jun 24, 2024

HongGunWoo self-assigned this Jun 24, 2024

jkea1 approved these changes Jun 25, 2024

View reviewed changes

JunilHwang merged commit c3cae79 into fe-clean-code-study:HongGunWoo Jun 26, 2024
1 check passed

jgjgill reviewed Jun 27, 2024

View reviewed changes

chasj0326 reviewed Jun 27, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[홍건우] 자동차 경주 미션 Step1 #6

[홍건우] 자동차 경주 미션 Step1 #6

HongGunWoo commented Jun 24, 2024

jkea1 left a comment

jkea1 Jun 25, 2024

HongGunWoo Jun 27, 2024

chasj0326 Jun 27, 2024

jkea1 Jun 25, 2024

HongGunWoo Jun 27, 2024

jkea1 Jun 25, 2024

jkea1 Jun 25, 2024

jkea1 Jun 25, 2024

jkea1 Jun 25, 2024

jkea1 Jun 25, 2024

jkea1 Jun 25, 2024

jgjgill left a comment

jgjgill Jun 27, 2024

HongGunWoo Jun 27, 2024

jgjgill Jun 27, 2024

HongGunWoo Jun 27, 2024

jgjgill Jun 27, 2024

HongGunWoo Jun 27, 2024

jgjgill Jun 27, 2024

HongGunWoo Jun 27, 2024

chasj0326 left a comment •

edited

Loading

chasj0326 Jun 27, 2024

chasj0326 Jun 27, 2024

chasj0326 Jun 27, 2024

chasj0326 Jun 27, 2024

HongGunWoo Jun 27, 2024

chasj0326 Jun 27, 2024

chasj0326 Jun 27, 2024 •

edited

Loading

HongGunWoo Jun 27, 2024

		@@ -0,0 +1,37 @@
		import { describe, it, expect, vi } from 'vitest';

		@@ -0,0 +1,17 @@
		export function validateName({ name, minLength, maxLength }) {
		if (typeof name !== 'string') return false;

[홍건우] 자동차 경주 미션 Step1 #6

[홍건우] 자동차 경주 미션 Step1 #6

Conversation

HongGunWoo commented Jun 24, 2024

과제 수행 중 고민했던 점

리뷰 받고 싶은 부분

궁금한 점

jkea1 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jgjgill 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

chasj0326 left a comment • edited Loading

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

chasj0326 Jun 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chasj0326 left a comment •

edited

Loading

chasj0326 Jun 27, 2024 •

edited

Loading