-
Notifications
You must be signed in to change notification settings - Fork 14
[임소형] JS 자판기 미션 제출 #20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: sohyung
Are you sure you want to change the base?
Conversation
- Added ts but didn't used
- Able to set multiple +, - calculating - Calculate total when get *, / - [Error] Calculate problem when has *, / with only one number
- Calculate total with every thrid operator - Reinitialize with result - If entering number from result, it reinitialize with entered number
- ts, webpack, eslint, prettier - reset css
- Store cares about states - Render with innerHTML - Markup pages
- 반환, 추가 ...
Vendingmachine
| $returnChargeButton.setAttribute('id', 'coin-return-button'); | ||
| $returnChargeButton.innerHTML = '반환하기'; | ||
| $returnChargeButton.addEventListener('click', () => { | ||
| const coins = [...COINS].sort((a, b) => b - a) as Coin[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요 코드는 최소한의 동전으로 반환하려고 sort 해주는 부분인가요? 원본 COINS[] 도 단위가 큰 순서로 구성되어 있어서 sort해도 변화가 없을 것 같아요! (만약 단위를 기준으로 내림차순하기 위한 의도라면 개인적으로는 reverse()가 좀더 가독성이 좋고 의미가 명확해 보일 것 같다는 소소한 의견입니당)
| export const ManageMenu = (store: store) => { | ||
| const charge = store.store.charge; | ||
|
|
||
| const $manageMenu = document.createElement('div'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코드 전체적으로 createElement, className을 사용하는 부분이 많다면 따로 유틸 함수로 분리해도 좋을 것 같아용
| let coinToPay = 0; | ||
|
|
||
| store.setProduct = store.store.product.map(({ name, price, quantity }) => { | ||
| if (name !== nameToBuy) return { name, price, quantity }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
상품명으로 구매하려고하는 상품인지 확인하려면 상품을 등록할 때 상품명이 중복되지 않게 검증해주는 로직이 있어야할 것 같아요! (만약 사용자가 같은 상품인데 가격이나 수량을 다르게해서 여러 개 입력했다면, 그 중 하나만 구매해도 다른 상품들도 구매되는 일이 생길 것 같아요.) 혹은 상품마다 고유한 ID를 주는 방법도 좋을 것 같습니당
| const nameToBuy = $row.querySelector('.product-purchase-name').dataset | ||
| .productName; | ||
|
|
||
| let coinToPay = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
여기서 coinToPay를 사용하지 않고 수량을 업데이트한 것 처럼 보유 금액도 업데이트 할 수 있을 것 같아용 (+ 현재는 보유 금액을 전부 사용하면 구매할 수 없는 처리가 안되어있는 것 같아서 그 부분도 추가하면 좋을 것 같습니당)
// let coinToPay = 0;
store.setProduct = store.store.product.map(({ name, price, quantity }) => {
if (name !== nameToBuy) return { name, price, quantity };
// coinToPay = price;
store.setInsertedCoin = store.store.insertedCoin - price;
return { name, price, quantity: quantity - 1 };
});
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
임시 변수를 없애면 자다가도 떡이 나온다. - 마틴
| }; | ||
|
|
||
| export const handleCharge = (store: store, el: HTMLInputElement) => { | ||
| const chargeAmount = +el.value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
저도 최근에 알게된건데 HTMLInputElement 인터페이스에 valueAsNumber, valueAsDate 라는 것두 잇다구하네용 공유합니다
(el.valueAsNumber 이런식으루 사용하는 건가봐요)
|
|
||
| export const handleAddMenu = (store: store, input: HTMLInputElement) => { | ||
| handleAddMenuError(+input.value); | ||
| store.setInsertedCoin = store.store.insertedCoin + +input.value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
store.store.insertedCoin 처럼 깊이 참조하는 부분들이 있으면 코드를 읽구 이해하는데 어려움이 생기는 것 같아용 당장 해결법은 떠오르지 않지만,,, 참조 뎁스를 줄이면 좀 더 가독성이 좋아질 것 같습니당
jiwonkim0131
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
소형니임~ CSS까지 야무지게 적용되어있네요👍 넘 고생하셨습니당🥰
테스트 결과
애초에 잔돈 갯수가 랜덤으로 충전되는데 어떻게 테스트가 되는지 모르겠음...
폴더 구조
컴퍼넌트 단위로 구성하고자 하였으나 VDOM, createElement 구현 등을 해낼 여력이 없었습니다.
store에서 클래스로 전체 상태를 관리합니다. index.ts 에서 store 의 인스턴스를 생성한 후 App 컴퍼넌트의 인자로 전해주는 것으로 전체 프로젝트에 옮겨가게 됩니다.
store,element 요소를 다루는 핸들러는 훅으로 분리하였습니다.