[tip-calculator-app] modi(경주원) 미션 제출합니다.#4
Conversation
j8won
changed the title
j8won
requested review from
2yunseong,
KINHYEONJI,
MINJOO-KIM,
baegyeong and
loopy-lim
loopy-lim
left a comment
loopy-lim
left a comment
There was a problem hiding this comment.
이번 미션 바쁘실텐데 수고하셨습니다!
항상 코드를 작성할 때 Reset을 어떻게 하면 좋을지 생각해보는데, 그럴때 저는 데이터를 새로운 객체로 만들어서 해야하지 않을까를 생각합니다.
하지만 style같은 것도 초기화를 하고 싶은데 어떻게 하는 것이 좋을것 같나요? 저는 일단 show, hide라는 함수를 나누어서 reset에 넣어서 진행할것 같습니다.
| const billInput = document.querySelector(".bill-input"); | ||
| const billAlert = document.querySelector(".bill-alert"); | ||
|
|
||
| const tipRatioInputs = document.querySelectorAll(".select-tip-button"); | ||
| const customTipRatioInput = document.querySelector(".custom-tip-input"); | ||
|
|
||
| const peopleInput = document.querySelector(".people-input"); | ||
| const peopleAlert = document.querySelector(".people-alert"); | ||
|
|
||
| const tipPerPersonOutput = document.querySelector(".tip-per-person"); | ||
| const totalPerPersonOutput = document.querySelector(".total-per-person"); | ||
| const resetButton = document.querySelector(".reset-button"); |
There was a problem hiding this comment.
의미에 맞게 나누어서 띄어쓰기를 한 것이 보기 좋은 것 같습니다.
| <input | ||
| type="text" | ||
| class="calculator-input bill-input" | ||
| placeholder="0" | ||
| oninput="this.value = this.value.replace(/[^0-9.]/g, '').replace(/(\..*)\./g, '$1');" | ||
| /> |
There was a problem hiding this comment.
이건 뭔가요??
저... 신기해요... oninput에 replace 2번에...저 이 코드 해석이 안되요....
There was a problem hiding this comment.
숫자만 입력하고 싶은데 type="number"로 할 때, 옆에 버튼이 생기는 게 싫어서 입력 정규식을 사용했습니다. 그런데... 저도 검색해서 사용했던 거라 자세히 보지 않았는데... 이 정규식은 소수 하나만 허용하는 정규식이라고 하네요... 미스테이크...
| let bill = 0; | ||
| let tipRatio = 0; | ||
| let people = 0; | ||
| let tipPerPerson = 0; | ||
| let totalPerPerson = 0; |
There was a problem hiding this comment.
변순 직관적이라서 좋지만, 관련 있는 데이터는 묶어서 데이터를 관리하는 것이 좋습니다.
.idea/tip-calculator-app.iml
Outdated
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <module type="WEB_MODULE" version="4"> | ||
| <component name="NewModuleRootManager"> | ||
| <content url="file://$MODULE_DIR$"> | ||
| <excludeFolder url="file://$MODULE_DIR$/temp" /> | ||
| <excludeFolder url="file://$MODULE_DIR$/.tmp" /> | ||
| <excludeFolder url="file://$MODULE_DIR$/tmp" /> | ||
| </content> | ||
| <orderEntry type="inheritedJdk" /> | ||
| <orderEntry type="sourceFolder" forTests="false" /> | ||
| </component> | ||
| </module> No newline at end of file |
There was a problem hiding this comment.
IDE 환경설정 파일은 안올리는게 맞는 것 같습니다! (.gitignore제외)
| billAlert.style.visibility = "hidden"; | ||
| e.target.style.border = "none"; | ||
|
|
||
| bill = Number(e.target.value); |
There was a problem hiding this comment.
단항 연산자 + 를 사용하면 명시적으로 숫자형으로 변환할 수 있습니다.
한번 사용해보시는 것도 좋을 것 같아요!
| bill = Number(e.target.value); | |
| bill = +e.target.value; |
There was a problem hiding this comment.
사실 당시에 parseInt()가 생각나지 않아서 썼던 코드인데 이 방법이 더 간편하네요!
| type="text" | ||
| class="custom-tip-input" | ||
| placeholder="Custom" | ||
| oninput="this.value = this.value.replace(/[^0-9.]/g, '').replace(/(\..*)\./g, '$1');" |
There was a problem hiding this comment.
oninput에서 사용되는 이벤트 핸들링 로직이 중복되므로 재사용성을 위해 함수로 분리 해도 좋을 것 같다고 생각합니다!
| tipRatioInputs.forEach((tipRatioInput) => { | ||
| tipRatioInput.classList.remove("selected"); | ||
| }); |
There was a problem hiding this comment.
자주 쓰이는 로직이므로 분리해서 사용하는 건 어떨까요?
| tipRatioInputs.forEach((tipRatioInput) => { | ||
| tipRatioInput.classList.remove("selected"); | ||
| }); | ||
| customTipRatioInput.value = null; |
There was a problem hiding this comment.
value에 null보다는 빈 문자열은 어떨까요 ?
자바스크립트에서는 이미 값이 없음을 나타내는 undefined 라는 타입이 있으니, 같은 개념을 가진 null을 사용할 필요가 없다고 합니다. (또한 input tag의 value 초기값은 빈 문자열이라 생각합니다.)
Why I banned ‘null’ from my Javascript code and why you should too
Don’t repeat yourself. Javascript already has the
undefinedtype to indicate the absence of value, so there is absolutely no need to have a second type that represent the same concept.
There was a problem hiding this comment.
스터디 때 다뤘던 내용!!!인데 순간 거꾸로 착각하고 코드를 작성했네요. 다음부터는 조금 차분하게 스프린트를 진행해야겠습니다.... 그나저나 input tag의 value 초기값이 빈 문자열인 건 이번에 처음 알았어요!
baegyeong
left a comment
baegyeong
left a comment
There was a problem hiding this comment.
이번 스프린트 수고 많으셨습니다! visible이나 forEach 등 제 코드에선 사용하지 않았던 것들을 배울 수 있어서 좋았어요~
| <div class="input-section"> | ||
| <div class="input-title-box"> | ||
| <div class="input-title">Bill</div> | ||
| <div class="input-alert bill-alert">Can't be zero</div> |
There was a problem hiding this comment.
저는 사람수가 0이 되는 경우에 대해서만 alert 처리를 했는데, input에 대해서도 0일 때를 처리하니 더 디테일이 사는 것 같아요!
| billAlert.style.visibility = "hidden"; | ||
| e.target.style.border = "none"; | ||
|
|
||
| bill = Number(e.target.value); |
4 participants
리액트만 다루다가 오랜만에 자바스크립트를 다루니 잊은 부분이 많아 어색했습니다. 제공된 반응형 디자인을 최대한 따르려고 노력했습니다. 원래 html/css 작업을 마치고 javascript를 작성하기 전에 다시 요구사항을 정의할 예정이었는데 깜빡한 점이 아쉽네요... 그리고 저의 경우 tip 계산과 total 계산을
calculate()라는 하나의 함수에서 진행했는데, 다른 분들은 분리하셨는지 궁금합니다. 저는 코드를 작성하며 고민하다가 계산하는 코드의 길이가 짧아 분리하면 오히려 가독성이 떨어진다고 생각해 하나의 함수에서 둘 다 계산했거든요!