[2단계 - 자동차 경주 구현] 하루(김하루) 미션 제출합니다. #31

365kim · 2021-02-12T13:50:25Z

안녕하세요!

자동차 경주 게임 2단계 리뷰요청 드립니다.
리팩토링을 했는데 혼돈을 거쳐 더 미궁속으로 빠져버린 느낌입니다....🙃
리뷰어님의 피드백이 절실합니다...! 🙏🙏

요약

구현 순서: 테스트 전체 작성 후, 각 테스트를 통과하기 위한 단위 기능 구현
구현 결과: 작성한 cypress 테스트 모두 통과 ✔
구현 내용
- 자동자 경주 진행
  - 게임 한 판을 1초 동안 진행한다.
  - 게임 진행 중에는 로더를 화면에 표시하고 게임이 완료되면 표시를 없앤다.
- 결과 출력
  - 게임완료 2초 후 축하 alert 메세지를 표시한다.

고민사항 / 미해결과제

고민사항: 시간지연의 실행주체 (코멘트💬로 질문드렸습니다!)
미해결과제: 데이터의 관리 (코멘트💬로 질문드렸습니다!)

도움주셔서 정말정말 감사합니다.
해피 설 연휴 되세요! 🍖🧀👨‍👩‍👧‍👦👨‍👩‍👧‍👦🍓🍒 😆

365kim · 2021-02-12T14:10:25Z

src/js/index.js

@@ -12,6 +12,7 @@ export default class RacingGame {
    $('#car-name-submit').addEventListener('click', () =>
      handleCarNameInput(this.cars),
    );
+    $('#game-restart-button').addEventListener('click', setToInitialView);
  }
 }


질문드립니다! 🙋🏻‍♀️ - 데이터의 관리

전체코드에서 필요한 3개의 이벤트리스너를 한 군데에서 관리하고 싶어서 index.js에서 아래와 같이 코드를 작성했었습니다.

export default class RacingGame { constructor() { this.cars = []; } init() { setToInitialView(); $('#car-name-submit').addEventListener('click', () => handleCarNameInput(this.cars), ); $('#racing-count-submit').addEventListener('click', () => handleRacingCountInput(this.cars), ); $('#game-restart-button').addEventListener('click', setToInitialView); } }

그런데 첫번째 핸들러 handleCarNameInput()에서 cars에 Car인스턴스들을 추가해도 handleRacingCountInput()에는 변경된 내용이 전달이 안되었습니다. 결국 cars의 변경된 내용을 전달하기 위해서 두번째 이벤트리스너는 handleCarNameInput 안에서 달아주게 되었습니다.

이 코드에서 동기화(?)가 되지 않는 문제를 되게끔 하는 방법, 혹은 인자로 전달하지 않고 cars를 공유하는 좋은 방법이 있을지 궁금합니다! 제가 어떤 개념을 더 공부하면 좋을지 힌트 부탁드리겠습니다 🙏🙏🙏

일반 function과, Arrow function의 차이점을 알아보시고
두 가지 함수 선언방식을 사용했을 때 this값이 어떻게 달라지는지 함수 밖과 함수 안에서
console.log(this)로 확인해보세요.

Search keyword: regular function vs arrow function, this binding, apply, call, bind

hint:

$('#car-name-submit').addEventListener( 'click', handleCarNameInput.bind(this), );

console.log(this); // Window 전역객체 const car = { name: 'tico', logNameRegFunc: function () { console.log(this); // { name: 'tico', logNameRegFunc: f ... } console.log(this.name); // 'tico' }, logNameArrowFunc: () => { console.log(this); // Window 전역객체 console.log(this.name); // '' }, }; car.logNameRegFunc(); car.logNameArrowFunc();

제안해주신 키워드 순서대로 학습하고 적용하니까 제가 원했던 대로 동작하네요! 😆 정말 감사합니다!👍👍👍 각 상황에서 this가 어떤 값을 가리키는지 지속적으로 익혀서 잘 사용 해야겠습니다!

365kim · 2021-02-12T14:18:08Z

src/js/views/alertGameOver.js

+export const alertGameOverAfterDelay = async (winners) => {
+  const { DELAY, MSG_SUFFIX } = GAME_OVER_NOTICE;
+
+  await wait(DELAY);
+  alert(`${winners}${MSG_SUFFIX}`);
+};


질문드립니다! 🙋🏻‍♀️ - 시간지연의 실행주체

'게임완료 2초 후에 축하의 alert 메세지를 띄우는 기능'을 구현하기 위해 시간지연을 주었습니다. 이런 경우에 시간을 지연하는 역할을 이렇게 뷰가 가져가도 괜찮은지 궁금합니다.

좋은 질문입니다. 👍
"N초 뒤에 ~~를 실행한다" 라는 작업은 도메인 로직입니다.
현재는 alert을 띄우는 정도라 코드가 간단해보이고 이정도 복잡도에서 그쳤지만,
예를 들어, "N초 뒤에 DB를 업데이트하고 업데이트가 완료되면 다른페이지로 이동 후 팝업을 3초보여줬다가 숨긴다.".
라는 요구사항이라면 어떨까요?
View에서 이러한 로직을 구현해서는 안됩니다.
View는 Model의 상태를 단순히 표현(representation)만해야합니다.

그렇다면 이렇게 변경해볼 수 있겠네요.

게임이 끝나면 Controller에서 2초뒤에 Model을 변경한다. (e.g. isGameOvered, isPopupShowing)

View는 Model의 상태변화를 감지하여 isPopupShowing 값이 변경되면 팝업을 보여준다.

예시로 들어주신 경우를 생각해보니 시간지연이 View에 있으면 안되겠다는 사실이 확실하게 이해가 갔습니다! 자세하게 설명해주셔서 감사합니다 :)

ysm-dev

아직 View와 Controller가 완벽하게 분리되진 않았지만
많이 좋아졌습니다 👍

async await를 이용한 비동기 처리부분은 훌륭하네요 👍

코멘트는 각 라인에 달았습니다.

ysm-dev · 2021-02-13T11:12:34Z

src/js/constants/racingRule.js

+export const RACING_RULE = {
+  MIN_SCORE: 0,
+  MAX_SCORE: 9,
+  THRESHOLD_SCORE: 4,
+  TURN_DURATION: 1000,
+};


좋은 도메인 분리네요! 👍

감사합니다!! ☺️

ysm-dev · 2021-02-13T11:33:37Z

src/js/views/alertGameOver.js

+export const alertGameOverAfterDelay = async (winners) => {
+  const { DELAY, MSG_SUFFIX } = GAME_OVER_NOTICE;
+
+  await wait(DELAY);
+  alert(`${winners}${MSG_SUFFIX}`);
+};


좋은 질문입니다. 👍
"N초 뒤에 ~~를 실행한다" 라는 작업은 도메인 로직입니다.
현재는 alert을 띄우는 정도라 코드가 간단해보이고 이정도 복잡도에서 그쳤지만,
예를 들어, "N초 뒤에 DB를 업데이트하고 업데이트가 완료되면 다른페이지로 이동 후 팝업을 3초보여줬다가 숨긴다.".
라는 요구사항이라면 어떨까요?
View에서 이러한 로직을 구현해서는 안됩니다.
View는 Model의 상태를 단순히 표현(representation)만해야합니다.

그렇다면 이렇게 변경해볼 수 있겠네요.

게임이 끝나면 Controller에서 2초뒤에 Model을 변경한다. (e.g. isGameOvered, isPopupShowing)

View는 Model의 상태변화를 감지하여 isPopupShowing 값이 변경되면 팝업을 보여준다.

ysm-dev · 2021-02-13T11:37:06Z

src/js/models/validateCarNames.js

+  if (!cars.every((car) => !isBlank(car.getName()))) {
+    return ERR_MESSAGE.CAR_NAME_CANNOT_BE_BLANK;
+  }


Array.prototype.some을 사용하면 조금 더 간결해질 것같은데요?

오 그렇네요! 좋은 메서드 제안 감사합니다 :) !! 👍👍

ysm-dev · 2021-02-13T11:40:12Z

src/js/models/Car.js

+  getName() {
+    return this.name;
+  }
+
+  getForwardCount() {
+    return this.forwardCount;
+  }


반드시 클래스 getter setter를 구현해서 인스턴스 속성에 getter setter로 접근할 필요는 없습니다.
이렇게 getter this.property를 바로 반환할것이라면, getter setter를 구현하지않고,
사용하는쪽에서 car.name으로 접근해도 괜찮습니다.

getter setter를 왜 사용하는지 덜 이해한 채로 코드를 작성했었습니다. 콕 짚어주셔서 다시 한 번 정리하고 넘어갈 수 있었습니다. 감사합니다!

ysm-dev · 2021-02-13T11:41:23Z

src/js/models/Car.js

+  getRandomNumber(min, max) {
+    return Math.floor(Math.random() * (max - min + 1)) + min;
+  }


이 메소드는 this를 사용하지 않는것같아요. 그리고 Car라는 도메인이랑도 관련없어보입니다.
즉 Car class안에 있을 필요가 없는 메소드입니다. 👍

Car 클래스 내 isMovingForward()에서만 호출된다는 이유로 함께 이사시켰는데 잘못된 판단이었군요,,, 앞으로 메서드의 위치를 결정할 때 '어디서 사용되는지'가 아니라 '어떤 역할을 하는지'로 판단하는 훈련을 해보겠습니다! 좋은 피드백 감사합니다. 👍👍👍

ysm-dev · 2021-02-13T11:44:43Z

src/js/handlers/handleGameResult.js

+import { getWinners } from '../models/getWinners.js';
+
+export const handleGameResult = async (cars, racingCount) => {
+  let winners;


let으로 선언할 필요 없이 아래에서 const winners = getWinners(cars); 하면 되겠네요!.
let을 사용할땐 항상 꼭 let을 사용해야하는 상황일지 다시한번 고민해보세요.
대부분의 경우는 let 선언 없이도 작성할 수 있는 경우가 많습니다.

필요한 모든 변수를 선언부에서 미리 선언해두는 것보다, 필요한 시점에 선언하는 것이 더 바람직한 방법이군요! (참고) 짚어주신 덕분에 새로이 배웠습니다. 👍👍👍
그렇다면 혹시 아래 두 가지 스타일 중에 어떤 방식이 더 낫다고 생각하시는지 궁금합니다! 저는 전자가 가독성이 더 좋아보이는데 다른 분들이 보시기엔 함수 중간에 불필요한 개행이 있어보이는건 아닌가 싶기도 해서요..!

export const handleGameResult = async (cars, racingCount) => { clearResidueForwardIcon(); showLoader(); await playRacingGame(cars, racingCount); hideLoader(); const winners = getWinners(cars); // 변수 선언 전 개행 추가O showGameResult(winners); alertGameOverAfterDelay(winners); };

export const handleGameResult = async (cars, racingCount) => { clearResidueForwardIcon(); showLoader(); await playRacingGame(cars, racingCount); hideLoader(); const winners = getWinners(cars); // 변수 선언 전 개행 추가X showGameResult(winners); alertGameOverAfterDelay(winners); };

개행에 있어서 정답은 없고, 일반적으로 prettier나 lint 룰이 있습니다.

이런부분은

세미콜론을 붙일것인가 생략할것인가

큰 따옴표를 쓸것인가 작은 따옴표를 쓸것인가

trailing comma를 찍을것인가 말것인가

탭을 쓸것인가 스페이스를 쓸것인가

위와 같은 성능에 전혀 영향을 미치지 않는 코드 컨벤션 선호의 차이입니다.
개인적으로 뭐가 더 나은지 고민해보고 선택하시면 되며, 추후에는 같이 일하는 팀원들과만 협의하시면 됩니다. 👍

아하 컨벤션 선호의 차이군요! 자세한 설명 감사합니다 😄👍 👍 👍 👍

ysm-dev · 2021-02-13T11:51:57Z

src/js/models/validateCarNames.js

+  if (!cars.every((car) => isValidLength(car.getName()))) {
+    return ERR_MESSAGE.CAR_NAME_TOO_LONG;
+  }


Suggested change

if (!cars.every((car) => isValidLength(car.getName()))) {

return ERR_MESSAGE.CAR_NAME_TOO_LONG;

}

if (!cars.map(car => car.name).every(isValidLength))) {

return ERR_MESSAGE.CAR_NAME_TOO_LONG;

}

틀린건 아니지만 이렇게도 작성할 수 있습니다.

ysm-dev · 2021-02-13T11:55:30Z

cypress/integration/racingGame.spec.js

@@ -109,23 +113,34 @@ describe('racing-game', () => {
  });

  it('난수를 생성하는 함수가 0 ~ 9 사이의 정수를 반환한다.', () => {
-    const possibleScores = Array.from({ length: 10 }).map((v, i) => i);
+    const car = new Car();
+    const { MIN_SCORE, MAX_SCORE } = RACING_RULE;


위쪽 7번라인에서 꺼내놨는데 다시 꺼낼 필요가 있을까요?

앗 제가 잘 import 되었는지 quokka 확인하고 나서 것을 깜빡하고 못지운 모양입니다...! 전역변수인 6,7번 라인을 없애는 방향으로 정리해보겠습니다. 꼼꼼히 봐주셔서 감사합니당 🥰🥰🥰

맨위에서 꺼내놓고 쓰는게 더 나아보여요! 👍

이 경우에는 꺼내놔도 값이 변경되거나 값을 추적할 일이 없어서 전역으로 사용해도 괜찮은 거군요! 피드백 감사합니다 :) 👍

ysm-dev · 2021-02-13T14:00:13Z

src/js/index.js

@@ -12,6 +12,7 @@ export default class RacingGame {
    $('#car-name-submit').addEventListener('click', () =>
      handleCarNameInput(this.cars),
    );
+    $('#game-restart-button').addEventListener('click', setToInitialView);
  }
 }


일반 function과, Arrow function의 차이점을 알아보시고
두 가지 함수 선언방식을 사용했을 때 this값이 어떻게 달라지는지 함수 밖과 함수 안에서
console.log(this)로 확인해보세요.

Search keyword: regular function vs arrow function, this binding, apply, call, bind

hint:

$('#car-name-submit').addEventListener( 'click', handleCarNameInput.bind(this), );

365kim · 2021-02-14T15:01:01Z

안녕하세요!
우선 황금 설연휴 중임에도 빠른 피드백 해주셔서 감사드립니다! 말씀해주신 방향 참고해서 수정 진행해 보았습니다!

📝 2단계 피드백 반영 목록

이벤트핸들러에서 bind 메서드로 cars를 전달할 수 있다.
시간지연의 실행은 Controller에게 위임한다.
every/some메서드를 가독성을 고려하여 적절하게 사용한다.
let 사용을 지양하고 변수 선언은 시작부분이 아니더라도 필요한 곳에서 한다.
Car 클래스 내부에는 관련없는 로직이 없도록 한다.
getter는 필요한 경우에만 사용한다.
test코드에 중복되는 변수를 정리한다.

한 가지 반영하고 싶었지만 하지 못한 것은 isGameOvered isPopupShowing과 같은 예시로 말씀해주신 것처럼 게임의 '상태'값을 관리해서 View가 모델의 상태변화를 감지하도록 하는 것입니다. 이를 위해 MVC 패턴 관련해서 좋은 피드백을 받은 크루의 코드를 참고하면서 제 코드도 수정을 시도했었는데 제가 아래 두 가지 사항을 이해하지 못한 채로 형태만 따라하다가 길을 잃어버리게 된 것 같습니다...!

Model은 상태변화 를 어떤 '형태'로 관리하는지
어떻게 View가 Model의 변화를 '감지'하는지

제가 리뷰어님으로부터 이미 좋은 어드바이스를 너무x100 많이 받았지만,,, 염치불구하지만 이와 관련해서 한번 더 작은 힌트 주시면 정말정말 감사하겠습니다...!! 🙏🙏🙏

ysm-dev · 2021-02-14T15:34:45Z

Model은 상태변화 를 어떤 '형태'로 관리하는지

cars나 게임 횟수처럼 isGameOvered 같은 게임의 상태를 표현하는 값들도 RacingGame 클래스의 멤버변수로 선언하면 됩니다.

어떻게 View가 Model의 변화를 '감지'하는지

관련해서는 "옵저버 패턴"이라는 키워드로 검색해서 관련 문서를 많이 읽어보세요. 👍

벌써 많이 발전한 모습이 보입니다 👍

설 연휴 2단계까지 수고하셨습니다. 👍

365kim · 2021-02-14T15:42:30Z

머지....! 🎉😆 옵저버 패턴 열심히 읽어보겠습니다! 감사합니다! :)

365kim · 2021-03-09T03:30:03Z

src/js/views/showGameResult.js

+export const showGameResult = (winners) => {
+  const { PREFIX, SUFFIX } = GAME_RESULT;
+
+  $('#game-result-text').innerHTML = `${PREFIX}${winners}${SUFFIX}`;


innerText로도 충분해 보인다 🤔

365kim · 2021-03-09T03:30:54Z

src/js/views/createCarElements.js

+
+export const createCarElements = (cars) => {
+  $('#game-process-screen').innerHTML = cars
+    .map((car) => carTemplate(car.name))
+    .join('');
+};
+


innerHTML로 넣기보다 DOM요소를 직접 생성해서 넣는게 좋겠다...!

365kim · 2021-03-09T03:31:55Z

src/js/models/Car.js

+  constructor(name) {
+    this.name = name;
+    this.forwardCount = 0;
+  }
+
+  resetForwardCount() {
+    this.forwardCount = 0;
+  }


forwardCount의 init value를 const로 관리해도 좋겠다.

365kim · 2021-03-09T03:32:59Z

셀프 코드리뷰 🤔

잘한 것은 무엇인가?

기획상 필요한 숫자를 상수로 잘 관리하고 있다.
함수 네이밍을 줄여쓰지 않았다.
유틸함수를 필요에 따라 활용하고 있다.

아쉬운 점은 무엇인가?

비동기 함수를 사용하면서도 Promise를 잘 모른 채 사용했다.
코드 내 메서드의 위치를 결정할 때 ‘어떤 역할을 하는지’ 보다 ‘어디서 사용되는지’에 따라 결정한 부분이 많았다.
innerHTML로 템플릿을 넣어주고 있다.

무엇을 배웠는가?

This 바인딩을 배웠다.
성급한 추상화의 단점을 배웠다.
다음미션에서 옵저버 패턴을 학습하고 활용해봤다.
View에 있어서는 안되는 로직이 무엇인지 구분할 수 있다.
getter와 setter의 사용을 배웠다.
전역에 있다고 해서 다 전역’변수’는 아님을 배웠다.

앞으로 어떻게 다르게 할 것인가?

Promise 사용패턴을 더 익히겠다!

365kim · 2021-04-25T08:16:13Z

학습로그

This 바인딩

Javascript - 'this' 키워드로 객체에 접근하기

학습로그

옵저버 패턴

상태관리(Stage Management)와 옵저버패턴(Observer Pattern)

학습로그

배열 내장 메서드

JavaScript - 유용한 배열 내장 메서드(find, filter, every, some, reduce, repeat)

[2단계 - 자동차 경주 구현] 하루(김하루) 미션 제출합니다. #31

[2단계 - 자동차 경주 구현] 하루(김하루) 미션 제출합니다. #31

Conversation

365kim commented Feb 12, 2021 • edited Loading

요약

고민사항 / 미해결과제

365kim Feb 12, 2021 • edited Loading

Choose a reason for hiding this comment

질문드립니다! 🙋🏻‍♀️ - 데이터의 관리

Choose a reason for hiding this comment

365kim Feb 14, 2021 • edited Loading

Choose a reason for hiding this comment

365kim Feb 12, 2021 • edited Loading

Choose a reason for hiding this comment

질문드립니다! 🙋🏻‍♀️ - 시간지연의 실행주체

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ysm-dev 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

ysm-dev Feb 13, 2021 • 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

365kim commented Feb 14, 2021

📝 2단계 피드백 반영 목록

ysm-dev commented Feb 14, 2021

365kim commented Feb 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

365kim commented Mar 9, 2021

셀프 코드리뷰 🤔

잘한 것은 무엇인가?

아쉬운 점은 무엇인가?

무엇을 배웠는가?

앞으로 어떻게 다르게 할 것인가?

365kim commented Apr 25, 2021

학습로그

This 바인딩

태그

365kim commented Apr 25, 2021

학습로그

옵저버 패턴

태그

365kim commented Apr 25, 2021

학습로그

배열 내장 메서드

태그

365kim commented Feb 12, 2021 •

edited

Loading

365kim Feb 12, 2021 •

edited

Loading

365kim Feb 14, 2021 •

edited

Loading

365kim Feb 12, 2021 •

edited

Loading

ysm-dev left a comment •

edited

Loading

ysm-dev Feb 13, 2021 •

edited

Loading