[윤장원]-vending-machine#4
Conversation
* 반환 버튼 유틸함수 * 투입 버튼 유틸함수 * 로그 함수 * 돈 자리수 계산 함수 * 물품 구매 함수
* 반환 핸들러, 투입 핸들러, 주문 핸들러
* index.html을 외부로 옮겨서 직관적으로 바꿈
| <div | ||
| class="vending-machine__calc-container flex flex-col gap-2 box-border p-4" | ||
| > | ||
| <div class="flex gap-2"> |
There was a problem hiding this comment.
사용자가 입력하고 제출하는 형태의 UI여서
form태그 사용하셔도 좋을거같습니다.
| @@ -0,0 +1,41 @@ | |||
| export const Items = [ | |||
There was a problem hiding this comment.
상수는 혹시 대문자로 시작하기로 규칙을 세우신 걸까요? 그렇다면 아래에 있는 btnStyle 도 같은 규칙으로 통일해주는 편이 좋을 것 같습니다. 다만, 첫 문자만 대문자로 사용하는 단어는 JS에서는 사실 클래스 이름에 주로 사용되는 규칙이라서 다른 방식을 고려하면 좋습니다. 널리 통용되는 방식 중 하나로는 대문자와 언더스코어(_) 문자만 사용하하는 게 있습니다.
https://javascript.info/variables#uppercase-constants
There was a problem hiding this comment.
Constants 전체 대문자로 한다는걸 놓친거 같습니다. 언더스코어는 명명은 몰랐는데 인사이트 감사합니다
| export const returnMoney = ( | ||
| amount, | ||
| totalBalanceView, | ||
| transactionLog, | ||
| transactionLogContainer, | ||
| ) => { |
There was a problem hiding this comment.
반환하는 함수가 로그 배열과 로그 컨테이너 엘리먼트의 참조까지 인수로 받는 건 너무 번거로워 보입니다. 다른 함수도 마찬가지고요. 이 부분을 어떻게 개선할 수 있을까요?
There was a problem hiding this comment.
transactionLog, transactionLogContainer 인수 부분을 addLog에서만 사용하게 해서 정리해 다시 커밋 했습니다.
| @@ -0,0 +1,125 @@ | |||
| // 돈 단위 3자리 계산 함수 | |||
| export const formatCurrency = (amount) => { | |||
| return amount.toString().replace(/\B(?=(\d{3})+(?!\d))/g, ','); | |||
There was a problem hiding this comment.
Intl 객체의 numberFormat을 사용해서 이 부분을 리팩토링 해볼 수 있을까요?
There was a problem hiding this comment.
Intl 객체를 잘 몰랐는데 이번기회에 한번 사용해보겠습니다.
| export const convertCurrency = (amount) => { | ||
| if (amount < 1000) { | ||
| return parseInt(amount, 10); | ||
| } | ||
|
|
||
| return parseInt(amount.replace(/,/g, ''), 10); | ||
| }; |
There was a problem hiding this comment.
코드로 봐서는 명확하지 않은데 amount는 숫자 타입인가요, 문자열 타입인가요?
주석에 있는 함수 설명이 더 명확해야 할 것 같습니다.
There was a problem hiding this comment.
주석을 통해 더 명확하게 작성해야할거 같습니다. 헷갈리는 부분이 있네요. 감사합니다. amount는 number입니다.
| export const findMinPrice = (items) => { | ||
| if (items.length === 0) return 0; | ||
|
|
||
| return Math.min(...items.map((item) => item.price)); |
There was a problem hiding this comment.
오 저도 min price를 상수로 빼지말고 찾아서 해야겠네요! 👍🏻👍🏻👍🏻👍🏻👍🏻👍🏻
No description provided.