[이효원] JS 자판기 미션 제출 #17
Conversation
hhhyyo
commented
Jul 24, 2022
hhhyyo
commented
| const name = productNameInput.value; | ||
| const price = productPriceInput.value; | ||
| const quantity = productQuantityInput.value; |
There was a problem hiding this comment.
이 부분을 if 문 바깥으로 보내면 if 문의 조건절에도 변수를 활용할 수 있을거라 생각해쓴ㄴ데 일부러 스코프를 줄이기 위해 안쪽에 위치시킨 것일까요?
| productManageItem.classList.add('product-manage-item'); | ||
| productManageName.classList.add('product-manage-name'); | ||
| productManagePrice.classList.add('product-manage-price'); | ||
| productManageQuantity.classList.add('product-manage-quantity'); | ||
| productManageName.textContent = name; | ||
| productManagePrice.textContent = price; | ||
| productManageQuantity.textContent = quantity; | ||
| productManageItem.append(productManageName); | ||
| productManageItem.append(productManagePrice); | ||
| productManageItem.append(productManageQuantity); |
There was a problem hiding this comment.
아직은 3개라 크게 공수가 들지 않았지만 반복되는 작업에 forEach문을 쓸 수 있을 것 같아요
There was a problem hiding this comment.
그러려면 미리 각 item, name, price에 대한 데이터를 객체 형태로 두고 key value 짝을 지어주어야 하긴 할테지만...
hustle-dev
left a comment
hustle-dev
left a comment
There was a problem hiding this comment.
공통 요구사항엔 원래 index.html 파일을 수정할 수 없는 조건하에 어떻게 코드를 작성하실지 궁금했는데 못보게 되어 아쉽네요... 😢
과제하시느라 고생하셨습니다!
| const vendingMachineCharge = document.getElementById('vending-machine-charge'); | ||
| const charge = document.getElementById('charge'); | ||
| const productPurchaseItems = document.getElementById('product-purchase-items'); | ||
| const chargeAmount = document.getElementById('charge-amount'); |
There was a problem hiding this comment.
document.getElementById 이 코드가 전체적으로 계속해서 쓰이는 것 같은데 간단하게 함수로 만들어서 사용하면 더 보기 좋을것 같아요
| const productPurchaseItems = document.getElementById('product-purchase-items'); | ||
| const chargeAmount = document.getElementById('charge-amount'); | ||
|
|
||
| productPurchase.addEventListener('submit',(e)=>{ |
There was a problem hiding this comment.
html 코드를 보면 form 형식으로 만들어서 이벤트를 거신것 같은데
요구사항에 추가하기 버튼이 있다는 것은 거기에 이벤트를 걸으라는 것을 의도했다는 생각이 들었어요.
그래서 임의의 form태그를 만들어서 하는것도 좋지만 요구조건대로 하는게 맞지 않을까? 생각이 들었습니다
There was a problem hiding this comment.
저는 오히려 이 부분에 대해 form 태그를 사용하는 것을 유도한게 아닌가? 라는 생각을 했었어요.
만약 form태그를 사용하지 않고 button에 이벤트를 바인딩한다면 html코드에서 어떤 요소들을 추가하는 버튼인지 유추하기 어려울 것 같아요.
하지만 form 태그로 응집화해두면 form태그 하위의 input들을 submit한다는 것을 유추할 수 있으니 기능을 더 명시적으로 알 수 있지 않을까라고 생각해서 저도 form으로 묶어서 사용했었습니다.
| <ul> | ||
| <li><button id="product-purchase-menu">상품 관리</button></li> | ||
| <li><button id="vending-machine-manage-menu">잔돈 충전</button></li> | ||
| <li><button id="product-add-menu">상품 구매</button></li> |
There was a problem hiding this comment.
이 부분 저도 처음에 이렇게 작성했는데 상품 구매가 product-purchase-menu 이더라구여
아마 순서를 바꾸어서 작성하신것 같아요
| const productPriceInput = document.getElementById('product-price-input'); | ||
| const productQuantityInput = document.getElementById('product-quantity-input'); | ||
|
|
||
| if(productNameInput.value && productPriceInput.value && productQuantityInput.value){ |
There was a problem hiding this comment.
nameInput 엔 trim 메서드도 같이 사용하면 좋을것 같아요
| const vendingMachineCoin50Quantity = document.getElementById('vending-machine-coin-50-quantity'); | ||
| const vendingMachineCoin10Quantity = document.getElementById('vending-machine-coin-10-quantity'); | ||
|
|
||
| const old500Quantity = parseInt(vendingMachineCoin500Quantity.textContent); |
There was a problem hiding this comment.
보통 parseInt는 특정 진수의 정수로 반환할때 많이 쓰이는것 같아서 이 경우에 Number를 사용하는게 더 좋아보이네요
|
|
||
| const vendingMachineChargeInput = document.getElementById('vending-machine-charge-input'); | ||
|
|
||
| let money = vendingMachineChargeInput.value/1; |
There was a problem hiding this comment.
나누기 1을 한것은 값을 Number로 바꾸기 위함인가요? 그런 경우라면 Number를 붙이는게 더 가독성 측면에서 좋은것 같다는 의견입니다.
|
|
||
| const targetItem = e.target.parentNode.parentNode; | ||
| const price = parseInt(targetItem.querySelector('.product-purchase-price').textContent); | ||
| const $quantity = targetItem.querySelector('.product-purchase-quantity'); |
There was a problem hiding this comment.
다른 변수명과 달리 이 변수명 앞에 달러사인을 붙인 이유가 있을까요? 궁금합니다.
sonwonjae
left a comment
sonwonjae
left a comment
There was a problem hiding this comment.
전반적으로 반복되는 코드가 많은 점이 아쉬웠어요.
이 부분은 한결님 조언대로 반복문을 사용하되 util형태로 만들어서 사용해도 좋았을 것 같아요!
또 탭 버튼을 눌렀을때 페이지 변환을 구현하는 것도 이 과제의 묘미 중 하나였는데 이 부분을 제대로 볼 수 없어서 아쉬웠던 것 같습니다.
그래도 업무 보시면서 과제하기 힘드셨을텐데 고생하셨습니다.
| const productPurchaseItems = document.getElementById('product-purchase-items'); | ||
| const chargeAmount = document.getElementById('charge-amount'); | ||
|
|
||
| productPurchase.addEventListener('submit',(e)=>{ |
There was a problem hiding this comment.
저는 오히려 이 부분에 대해 form 태그를 사용하는 것을 유도한게 아닌가? 라는 생각을 했었어요.
만약 form태그를 사용하지 않고 button에 이벤트를 바인딩한다면 html코드에서 어떤 요소들을 추가하는 버튼인지 유추하기 어려울 것 같아요.
하지만 form 태그로 응집화해두면 form태그 하위의 input들을 submit한다는 것을 유추할 수 있으니 기능을 더 명시적으로 알 수 있지 않을까라고 생각해서 저도 form으로 묶어서 사용했었습니다.
4 participants
