[2단계 - 자동차 경주 구현] 지그(송지은) 미션 제출합니다. #36

zigsong · 2021-02-13T11:43:29Z

안녕하세요☺️ 자동차 미션 2단계 리뷰 요청 드립니다!

✅ Todos

자동차 경주 게임의 턴이 진행 될 때마다 1초의 텀(progressive 재생)을 두고 진행한다.
정상적으로 게임의 턴이 다 동작된 후에는 결과를 보여주고, 2초 후에 축하의 alert 메세지를 띄운다.

📚 어려웠던 점

setInterval을 이용해서 1초마다 spinner와 진행 상황(arrow)를 번갈아 출력하다 보니, dom을 조작하는 데 있어 어려움이 있었습니다.
특히 moveCars 함수의 moveInterval에서, 즉시 실행 함수와 clearinterval을 함께 써야 하는 동시에 this binding까지 해야 해서 까다로운 부분들이 있었습니다.

🤔 궁금한 점

setInterval과 달리 requestAnimationFrame 함수가 브라우저 repaint 지점에 함수를 작동시킨다고 하는데, 이게 확실히 더 성능이 좋은 편인가요? 사실 이번에 처음 들어본 기능인데, 실제로도 많이 쓰이는지 궁금합니다!
moveCars 함수의 moveInterval 함수가 이것저것 깔끔하게 구현하려다보니 조금 방대해졌는데, 더 나은 방법이 있을지 알고 싶습니다.

자세한 내용은 코멘트에 달아 놓았습니다.
리뷰 감사드립니다 😆

zigsong · 2021-02-13T11:50:51Z

cypress/integration/racing.spec.js

+    const carsNum = 4;
+    const tryCount = 5;


carsNum과 tryCount는, 각각
clickAfterTypeCar();와 clickAfterTypeTryCount();에서 사용하는 상수를 가져온 것입니다.

요 부분은 제일 상단의 전역부분에서

const DEFAULT_TRY_COUNT = 5 const DEFAULT_CAR_UNITS = 4

와 같은 형태로 명명해두고,

clickAfterTypeTryCount(tryCount = DEFAULT_TRY_COUNT) {...}

와 같은 형태로도 쓰고, 아래에서도 쓰는 건 어떨까요~?

zigsong · 2021-02-13T11:51:35Z

cypress/integration/racing.spec.js

    clickAfterTypeCar();
    clickAfterTypeTryCount();

+    const tryCount = 5;
+    cy.wait(tryCount * 2000);


spinner 1000ms, arrow 1000ms가 출력되기 때문에 더한 값인 2000만큼 지연이 발생하도록 했습니다.

해당 부분을 * 2000같은 공식을 밑에서도 써야하니, 위와 같이 전역으로 값을 빼는것이 좋을 것 같습니다!

const SPINNER_SEC = 1000 const ARROW_DISPLAYING_SEC = 1000 const DEFAULT_TRY_COUNT_TIME = DEFAULT_TRY_COUNT * (SPINNER_SEC + ARROW_DISPLAYING_SEC) ... cy.wait(DEFAULT_TRY_COUNT_TIME)

zigsong · 2021-02-13T11:51:46Z

src/js/index.js

+        this.UIController.printProgress(car, isCarMoved);
+      });
+      return moveEverySecond.bind(this);
+    }).bind(this)(), 2000)


spinner 1000ms, arrow 1000ms가 출력되기 때문에 더한 값인 2000만큼 지연이 발생하도록 했습니다.
콜백 내에서 Racing 객체의 this.cars를 업데이트해야하기 때문에 어쩔 수 없이 bind(this)를 사용했는데,
좋지 않은 방식이라면 지적 부탁드립니다!

2000이라는 숫자는 위에서 언급한 전역변수로 대체하면 좋을 것 같습니다 👍

setInterval이 발생하는 시점에서 this context를 찾을 수 없으니 arrow function을 사용하지 못하셨나요? 아마 가능할 것 같은데... arrow function을 사용해보는 건 어떨까요?

arrow function으로 작성해보려 햇는데 setInterval 안에 moveEverySecond와 같은 함수의 이름이 있어야 80줄과 같이 return을 할 수 있더라고요 😬 가장 처음에는 2초의 지연 없이 즉시 실행되게 하려면 저렇게 리턴을 해야 한다고 하는데, (아니면 함수 바디의 내용을 함수 선언 이전에 한번 더 작성해서 미리 한 번 호출한다든가..? 근데 이건 좀 중복되고 지저분해 보여서요.) arrow function으로는 어떻게 작성해야 하는지 오래 고민해 보았지만 답을 찾지 못해서 어쩔 수 없이 bind(this)를 사용했습니다..!

첫 회차 실행을 위해 함수가 더 무거워진 느낌인데, 첫회 실행을 하고 2000 sec을 주시는게 더 좋아보입니다!

zigsong · 2021-02-13T11:53:27Z

src/js/index.js

+        clearInterval(moveInterval);
+        this.getWinners();
+        return;


clearInterval 후에도 한 번 더 spinner와 arrow가 출력되고 있어, 바로 return해버리게끔 했습니다.
보기에는 정상 작동하는데, 혹시 문제가 있을까요?

좋습니다! 👍 👍

보통 위의 bad case에서 return을 하도록 코딩을 작성하면, 하위의 로직을 돌지 않고 해당 scope 밖으로 던져지므로 성능 (엄청 미비하지만...) 상에서도 좋습니다.

이렇게 return으로 bad case를 먼저 선언하고, 메인 로직 (지금과 같은 상황은 아래의 로직들이 되겠죠) 을 짜는게 일반적인 방법입니다! 문제 없어보입니다!

유조님께 먼저 작성드린 피드백 내용인데, 참고하시면 좋을 것 같습니다 😄

zigsong · 2021-02-13T11:54:09Z

src/js/models/Car.js

+      return true;
    }
+    return false;


bool값을 받아 해당 턴에 car가 move하면 화살표를 하나 더하고, 아니라면 무시하는 것으로 바꿔보았습니다.

멤버변수에 isMove을 하나 선언하고, move에서 값을 가져오기보다 get isMoving() 을 만들어서 가져오는게 더 낫지 않을까요~? 이렇게 되면 move 함수는 한번에 두 가지 일을 하게 되네요!

함수는 단순히 딱 하나의 일만 해야하는게 좋습니다 👍 추후에 테스트 코드 검증을 위해서라도요!

zigsong · 2021-02-13T11:55:18Z

src/js/racingUI.js

+    }
+  }
+
+  showResult(cars) {


실시간으로 결과를 출력하는 printProgress와 구분하기 위해 함수 이름을 바꿨습니다.
showResult 안에서는 spinner, arrow와 상관 없는 car들의 이름과 그 컨테이너 객체만 뜨도록 했습니다.

Vallista

지그님! 2단계도 수고 많으셨습니다!

전반적으로 코드가 1단계에 비해 깔끔해진 걸 보니 좋네요!! 👍 👍
상세한 코드 리뷰는 코드 라인에 달아놓았습니다! 해당 피드백을 수정해주시면 감사하겠습니다!

그리고 질문주신 내용에 대한 피드백입니다!

setInterval과 달리 requestAnimationFrame 함수가 브라우저 repaint 지점에 함수를 작동시킨다고 하는데, 이게 확실히 더 성능이 좋은 편인가요? 사실 이번에 처음 들어본 기능인데, 실제로도 많이 쓰이는지 궁금합니다!

setInterval과 requestAnimationFrame은 쓰임의 용도가 다른 친구입니다! 물론 동일한 결과를 위해 이용할 수는 있겠지만요.

setInterval은 브라우저의 API로 브라우저 내부의 카운트를 진행해 콜백을 진행하는 함수입니다.
requestAnimationFrame은 이름에서 보이는 것 처럼, animation을 javascript로 제어하기 위해 탄생된 친구입니다.

requestAnimationFrame은 repaint가 되기 전에 호출된 함수를 실행하고, 그 후에 repaint를 진행합니다. 이렇게 되는 이유는 애니메이션의 특성 때문입니다. 애니메이션이 가장 부드럽게 보이려면 보고있는 디스플레이의 주사율과 최대한 비슷해야합니다. 최대한 비슷해야 하는 이유는 디스플레이 주사율이란 초당 깜빡이는 횟수를 나타냅니다. 즉 최대한 깜빡이는 시점이 비슷하다면 사용자 입장에서는 최대한 부드러워 보인다는 의미입니다. 지그님이 보고있는 모니터의 주사율이 60hz 라면, (1초에 60번 깜빡인다면) 웹 브라우저는 해당 하드웨어에 반응해 1초에 60번 requsetAnimationFrame을 호출할 것 입니다 (repaint가 되기 전, requestAnimationFrame을 루프에서 실행했다면, 최대한 브라우저의 깜빡임과 화면의 주사율을 갖게 하기 위해) repaint가 되기 전에 호출되어 repaint를 실행한다는 것은, 최대한 브라우저 단위에서 깜빡이는 시점을 비슷하게 가져가려는 노력일 것 입니다.

그래서 게임에서는 이러한 모니터의 주사율과 그래픽 카드의 연산으로 하여 게임이 진행되는 프레임레이트(fps, framerate)를 같게 하기위해서 AMC - FreeSync, NVIDIA - GSYNC 와 같은 기능을 모니터에 탑재하기도 하죠. (모니터에 그래픽카드 연산과 동일하게 맞추는 하드웨어를 넣습니다. 가격은 역시 비싸집니다..) 또 다른 이야기로는 아이폰 X ~ 11까지 디스플레이 주사율이 144여서 많은 부드러움이 있었으나, 12로 오면서 주사율을 60으로 하향해서 많은 욕을 먹곤 했었죠. 그만큼 디스플레이의 주사율은 사용성에 많은 차이를 줍니다.

이 requestAnimationFrame은 비단 위에서 언급한 컴퓨터 비전(computer vision)의 영역도 있지만 웹 브라우저상에서도 해당 콜백을 보관(eventloop 상에서의 requestAnimationFrame queue)했다가 다시 로직으로 돌려보내는(callstack - execute context queue) 행위에도 웹 브라우저의 queue 우선순위 라던가, 다양한 공부할 부분이 존재합니다. 이러한 공부를 진행하면 eventloop의 queue의 우선순위(microtask queue > request animation frame queue > task queue, 브라우저마다 다를 수 있음)를 이용해서 로직의 우선순위를 조정할 수도 있습니다.

딴 길로 많이 빠졌네요.. 결론을 말씀드리면 requestAnimationFrame은 repaint가 진행되기 이전에 작동되며, 많이 쓰이고, 특히 애니메이션 라이브러리 까보면 십중팔구 다 이거씁니다. 그래서 적극적으로 이용하시면 좋습니다. 같이 공부하면 좋을 것들은 EventLoop, Promise 등이 있겠습니다.

moveCars 함수의 moveInterval 함수가 이것저것 깔끔하게 구현하려다보니 조금 방대해졌는데, 더 나은 방법이 있을지 알고 싶습니다.

요 것은 코드 라인에서의 세부 코맨트에 답변을 달아두었습니다!

긴 글 읽어주셔서 감사합니다.

Vallista · 2021-02-15T16:41:51Z

cypress/integration/racing.spec.js

+    const carsNum = 4;
+    const tryCount = 5;


요 부분은 제일 상단의 전역부분에서

const DEFAULT_TRY_COUNT = 5 const DEFAULT_CAR_UNITS = 4

와 같은 형태로 명명해두고,

clickAfterTypeTryCount(tryCount = DEFAULT_TRY_COUNT) {...}

와 같은 형태로도 쓰고, 아래에서도 쓰는 건 어떨까요~?

Vallista · 2021-02-15T16:43:25Z

cypress/integration/racing.spec.js

    clickAfterTypeCar();
    clickAfterTypeTryCount();

+    const tryCount = 5;


요기에도 중복으로 쓰이니 중복으로 쓰이지 않도록 전역으로 관리하도록 합시다! (위와 같은 내용입니다!)

Vallista · 2021-02-15T16:45:39Z

cypress/integration/racing.spec.js

    clickAfterTypeCar();
    clickAfterTypeTryCount();

+    const tryCount = 5;
+    cy.wait(tryCount * 2000);


해당 부분을 * 2000같은 공식을 밑에서도 써야하니, 위와 같이 전역으로 값을 빼는것이 좋을 것 같습니다!

const SPINNER_SEC = 1000 const ARROW_DISPLAYING_SEC = 1000 const DEFAULT_TRY_COUNT_TIME = DEFAULT_TRY_COUNT * (SPINNER_SEC + ARROW_DISPLAYING_SEC) ... cy.wait(DEFAULT_TRY_COUNT_TIME)

Vallista · 2021-02-15T16:45:54Z

cypress/integration/racing.spec.js

+    clickAfterTypeTryCount();
+
+    const tryCount = 5;
+    cy.wait(tryCount * 2000);


여기도 위와 동일합니다!

Vallista · 2021-02-15T16:46:05Z

cypress/integration/racing.spec.js

    clickAfterTypeCar();
    clickAfterTypeTryCount();
+
+    const tryCount = 5;
+    cy.wait(tryCount * 2000);


여기두 동일합니다!

Vallista · 2021-02-15T16:48:32Z

src/js/index.js

+        clearInterval(moveInterval);
+        this.getWinners();
+        return;


좋습니다! 👍 👍

보통 위의 bad case에서 return을 하도록 코딩을 작성하면, 하위의 로직을 돌지 않고 해당 scope 밖으로 던져지므로 성능 (엄청 미비하지만...) 상에서도 좋습니다.

이렇게 return으로 bad case를 먼저 선언하고, 메인 로직 (지금과 같은 상황은 아래의 로직들이 되겠죠) 을 짜는게 일반적인 방법입니다! 문제 없어보입니다!

Vallista · 2021-02-15T17:14:22Z

src/js/index.js

+        clearInterval(moveInterval);
+        this.getWinners();
+        return;


유조님께 먼저 작성드린 피드백 내용인데, 참고하시면 좋을 것 같습니다 😄

Vallista · 2021-02-15T17:16:48Z

src/js/index.js

+        this.UIController.printProgress(car, isCarMoved);
+      });
+      return moveEverySecond.bind(this);
+    }).bind(this)(), 2000)


2000이라는 숫자는 위에서 언급한 전역변수로 대체하면 좋을 것 같습니다 👍

setInterval이 발생하는 시점에서 this context를 찾을 수 없으니 arrow function을 사용하지 못하셨나요? 아마 가능할 것 같은데... arrow function을 사용해보는 건 어떨까요?

Vallista · 2021-02-15T17:19:03Z

src/js/models/Car.js

+      return true;
    }
+    return false;


멤버변수에 isMove을 하나 선언하고, move에서 값을 가져오기보다 get isMoving() 을 만들어서 가져오는게 더 낫지 않을까요~? 이렇게 되면 move 함수는 한번에 두 가지 일을 하게 되네요!

함수는 단순히 딱 하나의 일만 해야하는게 좋습니다 👍 추후에 테스트 코드 검증을 위해서라도요!

Vallista · 2021-02-15T17:20:50Z

src/js/racingUI.js

@@ -33,19 +36,47 @@ export default class RacingUI {
    document.querySelector(className).style.display = '';
  }

-  showProgress(cars) {
+  toggleElementDisplay(className, show) {


show의 상태를 받기보다, 특정한 상태로 함수를 만드는게 좋겠습니다.

showElement() {...} hideElement(){...}

형태로요! 이렇게 되면 비즈니스 로직에서 toggleElementDisplay가 어디서 꺼졌고.. 어디서 켜졌는데 true false를 백날 봐야하는 단점이 있어요.. 이렇게보다 hideElement를 검색해서 한번에 보는게 더 낫지 않을까요~?

사실 toggleElementDisplay의 경우 기존에 있던 hideElement, showElement와 다르게 동일한 className을 갖는 복수의 dom에 대한 display를 바꿔주는 거라, 함수 바디에서 for loop을 돌기 때문에 여러 줄의 코드가 중복되어 하나로 묶고자 했던 것이었습니다. 그런데 t/f를 매번 봐야 하는 문제는 있겠네요 😅 말씀주신 대로 수정해보겠습니다!

zigsong · 2021-02-16T00:41:00Z

src/js/models/Car.js

+  get isMoved() {
+    return this._isMoving;
  }


고쳐보았습니다!

zigsong · 2021-02-16T00:41:58Z

src/js/models/Car.js

+  get name() {
+    return this._name;
  }
-
-  getPosition() {
-    return this.position;
+  
+  get position() {
+    return this._position;
  }

-  getIsWinner() {
-    return this.isWinner;
+  get isWinner() {
+    return this._isWinner;
  }


다른 멤버변수들에 대해서도 getter로 변경하고, 중복된 이름을 방지하기 위해 변수명에는 언더바를 붙였습니다.
이렇게 써도 괜찮을까요?

좋은것 같습니다! 멤버변수에 _를 붙이는 건 많은 팀에서 하는 코딩 컨벤션 중 하나입니다!

Vallista · 2021-02-16T10:28:28Z

src/js/index.js

-      this.cars.forEach(car => car.move());
-    }
-    this.UIController.showProgress(this.cars);
+    const moveInterval = setInterval((function moveEverySecond() {


moveCars = () => { const moveEverySecond = (interval) => { if (this.tryCount === 0) { interval && clearInterval(interval) this.getWinners(); return; } this.tryCount++; this.cars.forEach(car => { car.move(); const isCarMoved = car.isMoved; this.UIController.printProgress(car, isCarMoved); }) } moveEverySecond() const interval = setInterval(() => moveEverySecond(interval), DELAY.TURN_TIME) }

이런형태로 짜도 될 것 같네여

좋은 방법인 것 같습니다! 감사합니다 ☺️

zigsong · 2021-02-16T13:29:14Z

requestAnimationFrame()에 대해서도 이해하기 쉽게 정말 상세하게 설명해주셔서 감사합니다 😊🙃

Vallista · 2021-02-16T14:16:42Z

수고 많으셨습니다~

zigsong

3/9 자동차 경주 셀프 리뷰
😬😎😵

zigsong · 2021-03-09T03:07:02Z

cypress/integration/racing.spec.js

@@ -10,7 +16,7 @@ describe('자동차 경주', () => {
    cy.get('.car-name-btn').click();
  }

-  function clickAfterTypeTryCount(tryCount = 5) {
+  function clickAfterTypeTryCount(tryCount = DEFAULT_TRY_COUNT) {
    if (tryCount) {


파라미터에 디폴트 값을 받고 있으니 tryCount가 존재하는지 확인하는 if문은 없어도 될 것 같아요!

zigsong · 2021-03-09T03:09:29Z

cypress/integration/racing.spec.js

+    clickAfterTypeCar();
+    clickAfterTypeTryCount();
+
+    for (let i = 0; i < DEFAULT_TRY_COUNT; i++) {


let i for 문은 지양하는 게 좋습니다.
[...Array(DEFAULT_TRY_COUNT)].map(() => ...)으로 바꿔보는 건 어떨까요?

zigsong · 2021-03-09T03:10:35Z

cypress/integration/racing.spec.js

+
+    cy.wait(DEFAULT_TRY_COUNT_TIME);
+
+    cy.get('.restart-btn').click();
    resetUI();


여기서 resetUI()를 호출하면 테스트하는 의미가 사라지지 않을까요? 🤔

zigsong · 2021-03-09T03:12:18Z

src/js/index.js

+    };
+
+    moveEverySecond();
+    const moveInterval = setInterval(() => moveEverySecond(moveInterval), DELAY.TURN_TIME);


export const delay = ms => new Promise(resolve => { setTimeout(resolve, ms); });

와 같은 delay 유틸을 구현해줘도 좋을 것 같습니다!

zigsong · 2021-03-09T03:13:23Z

src/js/index.js

+      return b.position - a.position;
    })

-    let maxPosition = sortedCars[0].getPosition();
+    let maxPosition = sortedCars[0].position;
    for (let car of this.cars) {
-      if (car.getPosition() === maxPosition) {
+      if (car.position === maxPosition) {
        car.wins();
      }
    }

-    const winners = this.cars.filter(car => car.getIsWinner()).map(car => car.getName());
+    const winners = this.cars.filter(car => car.isWinner).map(car => car.name);


const maxPosition = Math.max(...this.cars.map((car) => car.position)); return this.cars .filter((car) => car.position === maxPosition) .map((car) => car.name);

zigsong · 2021-03-09T03:15:20Z

src/js/racingUI.js

+    for (let i = 0; i < allElements.length; i++) {
+      allElements[i].style.display = '';
+    }


[...Array(allElements.length)].map(() => ...)

zigsong · 2021-03-09T03:15:34Z

src/js/racingUI.js

+    for (let i = 0; i < allElements.length; i++) {
+      allElements[i].style.display = 'none';


[...Array(allElements.length)].map(() => ...)

zigsong · 2021-03-09T03:16:13Z

src/js/racingUI.js

+    if (!document.querySelector(className)) {
+      return;
+    }


선택자가 없어서 발생하는 에러의 경우 그냥 return하지 말고 throw Error를 이용하여 개발자 콘솔에 로그를 남기면 디버깅하기 좋습니다 👾

zigsong · 2021-03-09T03:16:54Z

src/js/racingUI.js

+          <div class="forward-icon mt-2">⬇️️</div>
+        `)
+      }
+    }, 1000)


매직넘버 대신 상수로 관리해주세요 😮

zigsong · 2021-03-09T03:16:59Z

src/js/racingUI.js

+
+    setTimeout(() => {
+      alert('축하합니다 🎉')
+    }, 2000);


매직넘버 대신 상수로 관리해주세요 222 😮

zigsong · 2021-04-18T04:33:37Z

RacingCar 학습로그

함수의 네이밍

행동을 먼저 말하고, 뒤에 서사를 풀어쓰는 것이 많은 경우의 코딩 컨벤션이다.
ex) typeCarAndClick보다는 clickAfterTypeCar
getXXX의 의미를 명확히 알고 사용하자. class를 객체로 생성하여 사용하는 곳에서 가져올 때, 또는 DOM 객체를 접근할 때 주로 사용한다.
ex) Car 인스턴스 생성 시 getCars()보다는 createCars()
ex) getTryCount 대신 mountTryCount

class의 getter와 setter

getter로 class 멤버 변수 가져오기

class property는 외부에서 바로 접근하지 못하도록 하는 것이 좋다. getter를 사용하여 한번 함수로 래핑한 후 가져오자.

class Car {
    constructor(name, position) {
        this._name = name;
        this.position = position;
    }

    get name() {
        return this._name;
    }
    
    get position() {
        return this._position;
    }
}

setter 사용은 지양하기

class Car {
    constructor(name, position) {
        // ...
        this.position = position;
    }

    // bad
    set position() {
        this._position += 1;
    }

    // good
    move() {
         this._position += 1;
    }
}

setInterval과 requestAnimationFrame

setInterval

브라우저 API로, 브라우저 내부의 카운트를 진행해 콜백을 진행한다. (Macrotask queue로, 가장 나중에 실행된다)

const moveEverySecond = (interval) => {
  if (this.tryCount === 0) {
    interval && clearInterval(interval)
    this.getWinners();
    return;
  }
  this.tryCount--;
  this.cars.forEach(car => {
      // ... 
  })
};

moveEverySecond();
const moveInterval = setInterval(() => moveEverySecond(moveInterval), DELAY.TURN_TIME);

requestAnimationFrame

animation을 JavaScript로 제어하기 위해 사용한다.
repaint가 되기 전에 호출된 함수를 실행하고, 그 후에 repaint를 진행한다. (requestAnimationFrame queue)
애니메이션을 가장 부드럽게 보이게 하기 위해 사용한다. 디스플레이의 주사율(초당 깜빡이는 횟수)과 가장 비슷한 시점에 호출된다.

링크

기타

테스트코드 분리

레이아웃 관점과 비즈니스 로직으로 테스트 코드를 분리하면 보다 직관적인 테스트가 가능하다.

mounting 시점에서의 이벤트 핸들링

많은 라이브러리 또는 프레임워크는 render 단계에서 DOM 객체를 생성, mount 단계에서 이벤트를 바인딩한다. 따라서 vanillaJS로 개발 시 하나의 component에서 모든 addEventListener들을 한 곳에서 처리해주는 것은 자연스러운 패턴이다.

DOM 접근 예외처리

document.querySelector(className)에서 해당 객체가 존재하지 않을 때를 대비한 예외처리를 하자.

for loop 대신 Array 내장 객체 사용하기

// bad
for (let i = 0; i < DEFAULT_TRY_COUNT; i++) { ... }

// good
Array.from({ length: DEFAULT_TRY_COUNT }, () => { ... })

zigsong added 3 commits February 13, 2021 20:04

2단계 todos 정리

3e43a7d

feat: 레이싱 진행 시 1초의 레이턴시를 두고 spinner와 전진 상황 출력

4bb068e

feat: 레이싱 결과 출력 후 2초 뒤에 축하의 alert 메시지 띄우기

697adfe

zigsong commented Feb 13, 2021

View reviewed changes

pocojang requested a review from Vallista February 13, 2021 12:27

pocojang assigned Vallista Feb 14, 2021

Vallista suggested changes Feb 15, 2021

View reviewed changes

zigsong added 3 commits February 16, 2021 09:07

refactor: pr review 반영 - delay 시간 상수화

04e9f2d

refactor: pr review 반영 - Car 객체 getter 사용

d2bdc65

refactor: pr review 반영 - toggleDisplay를 show, hide로 대체

107387d

zigsong commented Feb 16, 2021

View reviewed changes

zigsong requested a review from Vallista February 16, 2021 00:42

Vallista approved these changes Feb 16, 2021

View reviewed changes

refactor: pr review 반영 - moveCars() 내 setInterval 로직 수정

d1b1187

zigsong requested a review from Vallista February 16, 2021 13:30

Vallista merged commit 68812bb into woowacourse:zigsong Feb 16, 2021

zigsong commented Mar 9, 2021

View reviewed changes

		for (let i = 0; i < allElements.length; i++) {
		allElements[i].style.display = 'none';

[2단계 - 자동차 경주 구현] 지그(송지은) 미션 제출합니다. #36

[2단계 - 자동차 경주 구현] 지그(송지은) 미션 제출합니다. #36

Conversation

zigsong commented Feb 13, 2021 • edited Loading

✅ Todos

📚 어려웠던 점

🤔 궁금한 점

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

Vallista 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

Choose a reason for hiding this comment

zigsong commented Feb 16, 2021

Vallista commented Feb 16, 2021

zigsong 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

zigsong commented Apr 18, 2021 • edited Loading

RacingCar 학습로그

함수의 네이밍

class의 getter와 setter

getter로 class 멤버 변수 가져오기

setter 사용은 지양하기

setInterval과 requestAnimationFrame

setInterval

requestAnimationFrame

링크

기타

테스트코드 분리

mounting 시점에서의 이벤트 핸들링

DOM 접근 예외처리

for loop 대신 Array 내장 객체 사용하기

zigsong commented Feb 13, 2021 •

edited

Loading

Vallista left a comment •

edited

Loading

zigsong commented Apr 18, 2021 •

edited

Loading