[tip-calculator-app] loopy(임채승) 미션 제출합니다.#1
Open
loopy-lim wants to merge 3 commits intoJS-GreenTea:Lim-ChaeSeungfrom
Open
[tip-calculator-app] loopy(임채승) 미션 제출합니다.#1loopy-lim wants to merge 3 commits intoJS-GreenTea:Lim-ChaeSeungfrom
loopy-lim wants to merge 3 commits intoJS-GreenTea:Lim-ChaeSeungfrom
Conversation
2yunseong
reviewed
Nov 9, 2022
Comment on lines
+13
to
+36
| set billNum(changeNum) { | ||
| this._billNum = changeNum; | ||
| this.tipChangeObserver(); | ||
| }, | ||
| set tip(changeNum) { | ||
| this._tip = changeNum; | ||
| this.tipChangeObserver(); | ||
| }, | ||
| set numberOfPeople(changeNumber) { | ||
| this._numberOfPeople = changeNumber; | ||
| this.tipChangeObserver(); | ||
| }, | ||
| set tipAcount(value) { | ||
| this._tipAcount = value; | ||
| }, | ||
| set tipTotal(value) { | ||
| this._tipTotal = value; | ||
| }, | ||
| get tipAcount() { | ||
| return this._tipAcount; | ||
| }, | ||
| get tipTotal() { | ||
| return this._tipTotal; | ||
| }, |
Comment on lines
+56
to
+62
| const billNumChagneListner = function (event) { | ||
| tips.billNum = +event.target.value; | ||
| }; | ||
|
|
||
| const tipsButtonClickListner = function (event) { | ||
| tips.tip = +event.target.value; | ||
| }; |
Member
There was a problem hiding this comment.
은연듯 든 생각인데, 이것도 뭔가 합칠 수 있을 것 같아요! (just 극히 개인적인 생각)
[입코딩]
- event.target에서 클래스 네임이나 name 속성을 가져온다
- tips라는 객체의 property의 이름을 input 태그의 name속성이나 클래스네임과 동일하게 한다
이렇게 하면 함수를 일반화해서 사용가능하지 않을까?
Suggested change
| const billNumChagneListner = function (event) { | |
| tips.billNum = +event.target.value; | |
| }; | |
| const tipsButtonClickListner = function (event) { | |
| tips.tip = +event.target.value; | |
| }; | |
| const onChangeHandler = (event) => { | |
| tips[event.target.name] = +event.target.value; | |
| }; |
Member
Author
There was a problem hiding this comment.
오 React같은 component로 동작하는 라이브러리에서 짜면 함 생각해볼만한 것 같아요!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
이번 미션은 CSS를 구성할 때 많은 고민을 하면서 짰습니다.
크게 card와 index로 나누어 최대한 css로 처리 할 수 있는 부분은 CSS로 처리하였다고 생각합니다.
다만, JS를 CSS에 비교적 신경을 쓰지 못했다는게 아쉽습니다.
일단 JS에서 가지고 와야 할 요소만 10개가 넘고, 이를 처리할 이벤트도 덩달아 많았습니다.
tips라는 객체에 tip을 처리하는 모든 것을 넣고 싶어 40줄이 넘어갔습니다.
저는 가볍게 보고 시작을 하였지만, 결국 100줄이 넘어가는 코드가 된 것 같습니다.
여기서 여러분은 어떻게 나누는 것이 좋아 보이나요? MVC? MVVM? FLUX?
저는 많은 책임이 부여된 tips를 데이터와 기능으로 분리하고, Event가 많아 Flux를 사용할 것 같습니다.
좋은 분리 방식(꼭 패턴이 아니어도 되요!)이 있는거 같으면 말해주세요!!