[3기 이한솔]TodoList with Ajax #17
Conversation
index.html
Outdated
| <script> | ||
|
|
||
| </script> |
index.html
Outdated
| @@ -122,5 +110,6 @@ <h1 id="user-title" data-username="eastjun"> | |||
| </section> | |||
| </div> | |||
| <script src="js/index.js"></script> | |||
js/App.js
Outdated
| import { TodoApp } from "./components/TodoApp.js"; | ||
| import { TodoRepository } from "./repository/TodoRepository.js"; | ||
|
|
||
| window.addEventListener('DOMContentLoaded', async (event) => { | ||
| /*const todoApp = new TodoApp(); | ||
| todoApp.addObservers( | ||
| new TodoInput(todoApp), | ||
| new TodoList(todoApp), | ||
| new TodoCount(todoApp) | ||
| ); | ||
| todoApp.render();*/ | ||
| let todoRepository = new TodoRepository(); | ||
| let findUsers = await todoRepository.findUsers(); | ||
| debugger | ||
|
|
||
|
|
||
| }); |
There was a problem hiding this comment.
App.js가 작성은 되었는데 html에서도 그렇고 js에서도 그렇고 가져오는 코드가 없네요!
There was a problem hiding this comment.
전체적으로 코드를 하도 수정&재정의하면서 익숙치않은 방식의 설계로 하다보니 사용하지않는 잔여 코드들이 너무 많이 남아있네요. 오늘 정리할 예정입니다 ㅠㅠ
js/components/TodoApp.js
Outdated
| let userTitle = new UserTitle(_.userTitleTarget, this); | ||
| let userList = new UserList(_.userListTarget, this); | ||
| let todoInput = new TodoInput(_.todoInputTarget, this); | ||
| let todoList = new TodoList(_.todoListTarget, this); | ||
| let todoFooter = new TodoFooter(_.todoFooterTarget, this); |
There was a problem hiding this comment.
여기서 변수는 const로 하는게 맞는 것 같아요!
js/components/TodoApp.js
Outdated
| this.addObservers( | ||
| userTitle, | ||
| userList, | ||
| todoInput, | ||
| todoList, | ||
| todoFooter | ||
| ); | ||
| this.notify(); |
There was a problem hiding this comment.
컴포넌트가 재사용되고 있진 않기 때문에 그냥 addObservers인자에서 바로 생성해주는 것도 좋았을 것 같습니다!
js/repository/TodoRepository.js
Outdated
|
|
||
| saveUser(name) { | ||
| try { | ||
| return this.fetch("POST", "/users", { name }, null, null); |
There was a problem hiding this comment.
"POST" 라는 문자열보단
HttpMethod.POST
처럼 constant의 변수로 보내는건 어떨까요?
js/service/UserService.js
Outdated
| let todoItem = await this.#todoRepository.saveTodoItem(this.#selectedUser._id, contents); | ||
| this.#selectedUser.todoList.push(todoItem); | ||
| this.#subject.notify(); | ||
| } | ||
| deleteItem = async (itemId) => { | ||
| let todoList = this.#selectedUser.todoList; | ||
| let findIndex = todoList.findIndex(item => item._id === itemId); |
There was a problem hiding this comment.
변수는 일단 const로 정의해준다고 생각해주세요! let의 말 그대로 "변수"를 선언할 때 사용합니다. 하지만 생각보다 선언한 변수의 값이 변경되는 경우는 많지 않아요!
| */ | ||
|
|
||
|
|
||
| export const UserService = class { |
There was a problem hiding this comment.
Service, Repository 등의 코드를 singleton으로 디자인해보는건 어떨까요?
There was a problem hiding this comment.
소스코드의 최상위 컴포넌트(Subject) 혹은 App부분에서 선언된 클래스를 선언해서 주입하는방식과 싱글톤방식중 어느게 더 나을까요?
스프링을 하면서 하던 개념으로만 접근을해서 싱글톤보단 초창기 생성후 공유하는방식을 사용했는데,
싱글톤도 고려사항중 하나였거든요.
그러다가 그냥 익숙하게 하던대로 해보자라고해서 Subject부분에서 생성후 참조를 넘겨주는 방식으로했습니다.
어느 방법을 선택해야하는게 더 좋은가에 대한 고민을 항상하는데 부족함이 너무 많네요 ㅠ
There was a problem hiding this comment.
음 제 생각엔 어느 한가지가 정답이라기 보단 상황에 맞게 적절하게 사용하는게 중요한 것 같아요!
사실 스프링 방식처럼 하기 위해선 아예 스프링 컨테이너처럼 빈으로 등록하면 싱글톤으로 사용할 수 있도록 만드는게 베스트인데.. 그렇게까지 하기에는 또 쉽지 않아서요 ㅠㅠ 저의 경우 각 객체마다 고유의 멤버 변수가 필요하다면 그냥 객체로 사용하고
싱글톤 형태로 사용해야 한다면 그냥 Object로 만들어서 사용하고 있어요!
js/service/UserService.js
Outdated
| #decodePriority(priority) { | ||
| return priority === "0" | ||
| ? "NONE" : priority === "1" | ||
| ? "FIRST" : "SECOND" | ||
| } |
There was a problem hiding this comment.
priority도 상수로 관리하면 좋을 것같아요!
| this.init({ | ||
| userTitleTarget, | ||
| userListTarget, | ||
| todoInputTarget, | ||
| todoListTarget, | ||
| todoFooterTarget, |
There was a problem hiding this comment.
잘 생각해보면 todo의 변화가 user에게 전파되고 있진 않습니다.
그래서 서로 다른 observer를 가지는게 좋지않을까요?
지금은 todo에서 변화가 발생하면 user도 변하는 형태인데
현재 요구사항대로라면 user가 변화면 todo가 변할순 있어도 todo가 변했을 때 user가 변화하면 안 되는 상태입니다!
There was a problem hiding this comment.
User와 TodoItem을 분리해서 따로 관리하자는 말씀이신가요? 그 부분은 고민하다가 시간이 부족해 패스했는데, 조언해주신거보면 맞는 말씀이신거같네요.
There was a problem hiding this comment.
넵 그래서 이렇게 최상위에서 옵저버를 추가하는 방법이 있을 수도 있고,
혹은 각각의 컴포넌트에서 변화를 알리고 싶은 서브젝트를 추가하는 방법도 있습니다!
혹은 Object.defineProeprty를 이용해서 아예 자동화를 시킬수도 있어요!
예를들어
어떤 상태 A가 있고, 어떤 컴포넌트 B가 있습니다.
- A에 Object.defineProperty를 이용하여 A의 값이 호출(get)되었을 때 해당 값을 호출한 컴포넌트를 옵저버로 등록
- 그래서 A의 값이 변경될 때(set) A의 변화를 옵저버들에게 알려줌. 옵저버들은 변화된 값에 따라서 render를 실행
- 이 때 렌더링 코드가 최초로 한 번 호출되는 시점에만 옵저버로 등록
대신 이런식으로 디자인할 경우 조금 헷갈릴 수 있는게 컴포넌트가 서브젝트가 되기도 하고 옵저버가 되기도 합니다.
상위 컴포넌트의 변화를 자식에게 알려야할 수 있거든요!
2. API호출 클래스 이름 변경(HttpClient) 3. TodoHttpClient 어댑터 추가 4. templates제거 및 컴포넌트에서 담당하도록 수정 5. HttpMethod및 몇몇 변수값들 상수화 6. todoList 하단부분 기능구현
| userListTarget: document.querySelector("#user-list"), | ||
| todoInputTarget: document.querySelector(".input-container"), | ||
| todoListTarget: document.querySelector(".todo-list"), | ||
| todoFooterTarget: document.querySelector(".count-container") |
There was a problem hiding this comment.
성능적으로 차이가 있다고 말씀해주셔서 찾아봤는데,
Function | Live? | Type | Time Complexity
querySelector | N | Element | O(n)
querySelectorAll | N | NodeList | O(n)
getElementById | Y | Element | O(1)
getElementsByClassName | Y | HTMLCollection | O(1)
getElementsByTagName | Y | HTMLCollection | O(1)
getElementsByName | Y | NodeList | O(1)
라고 하네요.
There was a problem hiding this comment.
막연이 더 빠르다정도만 알고있었는데, 링크주신글을 보니 저도 좀 더 자세히 알꺼같네요 감사합니다 ㅎ
| static FIRST = "FIRST" | ||
| static SECOND = "SECOND" | ||
| static NONE = "NONE"; | ||
| } |
There was a problem hiding this comment.
class로 객체를 만들지 않으니, 객체를 export하면 어떨까요?
| return fetch(this.#url + path, this.#options).then(data => data.json()); | ||
| } | ||
|
|
||
| _excute(path) { |
There was a problem hiding this comment.
execute 함수가 중복인 것 같아요. private이라 그렇다면 public으로 바꾸는 게 어떨까요?
| res.push(i); | ||
| } | ||
| return res; | ||
| }; |
There was a problem hiding this comment.
이게 말씀해주신 내용이네요. 함수형프로그래밍 학습에 대한 필요성을 느낍니다 ㅜㅜ.
Footer부분을 제외한 기본적인 기능구현하여 PR합니다 .
프론트엔드 상태관리개념과, Observer Pattern의 기능구현을 적용하려니 익숙하지않아, 시간이 너무지체되어
미완성이라도 올렸습니다.
상위컴포넌트이자 ConcreteSubject인 TodoApp에서 각 이벤트에 대응하는 로직을 넣어주려하다가 너무 양이 많아
서비스클래스(UserService)를 만들어서 내려주는 방식으로 했습니다. Dependancy Injection을 적용하고싶어 알아보고 있습니다..
그게아니라면 해당 서비스로직들을 스태틱하게 만들어주는것과 고려중입니다.
모든 동작에 새로 렌더링을 통해 그려주는 방식으로 구현을 하려하니, 몇 애니메이션 효과들을 볼 수 없는데, 이런부분은 어떻게 처리해야할지 고민입니다.
기능 구현을 하는 하나하나가 어렵다기보다는 이 로직이랑 이런 부분은 어디에 위치해야하고, 데이터들은 어디서 관리해야하고
그런 계층을 나누고 데이터들의 위치를 정하는것이 너무 헷갈리고 지금도 계속 혼동되어 수정과 롤백이 반복되고 있네요..