[조수연] 자동차 경주미션 Step2 by suyeon1218 · Pull Request #12 · fe-clean-code-study/js-racingcar-custom

suyeon1218 · 2024-07-02T04:55:59Z

추가 요구사항

사용자는 몇 번의 이동을 할 것인지를 입력할 수 있어야 한다.
사용자가 잘못된 입력 값을 작성한 경우 에러 메시지를 보여주고, 다시 입력할 수 있게 한다.

미션을 수행하며 어려웠던 점

1. 도메인 로직과 어플리케이션 로직의 분리
도메인 로직에 대한 정의를 다시 세우면서 기능별로 로직을 분리했어요. 도메인 로직은 어플리케이션 로직에 간섭을 받지 않고 독자적으로 존재할 수 있어야 한다고 생각이 들어 최대한 view & 어플리케이션 로직을 도메인 로직과 분리하려고 했습니다.
그래서 main.js 파일엔 뷰 & 어플리케이션 로직이 존재하고, 도메인 로직은 오로지 어플리케이션 로직에서 호출받는 존재로 사용되도록 작성했어요. 그런데 다른 분들이 보기엔 아직 분리 되지 않은 부분이 있을 것 같다는 생각도 드네요. 🥲

2. 도메인 로직의 추상화
클래스 형태로 도메인 코드를 작성하면서 어떤 속성을 private 로 작성하고, 어떤 속성을 외부로 전달할지 고민하게 되었어요. 그리고 다음과 같은 최소 조건을 내걸었습니다.

  1. 원시 값이 아니거나 깊은 복사가 이뤄지지 않는 값은 외부에서 변경할 수 없게 한다.
  2. 변경이 필요할 경우 메서드를 통해 변경할 수 있도록한다.

3. 룰의 추가
룰의 변경이 일어날 경우를 대비해 Rule 클래스를 작성했어요. 해당 룰은 불리언 값을 반환하는 함수들로 이뤄진 객체를 가지고 있습니다. Rule 클래스의 run 메서드는 모든 룰이 통과한 경우 true 를 반환해요.

  interface rule {
      [key: string]: () => boolean;
  }

그런데 이렇게 작성한 클래스의 경우 룰을 적용하는 방법이 한정적이고... npm 을 통해 룰을 적용할 수 있는 방법도 아직 자세히 공부하지 못했습니다. ㅠㅠ

4. vitest
이제 테스트 코드에 조금 더 신경을 써야할 것 같습니다... 이젠 진짜 vitest 공부하겠습니다...

리뷰 받고 싶은 점 & 궁금한 점

onProceed
Race 클래스에서 라운드를 진행할 때 마다 현재 진행사항이 어떻게 되는지를 출력하는 요구사항이 있습니다. 이 요구사항을 만족시키기 위해 라운드를 증가시키는 (round++) 반복문 내부에서 view 와 관련된 코드를 어쩔 수 없이 호출해야 하는데 도메인 로직은 뷰 로직의 존재를 몰라야 한다고 알고 있습니다. 그래서 Race 클래스를 초기화할 때 onProceed 프로퍼티를 받고 round 가 증가할 때마다 onProceed 를 호출해주었데 이런 방식도 도메인이 뷰에 의존하게 되는 형태인지 궁금합니다.

초보 특... 무지의 무지. 제가 뭘 모르는지도 모르는 상태라 자유롭게 리뷰남겨주세요... 🥹

내가 생각하는 클린코드의 원칙

일관성...! 이라고 생각합니다. 코드 전체에서 지켜야 하는 규칙이 있다면 그걸 일관되게 지켜야 하고, 또 일관성이 있으면 하나의 파일에서 다른 파일로 넘어갈때도 코드의 구조가 어떻게 되어있는지 알기 쉽다는 생각이 들어요.

- 라운드 마다 '-' 를 사용해서 진행 척도 표시하기.

- 이전에 isValid 프로퍼티로 에러를 판별하던 걸 메세지로 판별하도록 변경

- 룰 프로퍼티를 바꿀 때 #initRules 메서드를 사용해서 초기화하기 - 초기화 및 프로퍼티 get, set 에서 유효성 검사하기

seeyoujeong

미션 수행하시느라 고생하셨어요!
고민한 흔적이 많이 보였습니다. ㅎㅎ

onProceed
저같은 경우는 경주를 진행할때마다 해당 라운드에 대한 기록을 저장해서 경주가 끝나면 보여주는 형태로 만들어봤어요! 이렇게 만들면 반복문 내부에 view 관련 코드를 호출하지 않아도 됩니다!

seeyoujeong · 2024-07-08T06:06:59Z

src/model/Race.js

+  static getRacerError(Racer) {
+    if (isSubClass(Racer, RacerClass) === false) {
+      return '인자로받는 Racer 프로퍼티는 Racer 클래스의 서브 클래스여야 합니다.';
+    }
+    return undefined;
+  }


검사를 통과했을때 반환해주는 값도 특정 문자열로 하는건 어떤가요?? 예를 들어, 통과같은 문자열로 해주는거죠!

뭔가 react-query 에서 data 가 없을 때 undefined 를 넣어주게 되니까 그걸 적용해보고 싶었어요! '통과'라고 작성해주게 되면 더 명시적이겠군요...!

seeyoujeong · 2024-07-08T06:08:38Z

src/model/Race.js

+      namesArr.some((carName) => {
+        carName = carName.trim();
+
+        return carName.length < 1 || carName.length > 10 ? true : false;


return carName.length < 1 || carName.length > 10;

이렇게만 써도 됩니다!

그러게요... ㅋㅋㅋㅋㅋ 왜 이렇게 비효율적으로 작성했을까요...!!

seeyoujeong · 2024-07-08T06:13:12Z

src/model/Racer.js

+    if (typeof name !== 'string') {
+      return '레이서들의 이름은 문자열이어야 합니다.';
+    }


특정 값이 문자열인지 확인하는 코드를 재사용 가능한 함수로 만들어보는것도 좋을 듯합니다. :)

타입 검사에 대한 부분은 재사용할 수 있는 코드로 만들것...! lotto 에서 적용해보겠습니다...!

seeyoujeong · 2024-07-08T06:15:54Z

src/model/Rule.js

+      if (keyError !== undefined || valueError !== undefined) {
+        throw new Error(keyError && valueError);
+      }


특정 값이 undefined가 아니면 에러를 던지는 코드도 재사용성 가능한 함수로 만들면 좋을듯합니다!

그러게요...!! 이것도 자주 반복되니까 한번 시도해보겠습니다!!

seeyoujeong · 2024-07-08T06:18:49Z

src/model/Rule.js

+  isExist(key) {
+    return this.#rules[key] === undefined ? false : true;
+  }


isExist(key) { return this.#rules[key] !== undefined; }

이렇게 바꿔도 됩니다!

자꾸... 비효율적인 코드를... 😇

seeyoujeong · 2024-07-08T06:22:21Z

src/view/RaceView.js

+  async inputRound() {
+    return await this.rl(
+      '몇번의 라운드를 진행할지 1 ~ 10 사이의 숫자를 입력해주세요! > ',
+    );
+  }
+
+  async inputRacerNames() {
+    return await this.rl(
+      '🏎️ 자동차 이름을 쉼표로 구분지어 입력해주세요! 🏎️ > ',
+    );
+  }


MVC 패턴을 따른다면 입력 받는 기능은 Controller에서 하는게 좋을 듯합니다!

input 에 관한 것은 View 에 해당하지 않는 건가요? 🧐 한번 찾아보겠습니다!

seeyoujeong · 2024-07-08T06:36:03Z

src/__tests__/race.test.js

+  test.each([
+    { round: '으아' },
+    { round: {} },
+    { round: null },
+    { round: undefined },
+    { round: function round() {} },
+  ])(
+    'Race 를 초기화할 때 round 에 숫자 형태로 바뀔 수 잇는 값($round)이 아닌 값을 전달하면 에러를 발생한다.',
+    ({ round }) => {
+      expect(() => {
+        makeRaceMockData({ round });
+      }).toThrow();
+    },
+  );
+
+  test.each([{ round: 0 }, { round: 11 }])(
+    'Race 를 초기화할 때 round 에 1미만 10초과의 숫자($round)를 전달하면 에러가 난다.',
+    ({ round }) => {
+      expect(() => {
+        makeRaceMockData({ round });
+      }).toThrow();
+    },
+  );


expect(() => { makeRaceMockData({ round }); }).toThrow();

위아래 테스트 코드의 실행, 검증 구절이 같다보니 같은 테스트를 하는건가라는 생각이 들었어요! toThrow()의 인수로 에러 메시지를 추가해서 명확하게 검증하면 어떨까요?

메시지를 명시적으로 작성하는 게 좋겠군요! 그렇게 할게요!!

seeyoujeong · 2024-07-08T06:38:18Z

src/__tests__/race.test.js

+  test('Racer 프로퍼티를 통해 현재 Racer 에 해당하는 클래스를 가져올 수 있다.', () => {
+    const race = makeRaceMockData({ Racer: Car });
+
+    expect(race.Racer).toBe(Car);


Car가 원시값이 아니라서 toEqual()을 사용해야합니다!

그러게요...!! 이번에 Vitest 공부하면서 배웠습니다! toEqual 자주 사용해야겠어요

seeyoujeong · 2024-07-08T06:41:08Z

src/__tests__/racer.test.js

+  test('position 프로퍼티를 가져올 수 있다.', () => {
+    const racer = makeRacerMockData({ position: 5 });
+
+    expect(racer.position).toBe(5);
+  });
+
+  test('position 프로퍼티를 가져올 수 있다.', () => {
+    const racer = makeRacerMockData({ position: 5 });
+
+    expect(racer.position).toBe(5);
+  });
+
+  test('init 메서드를 사용하면 postion 이 0 이 된다.', () => {
+    const racer = makeRacerMockData({ position: 5 });
+    racer.init();
+
+    expect(racer.position).toBe(0);
+  });
+
+  test('init 함수를 사용하면 postion 이 0 이 된다.', () => {
+    const racer = makeRacerMockData({ position: 5 });
+    racer.init();
+
+    expect(racer.position).toBe(0);
+  });


중복되는 테스트 발견!

도대체 왜 이런 걸까요... ㅋㅋㅋㅋ ㅠㅠㅠㅠ 부끄럽네요..... 미흡한 코드가 된 것 같아서...

chasj0326

고생하셨습니다 ! 뭔가 저로서는 생소한 문법으로 처리하신 부분들이 있어서, 배워가면서 코드를 읽었어요 ! 👍

onProcced

좋은 방법이라고 생각했어요 ! 저도 사실 라운드 마다 중간중간 출력도 해야하고... 로직이랑 뷰는 분리하라는 의견이 많고... 해서 애를 좀 먹었습니다 :-) 간단하게 한번씩만 뷰 출력을 시켜주기 위해서는 프로퍼티로 넘기는 방법이 적합해 보여요 !
의존하게 되는 형태인지는 ... 흠 저도 잘은 모르겠지만 의존하는 형태가 아니라고 생각해요 ! 왜냐면 저 Race 안에서, proceed 안에 무슨일이 벌어지고 있는지 모르기 때문이에요 👍

chasj0326 · 2024-07-08T12:40:02Z

src/__tests__/race.test.js

+function makeRaceMockData(raceProperty) {
+  const race = new Race({
+    Racer: Car,
+    round: 5,
+    rules: {
+      diceRule: () => gameSupport.dice(0, 10) >= 4,
+    },
+    names: '1, 2, 3',
+    ...raceProperty,
+  });
+
+  return race;
+}


오오..
매번 클래스를 생성해서 테스트하는게 불편했는데, 함수로 분리하자니 읽기가 어렵고 ... 이렇게 프로퍼티를 받아서 클래스를 생성하는 메소드를 만들면 되는군요 ! 👍

저도 이번에 처음 시도해본 패턴이에요...! 필요한 프로퍼티만 주고 싶은 경우 어떻게 해야하나 고민했는데 이런 방법이 나쁘지 않은 것 같아요!

chasj0326 · 2024-07-08T12:42:09Z

src/model/Rule.js

+  }
+
+  #definePropertyRule(obj, key) {
+    let _value = obj[key];


여기에 변수명에 _ 를 붙이신건 어떤 의미인가요 !

읽기 전용 프로퍼티를 만들고 싶었어요...! 실제로 JS에서 검증하는 효과는 없지만 관습적으로 이렇게 쓴다고 챗...지피티가 알려주었습니다... ㅋㅋㅋ set 과 get 프로퍼티로 할당/접근을 하게 될 경우 무한루프에 빠지는 걸 방지하기 위함도 있어요!

chasj0326 · 2024-07-08T12:45:55Z

src/model/Rule.js

+    Object.defineProperty(obj, key, {
+      get() {
+        return _value;
+      },
+      set(value) {
+        const keyError = Rule.getKeyError(key);
+        const valueError = Rule.getValueError(value);
+
+        if (keyError !== undefined || valueError !== undefined) {
+          throw new Error(keyError && valueError);
+        }
+
+        _value = value;
+      },
+    });
+  }


이건 룰을 추가할 때, 프록시 느낌으로 유효성검사를 하고 추가하기 위함인가요 ??

네 맞습니다!! 이게 과연 좋은 방법일진 모르겠지만... 도전해보고 싶어서 우선 이렇게 작성해보았어요!

chasj0326 · 2024-07-08T12:54:56Z

src/model/Rule.js

+    if (values.some((value) => Rule.getValueError(value) !== undefined)) {
+      return '룰의 값은 불리언을 반환하는 함수여야 합니다.';
+    }


함수타입이 아닐 때의 에러, 반환값이 boolean 타입이 아닐 때의 에러를 한번에 반환하려 하신 것 같아요! 따라서 undefined 가 아닐 때의 (에러메세지가 있을 때) 에러를 반환하셨다고 보입니다.

하지만 저는 조금 헷갈렸어요 !
getValueError 에서 반환된 문자열을 그대로 getRulesError 에서 반환할 수 있는 방법은 없을까요 ?!

undefined 가 아닌 경우만 return 시켜주기 위해선 좀 더 고민을 해봐야할 것 같아요...

우선 떠오르는 방법은 find 를 사용해서 undefined 가 아닌 key 값을 찾으면 해당 key 값에 해당하는 에러를 반환해주는 게 떠오르네요!!

chasj0326 · 2024-07-08T13:08:13Z

src/model/Race.js

+    return (names = names
+      .split(',')
+      .map((name) => name.trim())
+      .map((name) => new this.Racer({ name })));
+  }


이런게 된다는걸 처음알았어요 ! names 에 할당하고 그 names 를 반환하는 ... ! (맞죠 ?)

네...! 맞아요 그런데... 다시 보니까 names 에 다시 할당해야할 필요가 없었네요... ㅎㅎ!

chasj0326 · 2024-07-08T13:11:57Z

src/model/Race.js

+  startRace() {
+    for (let currentRound = 0; currentRound < this.round; currentRound++) {
+      this.#proceedRound();
+      if (this.onProceed && typeof this.onProceed === 'function') {
+        this.onProceed();
+      }
+    }
+
+    const racers = this.getRacerStatus();
+    const maxPosition = Math.max(...racers.map((racer) => racer.position));
+
+    return racers
+      .filter((racer) => racer.position === maxPosition)
+      .map((racer) => racer.name);
+  }


이건 개인적인 생각입니다만, startRace 에서는 레이스를 시작! 하는게 아니라 뭔가 전반적인 레이싱 과정을 처리하고 있다 보여져요.

proceedRound 와 onProceed 가 있으니, proceedRace 는 어떤가요 ?!

함수의 이름이 적절치 않군요...! 저도 race 를 전체적으로 처리하는 부분인데 게임을 시작해야한다는 생각에 이름을 이렇게 짓게 됐어요. 세진님 말대로 코드의 역할을 고려하면 proceedRace 가 적절해보여요! 아니면 porceed 만 해도 될것 같네용!

chasj0326 · 2024-07-08T13:15:35Z

src/model/Race.js

+    if (error !== undefined) {
+      throw new Error(error);
+    }


대부분 get...Error 를 한 이후에, undefined 가 아닐 경우 throw 에러를 하는 것이 반복되는 것 같아요 !
throwError(error : string | undefined) 이런 느낌으로 한 겹 묶으면 어떨까요 ?

Race.throwError(Race.getRoundError(round)) 이렇게 작성하는..!

래핑함수를 만들어주면 훨씬 편리해지겠군요...! 좋은 의견감사합니다!!

chasj0326 · 2024-07-08T13:19:31Z

src/model/Racer.js

+  _name;
+  _position;
+


멤버 변수를 숨길 떄 주로 언더바(protected)를 많이 사용하셨는데, #(private)가 아니라 protected 로 처리하신 수연님만의 기준이 있으신가요 ?!

제가 클래스랑 덜 친해서.. 여쭤봅니다 !

#private 의 경우엔 외부에서 읽기도 못하게 되어서 읽기 전용 속성이 필요했어요...!! 그런데 항상... #(private) 로 만들어서 get, set 메서드를 만들지 아니면 읽기 전용으로 만들지가 고민이 되네요 ㅎㅎ...!!

chasj0326 · 2024-07-08T13:26:17Z

src/main.js

+  while (true) {
+    roundInput = await view.inputRound();
+    const error = Race.getRoundError(roundInput);
+
+    if (error === undefined) {
+      break;


에러가 발생하면 입력을 재귀적으로 실행하기 위해 이렇게 작성하신 거로 이해했어요!

하지만, Race 에서 잘못된 라운드값이 들어왔을 때 .. Race 가 스스로 에러를 발생시킬 수 있다고 생각합니다 !

set round(round) { const error = Race.getRoundError(round); if (error !== undefined) { throw new Error(error); }

어라 안에서 이미 해주고 있군요 ! 그럼 차라리 try catch 로 에러를 받아서 다시 해주는 것이 더 자연스럽지 않을까요 ?

try { // 입력 받기 () // Race 생성하기 () }catch(error){ // 다시 처음부터 작동 }

이런 느낌으로 ...!

안 그래도 다른 분들은 try-catch 를 사용해서 재귀적으로 작동하게끔 하셨더라구요..!! 저도 해당 방법을 사용해보겠습니다!!

suyeon1218 added 30 commits June 25, 2024 14:25

refactoring: 테스트 코드 사이 줄 추가

e784a55

refactoring: 테스트 케이스 명시적으로 작성하기

0da0544

refactoring: 중복 유효성 검사 제거하기

4231bd6

refactoring: 다이스 범위 직관적으로 작성하기

d227960

refactoring: dice 를 util 함수로 분리하기

b7a06de

refactoring: 다이스 유틸 함수로 분리하기

4d6d200

refactoring: View 를 하나의 클래스로 만들기

fec7e43

chore: 사용하지 않는 파일 삭제

550b262

refactoring: MVC 패턴으로 역할 분리하기

2035dd6

chore: ESLint & Prettier 세팅

c0aadea

fix: 요구사항에 맞게 출력 수정하기

c887561

- 라운드 마다 '-' 를 사용해서 진행 척도 표시하기.

feat: 라운드를 사용자가 입력할 수 있도록 한다

bf700be

refactoring: 컨트롤러의 역할을 main 으로 변경

687fc97

refactoring: CarView -> RaceView로 이름 변경

a67c573

refactoring: racing과 관련된 상수 추가

1ebcfbb

chore: index.js -> game.js로 상수 파일 이름 변경

1b7e7bb

refactoring: Car 에서 Racer 에 해당하는 역할 분리하기

ab76a79

feat: 서브클래스 판별하는 함수 작성

3d679c2

refactoring: 유틸 함수를 default 값으로 묶기

f36a994

feat: Rule 도메인 클래스 작성

dc8f9c7

refactoring: Race 도메인 모델 작성

5dc5e87

feat: 유틸 함수에 대한 테스트 케이스 작성하기

208ca45

chore: 테스트 파일 이름 변경하기

4b2e862

fix: 에러 처리를 잘못하던 문제 수정

3b44288

- 이전에 isValid 프로퍼티로 에러를 판별하던 걸 메세지로 판별하도록 변경

fix: 룰 유효성 검사 수정하기

0f3cca6

- 룰 프로퍼티를 바꿀 때 #initRules 메서드를 사용해서 초기화하기 - 초기화 및 프로퍼티 get, set 에서 유효성 검사하기

feat: 룰의 키값들을 반환하는 메서드 추가

75520ab

feat: Rule 클래스에 대한 단위 테스트 진행

0102bc9

feat: move 메서드가 현재 포지션 프로퍼티를 반환하도록 변경

e752de1

feat: Racer 클래스에 대한 단위 테스트 진행

54dbb6b

feat: 목 데이터 프로퍼티 전달방식 변경

3366cba

suyeon1218 added 4 commits July 1, 2024 20:53

feat: 테스트를 위해 Animal 클래스 구현

837047e

feat: Racer 와 round 프로퍼티를 읽기 전용으로 변경

71db771

fix: rules 프로퍼티 에러검사를 하지 않던 문제 수정

fed37f9

feat: Race 클래스에 대한 테스트 케이스 구현

2dcc0c2

suyeon1218 requested review from JunilHwang, hyoribogo, jgjgill and seeyoujeong July 2, 2024 04:55

suyeon1218 self-assigned this Jul 2, 2024

suyeon1218 added the team1 label Jul 2, 2024

seeyoujeong reviewed Jul 8, 2024

View reviewed changes

chasj0326 approved these changes Jul 8, 2024

View reviewed changes

Conversation

suyeon1218 commented Jul 2, 2024

추가 요구사항

미션을 수행하며 어려웠던 점

리뷰 받고 싶은 점 & 궁금한 점

내가 생각하는 클린코드의 원칙

Uh oh!

seeyoujeong 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!

chasj0326 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