[이정민] JS 자판기 미션 제출 #18
Conversation
hanana1253
left a comment
hanana1253
left a comment
There was a problem hiding this comment.
바닐라JS로 컴포넌트 만드는 블로그 글이 정말 훌륭한 레퍼런스임을 오늘도 깨닫고 갑니다...
유틸 함수들을 적절하게 잘 만들어 두신 것이 인상적이었습니다.
sonwonjae
left a comment
sonwonjae
left a comment
There was a problem hiding this comment.
저는 함수 컴포넌트로 구현했는데 정민님이 클래스 컴포넌트로 구현하신것을 보고 많이 배웠습니다.
업무 보시면서 과제하기 힘드셨을텐데 고생많으셨습니다.
| this.domNode.insertAdjacentHTML("beforeend", '<main class="main"><main>'); | ||
|
|
||
| const $main = $(".main"); |
There was a problem hiding this comment.
현재 app컴포넌트에는 setState가 없어서 문제가 없지만 만약 setState로 인해 componentDidMount가 실행된다면 이 부분으로 인해
<div>
<main class="main"></main>
<main class="main"></main>
</div>이런식으로 중복 생성이 있을것 같아요.
이런 점을 개선하기위해 클래스 property로 main DOM 노드를 하나 생성하고 이를 재활용하는 방법은 어떨까요?
| if (this.state.currentTab === "product-add-menu") { | ||
| new ProductManage($main, { | ||
| productList: this.state.productList, | ||
| addProduct: this.addProduct.bind(this), | ||
| }); | ||
| } else if (this.state.currentTab === "vending-machine-manage-menu") { | ||
| new ChangeFill($main, { | ||
| holdingMoney: this.state.holdingMoney, | ||
| coinList: this.state.coinList, | ||
| chargeChange: this.chargeChange.bind(this), | ||
| }); | ||
| } else { | ||
| new ProductPurchase($main, { | ||
| inputMoney: this.state.inputMoney, | ||
| productList: this.state.productList, | ||
| coinList: this.state.coinList, | ||
| payCoinList: this.state.payCoinList, | ||
| holdingMoney: this.state.holdingMoney, | ||
| changeInputMoney: this.changeInputMoney.bind(this), | ||
| chargeChange: this.chargeChange.bind(this), | ||
| purchaseProduct: this.purchaseProduct.bind(this), | ||
| returnChange: this.returnChange.bind(this), | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
switch문이 안티패턴으로 알려져 있긴 하지만 이 부분에서는 페이지를 구분하는 것이니 명확히 어느 페이지의 태스크임을 구분하기 위해 switch, case문으로 변경하는건 어떨까요?
아니면 마지막 else문을 그냥 else가 아닌 else if로 currentTab을 명시하는 것도 좋은 것 같습니다.
There was a problem hiding this comment.
헛 switch 문이 안티패턴이라는 것은 처음들었는데 저도 이부분을 보면서 if else보다는 switch문이 가독성이 더 좋을 것 같다는 생각을 했습니다~
| return ` | ||
| <section class="change-fill"> | ||
| <h2>자판기 동전 추가하기</h2> | ||
| <input id="vending-machine-charge-input" type="number" /> |
There was a problem hiding this comment.
input에 step 속성을 사용하면 강제적으로 step단위의 숫자만 입력가능할 수 있게 됩니다.
isMultipleOfTen메서드를 사용하기보다 애초에 10단위의 숫자만 입력가능하게 하는건 어떻게 생각하실까요?
| const $price = Number($("#product-price-input").value); | ||
| const $quantity = Number($("#product-quantity-input").value); |
There was a problem hiding this comment.
$는 DOM노드에 주로 붙이셨던데 여기서 변수명에 $를 붙이신 이유가 있을까요?
|
간결하게 잘 구현하셨네요 |
4 participants
구조
index.js파일을 진입점으로 하여 컴포넌트 구조로 구조설계를 해보았습니다.core파일에 컴포넌트 추상화를 위한Component.js파일을 만들어 모든 컴포넌트에서 이를 상속하여 추상화된 형식으로 작동합니다.참고자료
Vanilla Javascript로 웹 컴포넌트 만들기