[1단계 - 자동차 경주 미션] 블링(김요욱) 미션 제출합니다. by uk960214 · Pull Request #77 · woowacourse/javascript-racingcar

uk960214 · 2022-02-11T08:10:00Z

소감

처음하는 페어 미션이었는데 페어와 호흡이 아주 잘 맞아서 순조롭게 진행할 수 있었다. 처음에만 하더라도 페어 프로그래밍의 효율성에 대해서 의구심을 품었었는데 아직 그 정확한 이유는 모르겠지만 더 간결하고 효율적으로 끝났다. 어떤 부분이 효과적, 효율적인지는 앞으로 몇 차례 더 해봐야 알 수 있을 것 같다.
테스트 코드를 작성하는 것은 어렵지만 재미있는 일이었다. 처음 써보는 도구기 때문에 아직은 익숙해질 필요가 있지만 어디까지 리팩터링이 가능한지 실험해보는 것도 재미있었다.

질문

현재 구현한 미션에는 재시작 로직을 새로고침으로 하는 것이 큰 문제가 없다고 판단했는데
추후에 발생할 수 있는 문제 점으로는
1. 페이지의 깜박임.
2. 전체 앱에 부수효과 발생 (상태 초기화 등)

이외에 단점 / 부작용이 있는지 궁금합니다.

비즈니스 로직과 UI view를 명확하게 분리하려고 노력했는데 지금 단계에서 미흡한 부분은 어디일까요?

좋은 주말 보내세요!

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

Co-authored-by: juunzzi juunzzi@users.noreply.github.com

- cypress 설정 - 자동차 이름 입력 검증 테스트 작성 - 횟수 입력 검증 테스트 작성 Co-authored-by: juunzzi juunzzi@users.noreply.github.com

@app/index.js, app/car.js - 게임 클래스 및 자동차 클래스 구현 - DOM 불러오기 및 이벤트 핸들러 연결 - 자동차 이름 값 입력 시 문자열 처리 후 자동차 객체 생성 @utils.js - 문자열 처리 함수 구현 Co-authored-by: juunzzi juunzzi@users.noreply.github.com

Co-authored-by: juunzzi juunzzi@users.noreply.github.com

Co-authored-by: juunzzi [juunzzi@users.noreply.github.com](mailto:juunzzi@users.noreply.github.com)

Co-authored-by: juunzzi juunzzi@users.noreply.github.com

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

구현 Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

게임 진행 테스트 - 랜덤 함수 검증 부분 제거 - 확정적인 우승자 검증 아닌 우승자 존재하는지 검증 코드로 수정 재시작 테스트 - 게임 진행 후 재시작 버튼 클릭 시 렌더링 된 부분이 모두 제거되는 지 테스트 추가 Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

EungyuCho

안녕하세요. 리뷰어 아사입니다!

우선 질문해주신 점에 대해서 생각해본 점을 알려드릴게요.

현재 구현한 미션에는 재시작 로직을 새로고침으로 하는 것이 큰 문제가 없다고 판단했는데
추후에 발생할 수 있는 문제 점으로는
페이지의 깜박임.
전체 앱에 부수효과 발생 (상태 초기화 등)
이외에 단점 / 부작용이 있는지 궁금합니다.

새로고침으로 상태를 재고침하는 부분에 있어서는 추가적인 문제점이 떠오르지는 않네요.
1번에서 말씀하신 페이지가 다시 로딩되는 부분에 있어 브라우저를 다시 렌더링해야한다는 성능적인 관점에서는 앱의 로딩 시간에 따라 새로고침 방법을 다시 생각해 볼 수 있겠네요 ㅎㅎ

비즈니스 로직과 UI view를 명확하게 분리하려고 노력했는데 지금 단계에서 미흡한 부분은 어디일까요?

너무 잘 작성해주셨네요! 🥇
분리가 잘 되어있어 구조에 관련해서 코멘트 드릴만한 부분이 보이지 않아요.. 😅:🙇

테스트 코드에서 given when then을 주석으로 조건을 구분지어주셔서 테스트코드 가독성이 좋았어요.
javascript 문법 숙련도가 대단하세요!
질문에서 질문해주신 부분이 좋았어요. 앱이 커짐에 따라 발생할 리스크를 생각하시다니 👍

아래 몇가지 코멘트를 남겼는데 한번 확인해주시고 개선할만하다고 생각하시는 부분에 대해서는 개선해보거나 코멘트 남겨주시면 좋을 것 같아요!
고생하셨습니다 :)

EungyuCho · 2022-02-11T14:57:22Z

+  const carNameInputProcess = (nameInput) => {
+    cy.get(`#${DOM.CAR_NAME_INPUT_ID}`).type(nameInput);
+    cy.get(`#${DOM.CAR_NAME_BTN_ID}`).click();
+  };
+
+  const countInputProcess = (countInput) => {
+    cy.get(`#${DOM.COUNT_INPUT_ID}`).type(countInput);
+    cy.get(`#${DOM.COUNT_BTN_ID}`).click();
+  };
+
+  const setAlertStub = () => {
+    const alertStub = cy.stub();
+    cy.on('window:alert', alertStub);
+    return alertStub;
+  };
+
+  // then function
+  const doesShowAlert = ({ inputSelector, inputValue, btnSelector, alertStub }) => {
+    // when
+    cy.get(inputSelector).type(inputValue);
+
+    // then
+    cy.get(btnSelector)
+      .click()
+      .then(() => {
+        expect(alertStub).to.be.called;
+      });
+  };
+
+  const isInitialStatus = () => {
+    cy.get(`#${DOM.CAR_NAME_INPUT_ID}`).should('not.have.text');
+    cy.get(`.${DOM.CAR_PROGRESS_CLASS}`).should('not.exist');
+    cy.get(`.${DOM.WINNER_CONTAINER_ID}`).should('not.exist');
+    cy.get(`#${DOM.COUNT_INPUT_ID}`).should('not.be.visible');
+  };


전역적인 사용될만한 함수의 경우 Cypress Command로 분리해도 좋을 것 같아요!
support 폴더에 있는 command.js에서 전역 커스텀 커맨드를 등록할 수 있어요.
Docs

command로 분리할 수 있었네요! 좋은 정보 감사합니다!
일단 자동차 이름 입력 함수와 횟수 입력 함수는 확실히 분리하는 것이 좋다고 생각해서 분리했습니다.

하나 확신이 없는 부분은 세 번째로 분리한 alert가 뜨는 입력 처리 함수를 분리한 부분인데
이것도 위의 두 가지와 마찬가지로 분리하는 게 맞는 걸까요?
위의 두 함수는 어떤 작동을 반복하는 함수였다면 이 함수는 expect가 포함된 검증 부분인 게 고민스럽습니다..

그대로 사용하시는 것도 괜찮아보이긴 하지만 expect를 하는 것 외에 기존 커맨드와 다른점이 없다면 첫번째, 두번째 커맨드에서 구현해도 괜찮을 것 같아요.

객체형태의 인자로 받아서 경고창을 표기할 것인지에 대한 인수로 받으면 좋을 것 같아요.
기존의 값과 함께 객체형태로 받으면 더 명확하게 이해할 수 있다고 생각해요.

EungyuCho

안녕하세요 블링님!
코멘트드린 내용을 잘 반영해주셨네요.
고생하셨습니다 👍

블링님이 코멘트 주신 부분은 별도로 코멘트 남겨드렸고 추가적으로 개선해보면 좋을만한 부분들을 코멘트로 남겼습니다. 참고해서 개선할만한 부분은 개선해보시면 좋을 것 같아요!

추가로 남긴 코멘트들의 범위가 넓지 않아서 다음 스텝에서 확인해봐도 괜찮을 것 같아 바로 머지하도록 하겠습니다.
다음 스텝도 잘 부탁드리겠습니다! 😄

EungyuCho · 2022-02-13T15:19:40Z

+/cypress/videos
+/cypress/fixtures
+# /cypress/support
+/cypress/support/index.js


커맨드 등록을 하신 commands.js가 cypress/support/index.js를 통해 import되다보니 이 부분을 gitignore에서 빼주셔야해요!

EungyuCho · 2022-02-13T15:26:07Z

+  cy.get(`#${DOM.COUNT_BTN_ID}`).click();
+});
+
+Cypress.Commands.add('inputShowsAlert', ({ inputSelector, inputValue, btnSelector, alertStub }) => {


alertStub를 인자로 주입받을 필요가 있을까요?

EungyuCho · 2022-02-13T15:34:48Z

+  cy.get(btnSelector)
+    .click()
+    .then(() => {
+      expect(alertStub).to.be.called;


alert가 표시되는지 보다 해당 동작을 했을 때 알맞은 에러메세지가 표기되었는지 검증해보면 좋을 것 같아요

EungyuCho · 2022-02-13T15:46:11Z

+        <section id="input-field">
+          <form id="car-name-input-form">
+            <label for="car-name-input">5자 이하의 자동차 이름을 콤마로 구분하여 입력해주세요.</label>
+            <div id="car-name-input-field">
+              <input id="car-name-input" type="text" />
+              <button id="car-name-btn">확인</button>
+            </div>
+          </form>
+          <form id="count-input-form">
+            <label for="count-input">시도할 횟수를 입력해주세요.</label>
+            <div id="count-input-field">
+              <input id="count-input" type="number" />
+              <button id="count-btn">확인</button>
+            </div>
+          </form>
+        </section>
+        <section id="result-field">
+          <section id="game-progress">
+          </section>
+          <section id="winners">
+          </section>
+        </section>


section태그를 사용 시 해당 section을 식별할 요소를 포함시켜주는 걸 추천드립니다! MDN Sectoin 사용일람

이 글을 한번 참고해주세요 💡

This reverts commit d53541a.

uk960214 and others added 24 commits February 10, 2022 11:48

build: cypress, eslint, prettier 설치, 설정

c91aaaa

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

style: 마크업 및 스타일 구현

2c8a6c9

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

docs(README.md): 기능 구현 목록 작성

5d426c7

Co-authored-by: juunzzi juunzzi@users.noreply.github.com

test(input): 이름과 횟수 입력값 검증 테스트 작성

639bef2

- cypress 설정 - 자동차 이름 입력 검증 테스트 작성 - 횟수 입력 검증 테스트 작성 Co-authored-by: juunzzi juunzzi@users.noreply.github.com

feat(checkNames): 자동차 이름 길이 검증 매서드 구현 및 오류 처리

4ec43a0

Co-authored-by: juunzzi juunzzi@users.noreply.github.com

feat(countInput): 횟수 입력 및 저장 구현

db28c0b

Co-authored-by: juunzzi [juunzzi@users.noreply.github.com](mailto:juunzzi@users.noreply.github.com)

feat(checkNames): 횟수 입력 검증 매서드 구현 및 오류 처리

aaa5ae5

Co-authored-by: juunzzi juunzzi@users.noreply.github.com

refactor: airbnb 컨벤션 적용

cc86b9e

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

test(game): 게임 실행 테스트 작성

77ef5d3

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

feat(simulate): 게임 실행

45c13de

구현 Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

feat(render): 게임 결과 렌더링 구현

8438a7a

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

test: 게임 실행테스트 코드 수정

91be7c7

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

feat(restart): 재시작 버튼 구현

9994542

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

feat(countInput): 자동차 이름 미 입력 시 횟수 입력란 숨기기 구현

c11dd29

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

feat(random): 랜덤 숫자 함수 자체 구현

5c94287

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

feat(inputButtonDisable): 게임 작동 후 입력 버튼 비활성화

80c3dc3

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

refactor: 문자열 및 숫자 상수화

d504de2

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

refactor: view와 game 클래스 리팩터링

f41931a

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

feat(generateCarId): 자동차 아이디 생성 함수 구현

e761e1f

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

fix: 자동차 이름 입력 오류 시에도 횟수 입력란 표시되는 오류 수정

bdfbb03

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

refactor: 테스트 코드 리팩터링

7ffa499

refactor: carId 생성 시 while(true) 패턴 제거

11d436b

Co-authored-by: juunzzi <juunzzi@users.noreply.github.com>

pocojang requested a review from EungyuCho February 11, 2022 09:19

pocojang assigned EungyuCho Feb 11, 2022

EungyuCho suggested changes Feb 11, 2022

View reviewed changes

uk960214 added 3 commits February 13, 2022 21:03

docs: 구현 완료한 요구사항 체크

bd5d30a

refactor: car id들의 저장 형식을 set으로 변경

786e8ae

refactor: 최대 이동거리 산출 코드 리팩터링

6d5f492

uk960214 added 4 commits February 13, 2022 21:20

refactor: 자동차 이름 최대 길이 확인 함수 명칭 변경

a0f2e03

refactor: querySelector를 함수로 분리

0950888

refactor: 전역 사용 가능한 테스트 함수를 command로 분리

7d23a13

refactor: 오류를 발생시키는 케이스의 작동 테스트 케이스를 command로 분리

17950bf

EungyuCho approved these changes Feb 13, 2022

View reviewed changes

EungyuCho merged commit d53541a into woowacourse:uk960214 Feb 13, 2022

jhy979 added a commit to jhy979/javascript-racingcar that referenced this pull request Feb 13, 2022

Revert "[1단계 - 자동차 경주 미션] 블링(김요욱) 미션 제출합니다. (woowacourse#77)"

3003b22

This reverts commit d53541a.

EungyuCho mentioned this pull request Feb 19, 2022

[2단계 - 자동차 경주 구현] 유세지(김용래) 미션 제출합니다. #127

Merged

Conversation

uk960214 commented Feb 11, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

소감

질문

Uh oh!

EungyuCho left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

EungyuCho Feb 11, 2022

Choose a reason for hiding this comment

Uh oh!

uk960214 Feb 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EungyuCho Feb 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

EungyuCho left a comment

Choose a reason for hiding this comment

Uh oh!

EungyuCho Feb 13, 2022

Choose a reason for hiding this comment

Uh oh!

EungyuCho Feb 13, 2022

Choose a reason for hiding this comment

Uh oh!

EungyuCho Feb 13, 2022

Choose a reason for hiding this comment

Uh oh!

EungyuCho Feb 13, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

uk960214 commented Feb 11, 2022 •

edited

Loading

uk960214 Feb 13, 2022 •

edited

Loading

EungyuCho Feb 13, 2022 •

edited

Loading