[자동차 경주] 장정안 미션 제출합니다. #336

ExceptAnyone · 2024-10-28T11:25:01Z

javascript-racingcar-precourse

Game Flow

자동차 이름 입력 받기
자동차 이름 입력 유효성 검증
시도할 횟수 입력 받기
시도할 횟수 유효성 검증
시도할 횟수만큼 자동차 경주 진행(무작위 값 4이상일 경우 전진)
경주 결과 출력
우승자 출력

자동차 이름 유효성 검증

자동차 이름이 빈 값일 경우
자동차 이름이 5자 보다 길 경우
자동차를 한 대만 입력했을 경우
자동차 이름이 쉼표(,)를 포함할 경우 -> 자동차 이름이 빈 값일 경우의 로직으로 함께 걸러짐
자동차 이름에 공백이 포함된 경우
자동차 이름이 중복된 경우

시도할 횟수 유효성 검증

시도할 횟수를 빈 값으로 입력했을 경우
시도할 횟수가 숫자가 아닐 경우
시도할 횟수가 0보다 작거나 9보다 클 경우 경우

자동차 이름 입력 에러

자동차 이름을 입력해주세요.
자동차 이름은 5자 이하로 입력해주세요.
자동차를 한 대만 입력했습니다.
자동차 이름은 쉼표(,)를 포함할 수 없습니다.
자동차 이름에 공백은 포함할 수 없습니다.
자동차 이름이 중복되었습니다.

시도할 횟수 입력 에러

시도할 횟수를 입력해주세요.
시도할 횟수는 숫자로 입력해주세요.
시도할 횟수는 0보다 크고 9보다 작아야 합니다.

우승자 출력 주의사항

우승자가 여러 명일 경우 쉼표(,)를 이용하여 구분

디렉토리 구조

commit convention

feat : 기능 추가
fix : 버그 수정
docs : 문서
style : 코드 스타일 수정
refactor : 리펙토링
test : 테스트코드 추가
chore : 기타 작업

고민했던 부분 및 의논점

App에서 game의 인자에 user는 인스턴스를, race는 클래스를 넣는 이유

  constructor(user, outputView, RaceClass) {
    this.user = user;
    this.outputView = outputView;
    this.RaceClass = RaceClass;
    this.raceInstance = null;
  }

raceInstance는 실제로 필요한 시점(cars가 준비된 후)에 생성.
Race 클래스의 생성자가 매개변수(cars)를 필요로 하기 때문에, App에서 미리 인스턴스화하면 cars를 어떻게 전달할지가 문제가 될 수 있음.

반면 User의 경우:

생성자에 특별한 매개변수가 필요없음
인스턴스를 즉시 사용할 수 있음
지연 초기화가 필요없음

따라서 User는 바로 인스턴스화해서 주입하고, Race는 클래스로 주입하여 나중에 필요한 시점에 인스턴스화 하였습니다.

책임의 범위와 관심사 분리

Car 클래스에서 무작위 값을 생성하는 로직을 담고있고, 이를 통해 move를 시켜주는 구조였는데요,

move(randomNumber) {
    if (randomNumber >= 4) {
      this.position++;
    }
  }

//리팩토링
move(enabled) {
    if (enabled) {
      this.position++;
    }
  }

이렇게 randomNumber관련 로직은 밖에서 판단하며,
move 메서드의 책임은 단순히 위치를 증가시키는 것으로 한정하였습니다.
하나의 클래스, 그리고 그 안에서의 메서드의 책임은 어디까지일지 고민하며 리팩토링하였습니다.

ui로직을 테스트할 필요가 있는가?

ui로직에 대해 테스트할 필요가 있는지 여러분들과 의논하고 싶습니다.
현재 미션에서는 우승자 출력 시 ','를 포함하여 출력하는 부분에서만 테스트 할 필요가 있다고 판단하였는데, 여러분들 의견이 궁금합니다.

- 입력받은 자동차들을 배열로 저장한 다음, 무작위 숫자가 4이상일 경우 앞으로 전진

- 입력된 이름으로 자동차를 생성하는지 - 랜덤 값이 4 이상일 때 자동차가 전진하는지 - 랜덤 값이 3 이하일 때 자동차가 전진하지 않는지 - 지정된 횟수만큼 경주 진행하는지

함수를 연달아 실행시켜주는 유틸함수를 구현하였습니다.

자동차 한 대만 입력했습니다는 유저 입장에서 와닿지 않다고 판단하여 두 대 이상 입력하라는 메세지로 변경하였습니다.

_pipe함수가 함수를 순차적으로 실행하니, 중복되는 로직을 가장 먼저 앞에서 실행함으로써 중복되는 로직을 줄였습니다.

기존 validator -> validateCarName

- 기존 RaceModel -> race로 클래스명을 변경하였습니다.

Car 클래스에서는 random Number를 pick해주는 로직을 알고있지 않아도 된다고 판단하여 분리하였습니다.

string 그대로 시도횟수를 받은 후, 유효성 체크에서 숫자형 타입으로의 변환하게 변경하였습니다

Race의 프로토타입을 직접 모킹하도록 구현하였습니다.

- toEqual은 순서마저 같아야 하므로 유연하지 못하다고 판단하여 every와 some을 이용해 테스트하는 방향으로 수정하였습니다

- 미션에서 제공해주는 랜덤 숫자를 뽑는 함수는 테스트 할 필요가 없다고 판단하여 삭제하였습니다.

- randomNumber를 move 메서드에서 알고 있을 필요가 없다고 판단하여 enabled라는 boolean값을 인자로 주어 더 명확하게 관심사를 분리하였습니다.

- 폴더구조 추가 - 구현하면서 고민했던 부분 및 의논점 추가

j-nary

섬세함이 돋보이는 코드였습니다! 코드리뷰하면서 많이 배워갔습니다
README에 의논할 점을 적어놓은 것도 인상적이었습니다 덕분에 코드리뷰할 때 그 부분들을 유의해서 본 것 같아요
시간 되시면 제 PR 코드리뷰 도 부탁드릴게용 :)

j-nary · 2024-10-29T11:06:30Z

src/Validator/validateCarName.js

+ * @param {string} input
+ * @returns {string[]}
+ */
+const splitCarNames = (input) => input.split(',');


해당함수는 util로 분리하는게 좋을 것 같은데 정안님 생각이 궁금하네요!
validateCarName 과 같은 관심사는 아니어보입니당

매우 동의합니다... 중복 처리하는 데에만 신경을 썼었네용

j-nary · 2024-10-29T11:06:48Z

src/Validator/validateCarName.js

+ */
+const splitCarNames = (input) => input.split(',');
+
+const validateCarName = _pipe(


pipe 쓰신게 돋보이네요! 배워갑니당 :)

j-nary · 2024-10-29T11:07:23Z

src/Validator/validateCarName.js

+
+/**@type {ValidationFunction} */
+const checkOneCarInput = (carNames) => {
+  if (carNames.length === 1) throwError(CAR_NAME_ERROR_MESSAGE.ONE_CAR_NAME);


carNames가 undefined일 경우 length 속성이 없을 수 있습니당
코드의 안전성을 위해 타입 체크도 해주면 좋을 것 같아요 :)

오 역시 꼼꼼하시네요. 그 부분은 splitCarNames에서 해주면 될 것 같슴다 ㅎㅎ
꼼꼼한 리뷰 감사해요!

j-nary · 2024-10-29T11:09:32Z

src/Validator/validateAttempts.js

+const checkAttemptsRange = (input) => {
+  if (input <= 0) throwError(ATTEMPT_ERROR_MESSAGE.OUT_OF_RANGE);
+
+  return input;
+};


양수뿐만 아니라 자연수인지도 확인하는 로직이 추가되면 좋을 것 같아요!

그러네요. isNaN에서 다 걸러질거라고 생각했는데, 꼼꼼하지 못했습니다 ㅠ 감사해요!

j-nary · 2024-10-29T11:11:51Z

src/util/random.js

+const random = {
+  generateNumber() {
+    return MissionUtils.Random.pickNumberInRange(0, 9);
+  },
+};


객체로 그룹화시켜준 이유가 궁금합니다!
어떤 확장성을 고려하신 걸까요 ?

현재 기능에서는 단순히 0과 9 사이에서의 숫자만 필요하지만,

generateBoolean() { return this.generateNumber() >= 4; }

이런 것들도 추후 추가가 될 수 있겠다 판단하여 처음 작성할 때 이렇게 구현하였습니다!

j-nary · 2024-10-29T11:13:55Z

src/model/Race.test.js

+    race.cars = [
+      { name: 'car1', position: 3 },
+      { name: 'car2', position: 3 },
+      { name: 'car3', position: 3 },
+    ];


random 숫자를 모킹해서 모의테스트를 만드는 방법도 있습니다!
앞서 코드리뷰 드린 것처럼 race의 필드를 직접적으로 접근해서 캡슐화를 느슨하게 하는 것보다는 모의 환경을 다른 유틸함수에서 조정하는 것은 어떨까요?

제가 jest에 익숙치 않아서 그런데,
모의 환경을 다른 유틸함수에서 조정한다는게 혹시 어떤 말씀이신지 설명 부탁드려도 될까요??

j-nary · 2024-10-29T11:14:48Z

src/model/Race.js

+  getCars() {
+    return this.cars;
+  }


getCars()를 정의하신 이유가 궁금합니다!
해당 함수가 정의됐으면 cars 필드는 private으로 캡슐화 해주는게 좋을 것 같아요!

private이 익숙치 않네요 하하 ㅠ
getCar를 한 이유는, 현재 이 미션에서는 this.cars로 대체하고 getCars를 삭제해도 별 차이는 없습니다!
다만, 추후에 자동차를 필터링해야만 하는 로직과 같은 것들을 확장성의 가능성에 놓고 생각하여 getCar를 정의하였습니다 ㅎㅎ!

j-nary · 2024-10-29T11:15:28Z

src/model/Race.js

+  race(attempts) {
+    if (attempts <= 0) return;
+
+    const cars = this.getCars();


this.cars 를 사용하지 않고 getCars() 라는 메서드를 호출하신 이유가 있으실까요?

위 리뷰 댓글에서 설명드렸습니다 ㅎㅎ!

j-nary · 2024-10-29T11:16:16Z

src/model/Car.js

+    this.name = name;
+    this.position = 0;


private 필드로 선언해서 캡슐화를 강화시키는 건 어떨까요?

제이너리님 덕분에 private의 중요성에 대해 배워갑니다.
앞으로의 미션에서는 더욱 고려하여 진행하겠습니다 !!!

silver0108 · 2024-10-29T11:25:19Z

다양하게 테스트 코드 작성하신 게 정말 인상 깊어요!
저는 아직 테스트 코드 작성하는 게 어려워서 많이 배워갑니다!!!
저도 코드 리뷰 한 번 부탁드릴게요 ㅎㅎ!
#129

silver0108 · 2024-10-29T11:19:15Z

src/model/Race.js

+    });
+  }
+
+  determineWinners() {


maxPosition을 구하고 이를 이용해 필터링하고 있는데, reduce 같은 메서드를 사용하면 한 번의 순회로 최대 위치와 우승자를 동시에 구할 수 있을 것 같아요!

소은님 코드에서 reduce를 이용하여 구현하신 것 봤습니다 ㅎㅎ!
안그래도 감명깊어 리뷰 달고 오는 길입니다!

silver0108 · 2024-10-29T11:21:47Z

src/model/Race.js

+    this.moveForward();
+    outputView.printRaceStatus(cars);
+
+    this.race(attempts - 1);


반복 횟수가 많아질 경우 재귀 호출은 스택 오버플로우가 발생할 수도 있을 것 같은데 for과 같은 반복 루프를 안쓰시고 재귀로 하신 이유가 있을까요?

맞습니다. 반복횟수가 엄청나게 많아질 경우 그러한 현상이 발생할 수 있을 것 같아요!
다만, 확장성을 많이 고려한다고 하더라도, 자동차 경주라는 특성상 무수히 많은 반복 횟수를 가지지는 않을 것 이라고 판단하였습니다.
이 로직에서는, for문으로 구현한다면, 어떤 처리든 for문 안에서만 진행해야한다는 점이 확장성을 고려한 코드에서 가독성을 낮출 것이라고 생각하여 재귀로 구현해봤습니다!!

또한, 제가 재귀에 익숙하지 않은데요. 얼마 전 토스 next의 코딩테스트에 재귀관련된 문제가 있었는데, 여기에서 영감을 받아 구현하였습니다 !

silver0108 · 2024-10-29T11:23:00Z

src/util/util.js

+  return acc;
+};
+
+const _pipe =


저도 파이프 사용하는 건 생각 치 못했는데 좋은 방법인거 같아요!!

감사합니다 ㅎㅎ

rtttr1

정말 고민의 흔적이 많이 보이는 코드 잘 보고갑니다!!
저도 더 많은 고민을 해야겠다는 동기부여가 되네요ㅎ 3주차도 화이팅입니다!!

rtttr1 · 2024-10-29T14:09:51Z

src/Validator/validateCarName.js

+ * @throws {Error}
+ */
+const checkEmpty = (carNames) => {
+  if (carNames.length === 0 || carNames.some((name) => name.trim() === '')) {


some 메서드는 몰랐었는데 js에 이런 메서드도 있었네요! 배워갑니다~!

감사합니다 ㅎ!!

rtttr1 · 2024-10-29T14:12:58Z

src/constants/messages.js

@@ -0,0 +1,30 @@
+const GAME_MESSAGE = Object.freeze({


freeze로 객체를 완전하게 상수화 하는거 배워갑니다!

rtttr1 · 2024-10-29T14:23:27Z

src/user/User.js

+  }
+}
+
+export default User;


입력을 받아오는 역할만 하는데 class로 선언한 이유가 있을까요?

현재 상황에서는 굳이 class일 필요는 없다고 생각이 드네요, 흠...
단순히 다른 클래스들과 동일하게 패턴을 구현하려고 했었던 것 같습니다!
규홍님 덕분에 당연하게 여기던 제 코드에 대해 다시 생각해보는 기회가 되었네요!!
감사해요 ㅎㅎ!

rtttr1 · 2024-10-29T14:26:53Z

src/view/outputView.js

+    this.printMessage(`${GAME_MESSAGE.FINAL_WINNER} ${winnersName}`);
+  },
+};
+export default outputView;


객체화 해주시는 이유가 무엇인가요??

음.. 우선 출력 관련 메서드들이 하나의 객체 안에 모여있게 하기 위함입니다.
이렇게 한다면, 어디서든 outputView를 import해서 사용이 가능하여 깔끔할 것이라 생각하였습니다!
또한 따로 개별함수로 구현하는 것 보다는 테스트 코드에서 모킹하는 과정이 쉬워지니까요!

ExceptAnyone added 30 commits October 22, 2024 18:46

docs: 요구사항 1차 분석

aba9d97

feat: 자동차 경주 게임에 필요한 메세지 추가

7b1db7a

feat: 게임을 담당하는 Game 클래스 작성

dcd95cc

feat: 게임의 프린트를 담당하는 outputView 작성

07c5bf6

docs: 자동차 이름 입력 유효성 검증 요소 추가

32aef7f

feat: 자동차 이름 입력 받는 기능 구현

79f164a

test: 자동차 이름 입력 받기 테스트 코드 작성

e3ec103

feat: 시도할 횟수 입력 받는 로직 작성

209b1de

test: 시도할 횟수 입력 받는 테스트 코드 작성

7589a42

feat: 자동차의 움직임을 관리하는 Car 클래스 생성

caf12e6

feat: 자동차 경주를 진행하는 로직 구현

a44b52d

- 입력받은 자동차들을 배열로 저장한 다음, 무작위 숫자가 4이상일 경우 앞으로 전진

fix: 무작위 숫자 4초과가 아닌 4이상으로 로직 수정

d97d5e7

test: Game 클래스 내 테스트코드 작성

9722d96

- 입력된 이름으로 자동차를 생성하는지 - 랜덤 값이 4 이상일 때 자동차가 전진하는지 - 랜덤 값이 3 이하일 때 자동차가 전진하지 않는지 - 지정된 횟수만큼 경주 진행하는지

feat: 최종 우승자 출력 로직 구현

6095822

test: 우승자를 결정하는 로직 테스트 코드 추가

acbace2

docs: 자동차 이름 유효성 검증 추가

bc71c9b

feat: 우승자 출력 시 쉼표 다음 공백 추가

2c59dda

test: 우승자 출력 메서드 테스트 추가

b5fcdf5

feat: _reduce와 pipe 함수 작성

e97d6df

함수를 연달아 실행시켜주는 유틸함수를 구현하였습니다.

docs: 유효성 검증에 대한 에러 메세지 추가

c2a86e2

feat: 유효성 검증에 대한 에러 메세지 추가

7ebae71

docs: 추가 유효성 검증 분석 후 추가

ccf9bdf

feat: 에러를 throw하는 함수 추가

a196fac

fix: 에러 메세지 수정

b36abe4

자동차 한 대만 입력했습니다는 유저 입장에서 와닿지 않다고 판단하여 두 대 이상 입력하라는 메세지로 변경하였습니다.

feat: 자동차 이름 입력 유효성 검증 추가

7a89bce

refactor: 중복되는 로직 간소화

04d24d2

_pipe함수가 함수를 순차적으로 실행하니, 중복되는 로직을 가장 먼저 앞에서 실행함으로써 중복되는 로직을 줄였습니다.

docs: 자동차 이름 입력 유효성 검증 완료된 목록 체크

e5d0db1

feat: 중복 처리되는 에러 하나 제거

9d56b3a

chore: 파일 이름 구체적으로 변경

1b9861a

기존 validator -> validateCarName

feat: 시도할 횟수 유효성 검증 구현

cc09926

ExceptAnyone added 26 commits October 25, 2024 20:33

feat: 시도횟수에 따른 실행결과 출력

bf38bdf

fix: 각 경주 라운드 별 줄바꿈 한 번으로 변경

6d2dca4

refactor: RaceModel 클래스 명 변경

bbc1857

- 기존 RaceModel -> race로 클래스명을 변경하였습니다.

chore: 필요없는 콘솔로그 제거

bb7937c

test: Race 클래스 테스트 코드 작성

8adf752

test: User 클래스 테스트 코드 인자 수정

c2e8fff

test: 출력과 관련된 outputView 테스트 코드 작성

5070a84

test: Race 클래스 테스트 코드 작성

296bd67

test: Game 클래스에서 올바른 순서 호출 여부 테스트

3059b3c

fix: 시도할 횟수 9 이상도 입력 가능하도록 수정

0f92ff6

refactor: Car 클래스에서 랜덤 넘버 관심사 분리

3b6d197

Car 클래스에서는 random Number를 pick해주는 로직을 알고있지 않아도 된다고 판단하여 분리하였습니다.

chore: 필요없는 코드 제거

54edbab

refactor: 유저 입력 받는 메서드 유연한 구조로 리팩토링

d40fcfc

chore: js style guide에 맞게 작은 따옴표로 변경

f56eeac

fix: 더 정확한 에러메세지로 변경

4276cb6

refactor: 시도 횟수의 숫자형 타입 변환 제거

fabfc02

string 그대로 시도횟수를 받은 후, 유효성 체크에서 숫자형 타입으로의 변환하게 변경하였습니다

test: 시도 횟수 유효성 검사 테스트 코드 each로 구조화

ace7700

style: 테스트 코드 오타 수정

f18ef5e

refactor: Race 클래스에서 Car클래스에 의존하지 않게 리팩토링

209bef9

test: 의존성 주입 변경에 따른 테스트 코드 수정

be386b7

Race의 프로토타입을 직접 모킹하도록 구현하였습니다.

refactor: race 메서드 내 for문 제거 후 재귀호출

557a628

test: 모든 자동차가 같은 위치에 있을 때의 테스트 코드 수정

4a33b0d

- toEqual은 순서마저 같아야 하므로 유연하지 못하다고 판단하여 every와 some을 이용해 테스트하는 방향으로 수정하였습니다

test: 미션 유틸 코드를 테스트하는 코드 제거

18ae12b

- 미션에서 제공해주는 랜덤 숫자를 뽑는 함수는 테스트 할 필요가 없다고 판단하여 삭제하였습니다.

refactor: move 메서드 로직 변경

ae392c0

- randomNumber를 move 메서드에서 알고 있을 필요가 없다고 판단하여 enabled라는 boolean값을 인자로 주어 더 명확하게 관심사를 분리하였습니다.

chore: todo 제거

bd79db3

docs: 리드미 업데이트

acd8dd6

- 폴더구조 추가 - 구현하면서 고민했던 부분 및 의논점 추가

j-nary reviewed Oct 29, 2024

View reviewed changes

silver0108 reviewed Oct 29, 2024

View reviewed changes

rtttr1 reviewed Oct 29, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[자동차 경주] 장정안 미션 제출합니다. #336

[자동차 경주] 장정안 미션 제출합니다. #336

ExceptAnyone commented Oct 28, 2024 •

edited

Loading

j-nary left a comment •

edited

Loading

j-nary Oct 29, 2024

ExceptAnyone Oct 31, 2024

j-nary Oct 29, 2024

j-nary Oct 29, 2024

ExceptAnyone Oct 31, 2024

j-nary Oct 29, 2024

ExceptAnyone Oct 31, 2024

j-nary Oct 29, 2024

ExceptAnyone Oct 31, 2024

j-nary Oct 29, 2024

ExceptAnyone Oct 31, 2024

j-nary Oct 29, 2024

ExceptAnyone Oct 31, 2024

j-nary Oct 29, 2024

ExceptAnyone Oct 31, 2024

j-nary Oct 29, 2024

ExceptAnyone Oct 31, 2024 •

edited

Loading

silver0108 commented Oct 29, 2024

silver0108 Oct 29, 2024

ExceptAnyone Oct 31, 2024

silver0108 Oct 29, 2024

ExceptAnyone Oct 31, 2024

silver0108 Oct 29, 2024

ExceptAnyone Oct 31, 2024

rtttr1 left a comment

rtttr1 Oct 29, 2024

ExceptAnyone Oct 31, 2024

rtttr1 Oct 29, 2024

rtttr1 Oct 29, 2024

ExceptAnyone Oct 31, 2024

rtttr1 Oct 29, 2024

ExceptAnyone Oct 31, 2024

[자동차 경주] 장정안 미션 제출합니다. #336

Are you sure you want to change the base?

[자동차 경주] 장정안 미션 제출합니다. #336

Conversation

ExceptAnyone commented Oct 28, 2024 • edited Loading

javascript-racingcar-precourse

Game Flow

자동차 이름 유효성 검증

시도할 횟수 유효성 검증

자동차 이름 입력 에러

시도할 횟수 입력 에러

우승자 출력 주의사항

디렉토리 구조

commit convention

고민했던 부분 및 의논점

책임의 범위와 관심사 분리

ui로직을 테스트할 필요가 있는가?

j-nary 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

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

ExceptAnyone Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

silver0108 commented Oct 29, 2024

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

rtttr1 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

ExceptAnyone commented Oct 28, 2024 •

edited

Loading

j-nary left a comment •

edited

Loading

ExceptAnyone Oct 31, 2024 •

edited

Loading