-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[1단계 - 음식점 목록] 루루(송지은) 미션 제출합니다. #18
Conversation
Co-authored-by: Rulu <hafnium1923@users.noreply.github.com>
Co-authored-by: Rulu <hafnium1923@users.noreply.github.com>
Co-authored-by: Rulu <hafnium1923@users.noreply.github.com>
Co-authored-by: Rulu <hafnium1923@users.noreply.github.com>
Co-authored-by: Rulu <hafnium1923@users.noreply.github.com>
Co-authored-by: Rulu <hafnium1923@gamil.com>
Co-authored-by: Rulu <hafnium1923@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 루루! 그루밍입니다 ~ 🥐
생각 정리 부분에서도 질문이 슥 보이길래 같이 적어봤어요.
혹시 이해가 안가시는 부분이 있다면 언제든지 코멘트로 남겨주세요~! 허헣
생각정리
1.
컴포넌트 분리는 때에 따라 다르게 분리할 수 있습니다. 꼭 따로 사용이 될 수 있는 모든 단위들을 분리하려고 하지 않아도 될 것 같아요. 다양한 방법으로 컴포넌트를 나누어보고, 왜 컴포넌트를 분리해야하는지, 해당 방법에 따라 분리하는 것은 어떤 장점과 단점이 있는지를 계속해서 익혀나가면 좋을 것 같습니다. 코드를 작성함에 있어 해야하는 것
은 없으니 루루만의 기준을 세워나가시길 바랍니다. 🪁
2.
루루는 flux 패턴에 대해 잘 이해하고 계신가요? 패턴을 잘 이해하지 못한 상태에서 코드를 작성했다고 느끼시고 계신 것 같아요. 해당 패턴을 도입하게 된 이유는 무엇 때문이었나요? flux 패턴을 사용해 배우고 싶었던 점이 어떤 부분이었는지, 혹은 단순히 누군가 추천을 해서 적용을 했는지, 이 미션을 flux 패턴으로 구현해 어떤 부분을 배웠는지에 대해 정리해보시면 좋을 것 같습니다 ㅎㅎ 🥹
3.
어떤 점에서 class와 interface를 비교하게 된 것인지 잘 모르겠어요. 두가지는 완전히 다른데 혹시 헷갈리시는 부분이 어느쪽일까요?
4.
어떤 부분에서 효율적으로 만들고 싶으신지, 왜 카테고리와 정렬기준을 모두 store로 관리를 하면 효율적이라고 생각하시는지에 대해 적어주시면 좋을 것 같아요.
의문에 대한 QnA
질문에 순서가 조금 헷갈려서 그냥 1번부터 적을게욧!
1.
저도 처음에 Javascript를 사용하다가 Typescript를 처음 배우고 사용할 때에는 같은 생각이었는데요, 점차 큰 프로젝트를 경험하게 되고, 복잡한 코드 구조를 만나게 되면서 생각이 바뀌게 되었어요. type을 미리 지정해 둠으로써 실수할 확률히 확연히 줄어들고, 어떤 타입이 들어와야하는지 명확해지기 때문에 오히려 나중에는 더 편해진답니다. 지금 그렇게 생각하는건 당연한거랍니다! 조금 더 익숙해지게되면 의문이 풀리게 될 거에요~ 😉
2.
↑ 위에서도 잠깐 말씀드렸는데, 두가지는 완전히 다른 개념이에요.
변수들을 정의하고있다고 해서, interface에 변수들을 담을 수는 없어요. 정확히 말하면, interface는 변수들의 타입을 담고 있는 것이에욥. 타입 문법은 컴파일 과정에서 전부 정리됩니다. 타입스크립트 자체에 대한 공부를 좀 더 해보시는건 어떨까요? 공식 handbook을 읽으셔도 좋고, 책을 사서 읽어보는 것도 좋을 것 같습니다.
3.
컴포넌트를 분리하는 기준은 개개인마다, 또 프로젝트마다 상이합니다. 정답을 가진 부분이 아니기 때문에 직접 구현해보고 경험해보시는 게 좋을 것 같아요. 작은 프로젝트에서는 굳이 컴포넌트를 나눌 필요가 없기도 합니다.
아래 질문을 계속 떠올리면서 계속 구현해보시는 연습을 해보시는게 좋을 것 같아요. 연습을 통해 감을 점차 잡아나가시면 됩니다 😉
- 컴포넌트는 왜 분리해야할까요?
- 컴포넌트가 비대해지면 어떤 단점들이 생길까요?
- 컴포넌트를 분리하면 그 단점들을 극복할 수 있을까요?
- 컴포넌트를 무작정 작게 분리하게되면 어떤 문제점들이 있을까요?
4.
↑ 위에서 답변을 드린 것 같아 패쓰합니닷! 😎
5.
저는 이벤트 핸들러는 사용자의 액션에 따라 실행되는 코드이기 때문에 UI 로직 중 하나라고 생각해요.
버튼을 클릭한다거나, textfield에 입력을 한다거나, 스크롤 하는 등의 모든 화면과 상호작용하는 행위들은 UI 로직이라고 생각합니다.
6.
질문이 조금 모호한 것 같아요. 무엇을 어디까지 나눈다는 말씀이신걸까용~? 🤔
7.
문자열 값만 가능합니다. 만약 문자열 값이 아니라면 문자열로 변환되기 때문에 [object Object]를 보게 되신 것 같네요!
8.
왜 UI에서 처리해야한다고 생각하시나용?! 그 이유와 함께 써주시면 더 좋을 것 같아요.
localstorage 자체는 도메인 레이어와 분리되어도 괜찮다고 생각합니다.
엉엉 우울해지지 마세요 루루!!!!!!!! 😞😞
어떤 느낌인지 잘 알 것 같아요. 배워야 할 내용은 많고, 구현해야 하는 미션은 어렵고, 시간 내에 제출해야하고, 무언가 만들고 있기는 하지만 내가 제대로 이해했는지, 이렇게 만드는게 맞는지 많은 고민과 생각이 들 것 같아요.
하지만 루루에게는 주변에 같은 고민을 하는 크루들이 있고, 고민이 생기면 언제든지 도와줄 수 있는 코치들이 있어요! (또 원격으로 응원하는 리뷰어도 있고^^,,~) (좀 양심없는 것 같지만 리뷰어도 한번 발 내밀어봅니다..🦶)
미션이 버겁지만 이렇게 열심히 해나가려고 노력하고 또 성장해내는 루루의 모습이 정말 멋지다고 생각해요!!!!!!
누구나 첫 발을 떼기는 정말 어렵답니다. 언젠가 나도 고럴때가 있었지 ~ 하고 회상하는 날이 오게 될 거에요.
새로운 미션에 있는 모든 요구사항을 한번에 충족하려 하지 말고, 본인만의 시간에 맞추어 하나 하나 배워나가는게 중요한 것 같아요. 한꺼번에 많은 것을 배우는 것 보다는 한 스텝 한 스텝을 잘 배우는 것이 좋으니, 걱정하지 마시고 지금처럼만 열심히 하시면 될 것 같아요. 화이팅입니다! ꒰◍ˊ◡ˋ꒱੭⁾⁾ 🔥🎁🔥🎁🔥🎁🔥
src/components/HeaderComponent.js
Outdated
show() { | ||
document.querySelector(".modal").classList.add("modal--open"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 이름만 보면 header components를 show하는 것 같아요. 정확히 어떤걸 show해주는건지 명시해주는 건 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 수정하겠습니다!
setEvent() { | ||
document | ||
.querySelector("form") | ||
.addEventListener("submit", (e) => this.addRestaurant(e)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요부분은 addEventListener('submit', this.addRestaurant)
로 줄여볼 수 있겠네요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 다른 컴포넌트들도 addEventListener(..) 부분에 사용된 쓸모없는 화살표 함수는 모두 제거하겠습니다.
addRestaurant(e) { | ||
e.preventDefault(); | ||
const category = document.querySelector("#category").value; | ||
const name = document.querySelector("#name").value; | ||
const distance = document.querySelector("#distance").value; | ||
const description = document.querySelector("#description").value; | ||
const link = document.querySelector("#link").value; | ||
|
||
const restaurant = { | ||
category, | ||
name, | ||
distance, | ||
description, | ||
link, | ||
}; | ||
|
||
dispatcher(RESTAURANT_ACTION.ADD_RESTAURANT, restaurant); | ||
|
||
document.querySelector(".modal").classList.remove("modal--open"); | ||
document.querySelector("form").reset(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eventlistener의 네이밍에 대해서도 한번 더 고민해주시면 좋을 것 같아요.
이름은 addRestaurant인데 modal을 닫아주고 form을 reset해주는 등의 많은 역할을 함께 수행해주고 있어요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addRestaurant의 모달 닫는 기능과 폼 리셋 기능을 따로 분리하여 addEventListener에서 화살표 함수로 묶어서 작동하도록 하였습니다.
추가로 취소하기 버튼을 ModalComponent에서 이벤트 감지를 하였는데, 버튼이 없는 곳에서 이벤트 감지하는것이 간접적으로 다른 컴포넌트에 관여하고 있다고 생각이 들었습니다.
또한 RestaurantAddFormComponent에서 submit을 할때 모달을 숨기는 기능을 따로 함수로 분리했기 때문에 취소버튼 클릭시 함수를 재사용하는 방법도 있기에 RestaurantAddFormComponent에서 취소하기 버튼을 감지하도록 바꿨습니다.
src/domain/RestaurantsStore.ts
Outdated
this.#restaurantList = JSON.parse( | ||
localStorage.getItem(RESTAURANTS_STORAGE)! | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typescript에서 not null 연산자는 권장되지 않아요.
!
를 활용해서 당장의 타입 검사만 모면하기보다는 조건문을 통한 처리를 해주는 등의 방향이 더 적절해보입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 확인했습니다!! not null 연산자를 제거했습니다
src/domain/RestaurantsStore.ts
Outdated
#restaurantList: Restaurant[] = []; | ||
#category: Category = CATEGORY_DEFAULT; | ||
#sortMethod: SortMethod = SORT_METHOD.NAME; | ||
|
||
#subscribers: CustomElement[] = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
도메인에 종속된 상태들(restaurantList, category, sortMethod)과 store에 관련된 로직 관련 상태들이 함께 있으니, 조금 헷갈리네요. 범용적이고 변화될 가능성이 없는 메서드는 분리하는게 더 보기 편하지 않을까 - 하는 생각이 듭니다.
예를 들어 Store 객체를 하나 만들어 subscriber, reducer, subscribe 등과 같은 것들을 넣어두고 extends해 사용하면 도메인 종속적인 로직들과 그렇지 않은 로직들이 더 잘 구분가지 않을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
음식점과 관련된 상태는 RestaurantsStore에, dispatcher과 연결되는 부분들은 Store로 분리했습니다.
src/domain/Dispatcher.ts
Outdated
const dispatcher = ( | ||
type: string, | ||
data?: Restaurant | Category | SortMethod | ||
) => { | ||
const action: Action = { type, data }; | ||
RestaurantInstance.reducer[type](action); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
욕심을 조금 더 부려서 dispatcher는 쬐끔 더 범용적으로 만들 수 있지 않을까 라는 생각이 들어요!
지금은 RestaurantInstance를 알고 있기 때문에 Restaurant에 한정된 dispatcher가 되어버렸는데, Store 객체 또한 외부에서 주입받도록 변경해보면 domain에 종속되지 않은 dispatcher를 만들어볼 수 있을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
객체를 외부에서 주입받는 것을 생각해보았는데 dispatcher가 필요없이 컴포넌트에서 바로 Store을 연결하는 방법밖에 생각이 나지않더라구요. 그래서 다른 store들이 있을 때에도 활용할 수 있도록 reducer을 쪼개서(?) 연결하도록 해보았습니다!
|
||
sortRestaurants(sortMethod: SortMethod) { | ||
this.#sortMethod = sortMethod; | ||
switch (this.#sortMethod) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
switch를 활용한 부분 좋네용! 👍
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
조금 더 안전한 구현을 위해 default를 적어주시는 것은 어떨까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
처음 화면을 로딩하였을 때 이름순이 기본으로 되어있기 때문에 default는 이름순으로 정렬하게 했습니다.
안녕하세요 그루밍! 🫧 작성해주신 QnA도 아래에 써주신 메시지도 모두 잘 읽어보았습니다. 다른 크루분들이랑 비교하지 않으려해도 저도 모르게 비교가 돼서 많이 속상했는데 그루밍한테 위로를 받은듯해 많이 도움이 되었습니다. 감사합니다.🥰 생각정리
추가로 그전 코드에서 필터를 적용한 후 음식점을 추가하면 필터가 적용된 음식점과 직전에 추가된 음식점 외 정보들이 사라지는 오류를 발견해서 같이 고쳐보았습니다. |
</main> | ||
</body> | ||
|
||
</html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
요기 end of file 에러가 아직 보이네욧! 호홋
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eof 고쳐보려했는데 저장하면 맨 마지막줄이 자동으로 없어집니다ㅠㅠ eslint 설정을 바꾸면 되는 거겠죠?!?!
this.#restaurantList = JSON.parse( | ||
localStorage.getItem(RESTAURANTS_STORAGE) as string | ||
); | ||
this.#restaurantList.push(restaurant); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
만약 getItem의 return 값이 undefined일 경우에는 오류를 발생시킬 수 있으니,
요렇게 as~ 로 type assertion을 하는 것 보다는, 예외처리를 해주는게 더 좋을 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
오! 그렇네요 지금은 로컬에 아무 데이터가 없는 상태로 연결이 되면 constructor에서 빈 배열을 넣어주니 문제가 없지만 2단계에서 삭제를 만들게 되면 오류를 던지겠네요! 감사합니다ㅎㅎ 반영해서 고쳐보겠습니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안뇽하세요 루루!
리뷰 반영해주시느라 고생 많으셨어요~! 🐾🐾
코멘트 달아주신 것 보니, 질문에 해주신 내용에 대해서 깊게 고민해 보신 것 같아서 정말 좋았어요! 👍
잘 해내고 계시군요! 앞으로도 계속 조금씩 이렇게 고민하면서 이유있는 코드를 만들어나가면 좋을 것 같아요~ 77ㅑ~
오오우 영화도 보고 오셨꾼요!!!!!!! 진짜 진짜 잘하셨어요 😆
잘 쉬어야 또 다음 스텝으로 넘어갈 힘이 생기니까요!!!!!
조금은 어렵겠지만 나만의 흐름을 찾고 그 흐름에 맞추어 즐겁게 공부해봐요~! 할수있드아악!!
저도 오랜만에 영화관가서 추천해주신 영화 한번 보고와야겠어요 추천해주셔서 고마와요 🐤
몇가지 코멘트를 남겨두었는데, 2단계에서 반영해주시면 될 것 같아요 🔥
2단계에서는 타입스크립트를 조금 더 효율적으로 사용하는 방법을 익혀봐도 좋겠네요 ㅎㅎ
고생 많으셨어요!
다음단계에서 봐용! 뽜이팅!!! 🐝🐝
안녕하세요 그루밍 🫧!
저는 약 2주간 점심뭐먹지 미션의 리뷰를 받게 된 루루입니다.
만나게 되어 완전 반갑습니다ㅎㅎ
앞으로 잘 부탁드리겠습니다.😆
##미션페이지
https://hafnium1923.github.io/javascript-lunch/
개발하며 신경 쓴 부분
구조
사용한 패턴: flux패턴
![image](https://user-images.githubusercontent.com/79538610/222669411-b06af616-ea8a-43d2-a49f-7a3f71c54153.png)
액션 -> 디스패처 -> 스토어 -> 뷰
뷰에서 액션이 들어올 경우 다시 디스패처로 액션이 전달된다.
미션을 진행하며 든 나의 생각 정리.
미션을 진행하며 든 의문
1-1.
2-1.
5.1
느낀점
사실 이번 미션은 제대로 모르는 것을 해결하지 못한 상태로 새로운 기술과 새로운 이론을 이해하는 것으로 벅찬 미션이었습니다. 그래서 제가 질문하고 있지만 이 질문이 제대로 된 질문인지도 모르겠습니다. 제 코드지만 다시 한번 만들어보라고 하면 못할 것 같기도 합니다. 그저 지금은 이해하고 기술을 배우는 것이 제가 할 수 있는 최선이라고 생각하면 조금 우울해지기도 합니다. 어쩔 수 없는 지식의 부족때문이고 그것을 채워가는 것이 앞으로 제 숙제라고 생각하지만, 약간은 막연하게 느껴지는 미션이었던 것 같습니다. 페어가 끈기있게 제가 이해하기를 기다려주고 모르는 것을 가르쳐 주어서 미션을 잘 완성할 수 있었던 것 같습니다..ㅎㅎ.. 어려웠지만 새로운 것들을 적용해보는 것은 재미있었던 것 같습니다.