[1단계 - 자동차 경주 구현] 병민(윤병인) 미션 제출합니다. #93

airman5573 · 2022-02-11T08:57:49Z

어제 계산기 미션을 늦게까지 하느라 자동차 경주를 너무 늦게 시작했습니다.
그래서 이번 미션은 시간내에 완수하지 못했습니다.

페어프로그래밍을 통해서도 많이 배웠습니다.
감사합니다 :D

commitlint, commitizen, cypress, eslint, prettier를 설정했다.

구현을 다 못했다

glassyi · 2022-02-12T05:29:37Z

index.html

+          <h1 class="text-center">🏎️ 자동차 경주 게임🏁</h1>
+          <p>5자 이하의 자동차 이름을 콤마로 구분하여 입력해주세요.</p>
+        </section>
+        <section class="mb-5">


Suggested change

<section class="mb-5">

<section class="mb-5">

<p>5자 이하의 자동차 이름을 콤마로 구분하여 입력해주세요.</p>

html 영역을 그룹핑할 때 그 안에 들어가는 요소들의 스타일이 동일한 경우 대체로 자식요소들이 동일한 위치에 있도록 하는게 좋아요.

마찬가지로 입력 필드에 대한 설명(또는 제목), 입력 필드, 버튼을 section 태그로 그룹핑했을 때, 자동차 이름 입력 필드 섹션과 시도 횟수 입력 필드 섹션에 자식 요소들이 동일하게 위치 하는게 더 일관성있는 코드가 되지 않을까요?

그리고 p태그보다 더 웹표준에 적합한, 입력 필드의 설명 또는 제목을 나타낼 수 있는 태그가 무엇일까요?

네, 이해했습니다. 수정하겠습니다.

p태그보다 label이 더 적합해보입니다.

glassyi · 2022-02-12T05:42:19Z

index.html

+            <button id="car-names-button" type="button" class="btn btn-cyan">확인</button>
+          </div>
+        </section>
+        <section id="racing-count-form" hidden>


hidden attribute를 사용하셨군요~ 👍 👍

glassyi · 2022-02-12T05:46:45Z

src/css/index.css

+  padding-top: 10px;
+  padding-left: 30px;
+  padding-right: 30px;
+  padding-bottom: 7px;


상하좌우 4방향 모두 padding을 주는 경우 shorthand properties(약식)으로 작성해 볼 수도 있겠어요~

네, 수정하겠습니다.

glassyi · 2022-02-12T05:58:29Z

src/js/constants/constants.js

+  CAR_RACING_STATUS: '.car-racing-status',
+};
+
+export const GAME = {


GAME 보다 좀 더 구체적인 상수명을 지어주는게 더 좋을 것 같아요~ 구성된 값들이 어떤것을 지칭 하는지 좀 더 생각해보면 좋겠네요!

네, 더 고민해보겠습니다.

glassyi · 2022-02-12T06:00:03Z

src/js/constants/constants.js

+  MIN_INPUT_COUNT: 1,
+};
+
+export const ERR_MESSAGE = {


가급적이면 단어를 줄이지 말고 작성하는것을 좀 더 추천합니다 🙂

네, 알겠습니다.

glassyi · 2022-02-12T06:04:42Z

src/js/index.js

+
+export default function racingCarGame() {
+  init();
+}


지금은 어디서 racingCarGame 모듈을 가져와서 쓰는 코드가 없어서 racingCarGame를 export해서 쓰려는 이유가 궁금하네요~ 😅

앗, 수정하겠습니다

glassyi · 2022-02-12T06:11:48Z

src/js/input/handleCarNames.js

+  return name.length >= MIN_CAR_NAME_LENGTH;
+}
+
+function isValidCarNames(carNames) {


isValidCarNames 함수가 현재 예외 조건 체크와 에러메세지를 보여주는 역할을 다 가지고 있는데요. 예외 조건 체크하는 함수와 에러메세지 처리 함수로 역할을 분리시켜보는게 어떨까요?

동감합니다.

혹시

function isValid(~) { ~~~ return { hasError: false, message: ~~~ } } const { hasError, message } = isValid(~); if (hasError) { alert(message); return; }

이런 방식은 어떨까요?

오 네~ 이런 방식도 괜찮습니다. 👍 여기서 추가로 제안을 해보자면 carName은 결국 MIN_CAR_NAME_LENGTH 이상이고 MAX_CAR_NAME_LENGTH 이하인 조건을 둘다 충족해야 하는거 아닐까요? 이 때 사용자에게 두개의 케이스를 안내할 수 있는 적절한 메세지 하나로 변경한다면 isValidCarNames 함수는 더 간결해 질 것 같은데 어떤가요?

glassyi · 2022-02-12T06:13:17Z

src/js/input/handleRacingCount.js

+import startRacingGame from '../game/startRacingGame.js';
+import $ from '../utils/dom.js';
+
+function isValidRacingCount(racingCount) {


링크
동일한 코멘트 내용이라 링크로 남겨놓아요~

glassyi · 2022-02-12T06:18:18Z

src/js/utils/setAttribute.js

@@ -0,0 +1,7 @@
+export function hideElement(element) {
+  return element.setAttribute('hidden', true);


hidden attribute로 처리하는 방식 좋습니다 👍

glassyi · 2022-02-12T06:59:17Z

test/test.spec.js

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


alert이 호출되었는지 작성해주셨군요~ 여기에 추가로 유효하지 않은 조건에 부합하는 에러메세지와 alert 메세지가 일치하는지도 시나리오를 추가하면 더 좋을 것 같습니다!

네, 수정하겠습니다

glassyi · 2022-02-12T07:19:54Z

src/js/init/cars.js

+export const cars = [];
+
+export function makeObjectCars(carNames) {
+  carNames.forEach((name) => cars.push(new Car(name)));


cars가 모듈로서 직접적으로 외부에서 쓰이면 관리하기가 어려워집니다. 어디서 변경될지 모르기 때문이죠. 그래서 이런 경우 보통은 객체나 클래스로 감싸서(데이터 캡슐화라고 하죠) 그 객체가 스스로 데이터를 자율적으로 관리하도록 해요. 그러면 외부에서 데이터를 쓰기 위해서는 그 데이터를 관리하는 객체를 통해서만 접근이 가능하기 때문에 어디서 어떻게 변경되는지 예측하기 쉬워집니다.

cars와 cars를 변경하는 makeObjectCars 메소드를 가지는 객체 또는 클래스를 만들어 적용해보세요~
추가로 makeObjectCars 네이밍도 좀 더 고민해보시면 좋겠네요~ 🙂

네, 더 고민해보겠습니다.

glassyi

병민님 안녕하세요! 리뷰를 맡게된 율리라고 합니다~ 😀

이전 미션을 수행하느라 이번 미션에서 시간이 부족 하셨나봐요.
그래도 여기까지 오느라 고생 많으셨습니다!

다음 스텝 또는 미션에서는 요구사항에 대한 기능구현에 목표를 두고 미션에 임해보셔요.
이번엔 아쉽게도 기능구현이 다 되어 있지 않아서 우선 작성해 주신 코드에서
코멘트 드릴 수 있는 내용으로 리뷰드려요~

우선 style을 구조적으로 잘 분리해 놓은 부분이 눈에 띄었습니다.
그리고 css 파일들이 대부분 마지막 줄에 new line 추가가 안되어 있더라구요. 수정 요청드려요!

전반적으로 디테일한 리뷰 내용은 코멘트로 달아두었으니 참고해 주세요~
보시면서 반영할 수 있는 부분들 위주로 개선해 보시면 좋을 것 같습니다.
확인 부탁드려요~! 🙂

es2022에서 lint가 작동하지 않는다. es2021과 (지원하는 global 기능중에서) 달라진 점이 없다 eslint/eslint#15580 8.9.0버전에서 es2022를 써도 되게끔 수정 했다고 하는데, 나는 아직 반영이 안되서 그냥 es2021을 사용한다

document에서부터 싹 검색해서 찾는것보다 빠르다

…설정한다

glassyi · 2022-02-17T01:06:08Z

이번 스텝 바로 머지하고 다음 스텝에서 추가 커밋들에 대한 리뷰를 같이 진행할게요~! 스텝2에서 뵈어요! 😀

airman5573 · 2022-02-17T06:56:40Z

이번 스텝 바로 머지하고 다음 스텝에서 추가 커밋들에 대한 리뷰를 같이 진행할게요~! 스텝2에서 뵈어요! 😀

네, 늦어져서 죄송합니다 ㅠ

airman5573 added 11 commits February 10, 2022 21:52

🚚 chore: 프로젝트 기본 환경 설정을 한다

c2509ce

commitlint, commitizen, cypress, eslint, prettier를 설정했다.

📝 docs(readme.md): 기능구현 목록을 추가한다

e2dbc24

📝 docs(readme.md): css관련 구현 항목을 추가한다

40be3e6

✨ feat(markup): 자동차 경주 template을 만든다

97a67b9

✨ feat(markup): spinner를 추가한다

7d47f8e

📝 docs(readme.md): 완료한 항목을 표시한다

2934725

♻️ refactor(css): css파일을 분리한다

2661ee1

✨ feat(car-game): 자동차의 이름을 입력받는다

327b211

✨ feat(car-game): 시도할 횟수를 입력받고 유효성 검사를 한다

cfc63dc

✏️ typo(eslintrc.js): 작은따움표를 사용한다

37a7d55

✨ feat(car-game): 횟수를 입력한후 게임을 시작한다

468f175

구현을 다 못했다

pocojang requested a review from glassyi February 11, 2022 09:13

pocojang assigned glassyi Feb 11, 2022

glassyi reviewed Feb 12, 2022

View reviewed changes

glassyi suggested changes Feb 12, 2022

View reviewed changes

airman5573 added 5 commits February 15, 2022 17:17

✏️ typo(index.html): lang을 kr -> ko로 변경한다

81b77a8

✏️ typo(package.json): description을 변경한다

a9f269f

🚚 chore(.eslintrc.js): 불필요한 설정을 삭제하고 각 설정에 대한 주석을 기재한다

b84b719

🚚 chore(.prettierrc.js): 각 설정에 대한 주석을 기재한다

1d4b1f7

airman5573 added 22 commits February 15, 2022 20:52

🚚 chore(pacakage.json): eslint버전을 업데이트한다

afbc4f7

✏️ typo(eslint): minor

5b4e4eb

♻️ refactor(structure): 코드의 흐름을 MVC패턴으로 재건축한다

58bca5f

💄 style(css): css파일을 재분류 스타일을 수정한다

5d791a0

💄 style(index.html): class와 구조를 수정한다

44f61bc

♻️ refactor(etc): class를 id로 변경한다

6bafe15

♻️ refactor(constants): html에서 새로 부여한 selector들을 상수로 등록한다

d143993

♻️ refactor(etc): 동적으로 view를 조작할것이기 때문에 기존에 static하게 작성한 마크업을 삭제한다

c7c2472

♻️ refactor(car.model): 더 적절한 변수명으로 변경한다

2724ec5

🐛 fix(style): 사용하지 않는 css파일을 import하지 않는다

7df9c32

♻️ refactor(util): $함수에 찾으려는 element의 root node를 설정할 수 있다

c623289

document에서부터 싹 검색해서 찾는것보다 빠르다

✨ feat(validation): 유효성 검사 실패시에 throw할 커스텀 에러 클래스를 추가한다

86c002b

✨ feat(validation): 유효성 검사후에 그 결과를 담는 클래스를 추가한다

d3e9bbc

✨ feat(car-names-input): 사용자로부터 자동차 경주에 등록할 자동차들의 이름을 입력받는다

ecffd2f

✨ feat(racing-count-input): 사용자로부터 자동차가 움직이려고 시도하는 횟수를 입력받는다

bf08972

♻️ refactor(racing-count-input): 자동차 이름 입력이 선행되어야 한다

f1448c8

✏️ typo(util): 파일 이름을 하이픈 스타일로 변경한다

c17c0c4

🚚 chore(eslint): class안에 static property를 사용하기 위해서 ecmaVersion을 최신으로 …

8972216

…설정한다

♻️ refactor(util): 여러개의 node도 찝을 수 있도록 개선한다

225f729

♻️ refactor(naming): element의 class를 변경한다

f693192

✨ feat(racing-screen): 게임을 시작하면 screen에 자동차들의 이동거리를 출력한다

3b95f0b

✨ feat(racing-game-prepare-form): 게임시작하면 경기에 필요한 정보들을 입력할 수 없도록 설정한다

af8de81

glassyi merged commit 4953ab5 into woowacourse:airman5573 Feb 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 자동차 경주 구현] 병민(윤병인) 미션 제출합니다. #93

[1단계 - 자동차 경주 구현] 병민(윤병인) 미션 제출합니다. #93

airman5573 commented Feb 11, 2022

glassyi Feb 12, 2022

airman5573 Feb 13, 2022

glassyi Feb 12, 2022

glassyi Feb 12, 2022

airman5573 Feb 13, 2022

glassyi Feb 12, 2022

airman5573 Feb 13, 2022

glassyi Feb 12, 2022

airman5573 Feb 13, 2022

glassyi Feb 12, 2022 •

edited

Loading

airman5573 Feb 13, 2022

glassyi Feb 12, 2022

airman5573 Feb 13, 2022

glassyi Feb 14, 2022

glassyi Feb 12, 2022

glassyi Feb 12, 2022

glassyi Feb 12, 2022

airman5573 Feb 13, 2022

glassyi Feb 12, 2022 •

edited

Loading

airman5573 Feb 13, 2022

glassyi left a comment

glassyi commented Feb 17, 2022

airman5573 commented Feb 17, 2022

	<section class="mb-5">
	<section class="mb-5">
	<p>5자 이하의 자동차 이름을 콤마로 구분하여 입력해주세요.</p>

		@@ -0,0 +1,7 @@
		export function hideElement(element) {
		return element.setAttribute('hidden', true);

[1단계 - 자동차 경주 구현] 병민(윤병인) 미션 제출합니다. #93

[1단계 - 자동차 경주 구현] 병민(윤병인) 미션 제출합니다. #93

Conversation

airman5573 commented Feb 11, 2022

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

glassyi Feb 12, 2022 • 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

glassyi Feb 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glassyi left a comment

Choose a reason for hiding this comment

glassyi commented Feb 17, 2022

airman5573 commented Feb 17, 2022

glassyi Feb 12, 2022 •

edited

Loading

glassyi Feb 12, 2022 •

edited

Loading