[1단계 - 행운의 로또 미션] 하루(김하루) 미션 제출합니다. #7

365kim · 2021-02-17T13:22:29Z

Jbee님, 안녕하세요!

처음 인사드리는 🙋🏻‍♀️ 하루(@365kim) 입니다. 로또 미션 잘부탁드리겠습니다 👍👍👍
저는 👩🏻‍🦰 썬(@SunYoungKwon)과 페어 프로그래밍을 진행했습니다.

요약

데모　　: 로또미션 1단계 - 하루
구현 순서: 테스트 전체 작성 후, 각 테스트를 통과하기 위한 단위 기능 구현
구현 결과: 작성한 cypress 테스트 모두 통과 ✔

git clone https://github.com/365kim/javascript-lotto.git
cd javascript-lotto
git checkout 365kim
npm install
npm run cypress

1단계 - 구현기능 목록

로또 구입 금액을 입력받는다.
- 최소 화폐단위 미만의 자릿수가 포함된 경우 ➡️ alert 후 재입력 요청
- 1,000원 미만일 경우 ➡️ alert 후 재입력 요청
- 1,000원으로 나누어 떨어지지 않을 경우 ➡️ alert으로 거스름돈 금액을 알려주고 구매 계속 진행
금액으로 살 수 있는 갯수만큼 로또를 발급한다.
- 모든 로또를 자동구매 옵션으로 발급한다.
- 로또 한 장 당 번호의 개수는 6개로 한다.
- 번호는 1 ~ 45 사이 랜덤값으로 구성한다.
- 각 번호는 서로 중복되지 않는다.
- 각 번호는 오름차순으로 정렬되어 있다.
로또 발급 후 번호보기 토글 버튼을 클릭하면 복권번호 표시여부를 결정할 수 있다.
- 표시여부 기본값은 복권번호를 표시하지 않는 것으로 한다.

추후 구현 예정

2단계 - 당첨결과 기능
3단계 - 수동구매 기능
2단계까지는 페어와 함께, 3단계는 혼자 작성해서 리뷰요청 드릴 예정입니다 🙏

이상입니다! 시간 내서 리뷰해주셔서 감사드려요.
크고 작은 코멘트 많이 부탁드리겠습니다!
감사합니다 😆

+ 2021. 2. 22. 1단계 구조도 작성해보았습니다.🚀

365kim · 2021-02-18T10:46:46Z

src/js/components/PurchasedLotto.js

+  onToggleShowingNumbers({ target }) {
+    if (target.type === 'checkbox') {
+      target.checked ? this.showNumbers() : this.hideNumbers();
+    }
+  }


질문드립니다! 🙋🏻‍♀️ - 삼항연산자의 활용

22라인에서 depth를 줄일 수 있겠다고 생각해서 삼항연산자를 사용했습니다. 그런데 삼항연산자를 사용하면 값을 평가해서 반환한다는 점이 신경쓰여 질문드립니다. 평가할 반환값이 없고 showNumbers, hideNumbers 처럼 단지 statement의 실행을 목적으로도 삼항연산자를 사용해도 괜찮을지, 보편적인(?) 방법인지 궁금합니다...!

값을 평가해서 반환한다는 점

이 어떤 면에서 신경쓰였는지도 공유해주세요!ㅎㅎ
(showNumbers, hideNumbers는 무엇을 반환하나요?)

showNumbers() { this.$lottoTicketContainer.classList.add('flex-col'); $$('.lotto-numbers').forEach(($lottoNumbers) => $lottoNumbers.classList.remove('d-none')); } hideNumbers() { this.$lottoTicketContainer.classList.remove('flex-col'); $$('.lotto-numbers').forEach(($lottoNumbers) => $lottoNumbers.classList.add('d-none')); }

앗 제가 모호하게 말씀을 드렸군요, 죄송합니다! 🙏 저희가 작성한 showNumbers()와 hideNumbers() 모두 위와 같이 리턴값이 없습니다. 따라서 라인 22에서 삼항연산자가 결국 undefined라는 값으로 평가되는데, 과장 아닌 과장을 한다면 읽는 사람에 따라서 아래와 같이 보일 수도 있을 것 같다고 생각했습니다. undfined가 함수 중간에 둥둥 떠다니는 것처럼 보인다면 어색한 문장이 될 것 같아서요...! 어색하다고 느껴지시는지 아니신지가 궁금했습니다. 😅

onToggleShowingNumbers({ target }) { if (target.type === 'checkbox') { undefined; } }

개인 취향에 따라 다르겠지만 전 삼항연산자를 선호해요!

JaeYeopHan

1차 코멘트 남겼습니다~
react 유 경험자시군요.ㅎㅎ

JaeYeopHan · 2021-02-18T14:54:39Z

src/js/utils/DOM.js

+export const $ = (selector) => document.querySelector(selector);
+export const $$ = (selector) => document.querySelectorAll(selector);
+
+export const clearInput = ($target) => ($target.value = '');


clearInput이라고 하면 모든 input type에 대해 초기화가 될 것 같아요 ㅎㅎ checked도 defaultValue로 지정해준 값으로 돌아가거나 false가 되어야 할 것 같은?

disabled도 변경이 되었다면 clear되어야 할 것 같은데 어떻게 생각하시나요?

말씀해주신 내용을 듣고나서야 시야가 확대되었습니다! 감사합니다👍👍 해당 함수는 value 초기화 목적으로만 사용할거라 clearInputValue라고 이름 변경했습니다. 함수 네이밍에서 다른 효과가 연상 되지는 않는지 앞으로도 주의하겠습니다 :)

JaeYeopHan · 2021-02-18T14:57:04Z

src/js/components/PurchasedLotto.js

+  onToggleShowingNumbers({ target }) {
+    if (target.type === 'checkbox') {
+      target.checked ? this.showNumbers() : this.hideNumbers();
+    }
+  }


값을 평가해서 반환한다는 점

이 어떤 면에서 신경쓰였는지도 공유해주세요!ㅎㅎ
(showNumbers, hideNumbers는 무엇을 반환하나요?)

JaeYeopHan · 2021-02-18T14:59:51Z

src/js/components/PurchasedLotto.js

+
+  createLottoTicketHTML(lottoTicket) {
+    return `
+    <li class="mx-1 text-4xl d-flex items-center">


li 👏 👏 👏 👏 👏

감사합니다!

JaeYeopHan · 2021-02-18T15:00:14Z

src/js/components/PurchaseAmountInput.js

+    const change = purchaseAmount % LOTTO_PRICE;
+
+    if (change) {
+      alert(ALERT_MESSAGE.PURCHASE_AMOUNT_HAS_CHANGE(change));
+    }


정말 좋은 extra mile UX 라고 생각해요 ㅎㅎ 👏 👏 👏 👏 👏

감사합니다 ☺️☺️ 페어와 함께 시나리오 상상하면서 저런 흐름으로 작성했었습니다 :)

JaeYeopHan · 2021-02-18T15:02:05Z

src/js/components/PurchaseAmountInput.js

+    this.$purchaseAmountButton.addEventListener('click', this.onSubmitPurchaseAmount.bind(this));
+    this.$purchaseAmountInput.addEventListener('keydown', (e) => {
+      if (e.key === 'Enter') {
+        e.preventDefault();
+        this.onSubmitPurchaseAmount();
+      }
+    });
+  }
+
+  onSubmitPurchaseAmount() {
+    const purchaseAmount = this.$purchaseAmountInput.value;


form tag의 submit이벤트를 활용해보시는 것을 추천드립니다 ㅎㅎ

<form>의 submit 이벤트를 활용하니까 원래 있던 2가지 이벤트리스너 (click이벤트, Enter키이벤트)를 submit 이벤트리스너 하나로 해결할 수 있었습니다. 말씀해주신 덕분에 <form> 태그에 대해서 더 학습할 수 있었습니다! 감사합니다 👍👍 말씀해주신 내용 참고해서 <label> 태그의 자식으로 <input>을 넣어주었습니다. 함께 커밋된 CSS 파일은 이 부분에서 레이아웃이 바뀌어서 수정한 사항입니다. 또 문서에 있는 h1, h4 를 감싸는 <div>는 <section>으로 변경해 주었습니다. 웹표준에 대해서 짚어주셔서 감사합니다! ☺️

JaeYeopHan · 2021-02-18T15:05:34Z

src/js/components/PurchaseAmountInput.js

+
+    const change = purchaseAmount % LOTTO_PRICE;
+
+    if (change) {


개인 의견 차가 있을 부분이지만, 이 부분에 대해서 제 생각을 공유드려보면
condition으로 들어가는 값은 boolean type임이 명시적이면 명시적일수록 좋다고 생각해요!

truthy, falsy 값으로 condition check가 가능하더라도 boolean이 아닌 값을 condition에서 사용할 경우 식을 통해 boolean을 전달하는 것을 선호합니다.

말이 길었는데 그냥 다음이 더 명시적이라고 생각한다는 뜻입니다 ㅋㅋㅋ

if (change > 0) { ... }

// Boolean 타입이 아닌 truthy, falsy 조건식 if (change) {} // Boolean 타입인 조건식 if (!!change) {} if (change > 0) {} if (change !== 0) {} if (Boolean(change)) {}

말씀해주신 내용보고 if 조건식에 Boolean 타입으로 넘겨주는 방법을 찾아보니 방법이 정말 많네요..! 👨‍👩‍👦‍👦👨‍👩‍👦‍👦 페어와 함께 이야기해보니 역시 가독성도 좋고 숫자타입인 것도 보여주면서 불리언 타입으로 넘길 수 있는 리뷰어님께서 말씀해주신 방법이 가장 좋다고 생각해서 말씀해주신대로 수정했습니다.

JaeYeopHan · 2021-02-18T15:06:49Z

src/js/model/LottoTicket.js

+      return array;
+    }
+
+    if (array.indexOf(number) === -1) {


includes가 더 어울려보여요 ㅎㅎ

index를 반환받아서 활용할 것이 아니라면 indexOf 보다 includes가 더 어울리는 것이군요! 적절한 메서드 짚어주셔서 감사합니다 👍👍

…ection 태그 사용

365kim · 2021-02-19T17:08:28Z

Jbee🐝 님 안녕하세요!
빠르고 꼼꼼한 피드백 감사드립니다. 말씀해주신 방향 참고해서 페어와 함께 수정진행해 보았습니다!

p.s. 컴포넌트 구성을 리드해주신 제 페어가 리액트 유경험자이십니다,,,ㅎㅎ

📝 1단계 피드백 반영 목록

Input 태그의 value만 초기화해주는 함수는 역할에 맞게 함수명을 수정한다. (clearInput ➡️ clearInputValue)
HTML 태그는 웹표준을 준수해서 작성한다.
- form 태그에서 submit을 활용한다.
- input 태그는 label 태그와 연결한다.
- 문서의 있는 헤더태그 (h1, h4) 를 감싸는 블럭은 div 대신 section 태그를 사용한다.
조건문의 컨디션은 최대한 명시적으로 Boolean 타입으로 만들어준다.
더 적합한 배열 메서드를 적용한다. (indexOf ➡️ includes)

따로 질문드릴 창구도 만들어주시고 신경 많이 써주셔서 정말 감사합니다 ㅎㅎ
감사합니다 😆

JaeYeopHan

수고하셨어요!👍

365kim and others added 19 commits February 16, 2021 15:01

docs: 1단계 기능 구현목록 작성

897eca1

chore: 코드포매터, 린터 설정, 사이프레스 설치

f21d5f1

chore: package.json name, description 수정

ec9ce87

test: 입력된 로또 구입 금액이 로또 한 장의 금액보다 적을 경우

7f2c98e

test: 입력된 로또 구입 금액이 로또 한 장의 금액으로 나누어 떨어지지 않을 경우

bb2e39c

test: 입력된 로또 구입 금액에 화폐단위 미만의 자릿수가 포함된 경우

e444f59

test: 입력된 로또 구입 금액으로 발급한 로또를 화면에 표시

777255c

test: 입력된 로또 구입 금액으로 발급한 로또를 화면에 표시

4048b7d

test: 번호보기 토글이 비활성화 되어있는 상태에서 토글을 누른 경우

a11716e

test: 표시된 로또 번호의 개수, 중복여부, 범위 검사

07cccf5

test: 번호보기 토글이 활성화된 상태에서 토글을 누른 경우

c3cf005

feat: 입력된 로또 구입금액에 최소 화폐단위 미만의 자릿수가 포함된 경우 alert 구현

d487678

feat: 입력된 로또 구입 금액이 로또 한 장의 금액보다 적을 경우 alert 구현

3f964cc

feat: 입력된 로또 구입 금액이 로또 한 장의 금액으로 나누어 떨어지지 않을 경우 alert 구현

ee39bfa

chore: 가독성을 위해 alert 메세지에 개행 추가

ef8aa1c

docs: step1 구현목록 추가(로또 번호 중복여부 검사, 정렬)

6be533d

feat: 입력한 금액으로 발급된 로또를 화면에 표시

6dbf11a

feat: 번호보기 토글 구현

90c0c13

feat: 중복없이 로또번호를 생성하는 함수 구현

a14833a

pocojang requested a review from JaeYeopHan February 17, 2021 15:05

pocojang assigned JaeYeopHan Feb 17, 2021

365kim and others added 4 commits February 18, 2021 11:54

test: 로또를 재구입하면 기존 로또를 삭제하고 새로 구매한 로또 표시

56dde7e

fix: 번호보기 토글이 활성화된 상태에서 로또 재구입 시 번호가 사라지는 문제 해결

691c800

test: Enter키 이벤트로 로또를 구입할 경우

dc041fb

feat: 로또 구입금액 제출 Enter 키 이벤트리스너 구현

da88187

365kim commented Feb 18, 2021

View reviewed changes

JaeYeopHan suggested changes Feb 18, 2021

View reviewed changes

365kim added 3 commits February 19, 2021 21:12

refactor: clearInput -> clearInputValue 로 네이밍 변경

0cfd3fb

refactor: 웹표준 및 접근성에 맞도록 form 태그에 submit 적용, label 태그와 input 태그 연결, s…

fea675b

…ection 태그 사용

refactor: if문의 조건식 Number 타입이 아닌 Boolean 타입으로 전달

f011d91

refactor: indexOf 메서드 대신 includes 적용

a97b7d2

365kim requested a review from JaeYeopHan February 19, 2021 17:08

JaeYeopHan approved these changes Feb 20, 2021

View reviewed changes

JaeYeopHan merged commit 0bdd106 into woowacourse:365kim Feb 20, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 행운의 로또 미션] 하루(김하루) 미션 제출합니다. #7

[1단계 - 행운의 로또 미션] 하루(김하루) 미션 제출합니다. #7

365kim commented Feb 17, 2021 •

edited

365kim Feb 18, 2021

JaeYeopHan Feb 18, 2021

365kim Feb 18, 2021

JaeYeopHan Feb 20, 2021 •

edited

JaeYeopHan left a comment

JaeYeopHan Feb 18, 2021

365kim Feb 19, 2021

JaeYeopHan Feb 18, 2021

JaeYeopHan Feb 18, 2021

365kim Feb 19, 2021

JaeYeopHan Feb 18, 2021

365kim Feb 19, 2021

JaeYeopHan Feb 18, 2021

365kim Feb 19, 2021

JaeYeopHan Feb 18, 2021

365kim Feb 19, 2021 •

edited

JaeYeopHan Feb 18, 2021

365kim Feb 19, 2021

365kim commented Feb 19, 2021 •

edited

JaeYeopHan left a comment

[1단계 - 행운의 로또 미션] 하루(김하루) 미션 제출합니다. #7

[1단계 - 행운의 로또 미션] 하루(김하루) 미션 제출합니다. #7

Conversation

365kim commented Feb 17, 2021 • edited

요약

1단계 - 구현기능 목록

추후 구현 예정

Choose a reason for hiding this comment

질문드립니다! 🙋🏻‍♀️ - 삼항연산자의 활용

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JaeYeopHan Feb 20, 2021 • edited

Choose a reason for hiding this comment

JaeYeopHan 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

365kim Feb 19, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

365kim commented Feb 19, 2021 • edited

📝 1단계 피드백 반영 목록

JaeYeopHan left a comment

Choose a reason for hiding this comment

365kim commented Feb 17, 2021 •

edited

JaeYeopHan Feb 20, 2021 •

edited

365kim Feb 19, 2021 •

edited

365kim commented Feb 19, 2021 •

edited