[Mission002] TODOS 시스템에 비동기/온오프라인을 고려하여 업데이트하기#37
Conversation
Copy mission01 files to here.
Modify data to todos.
HoseokNa
changed the title
Adjust eslint and delete unused code.
mission002/hsna7024/js/App.js
Outdated
| onKeyEnter: async todo => { | ||
| // TODO : 변경 된 todo로 갱신하기 | ||
| // api.updateTodo(USERNAME, todo); | ||
| // refreshTodos(); | ||
| }, |
There was a problem hiding this comment.
onKeyEnter에서 실행되는게 없는 것 같은데 남겨두신 이유가 있는지 궁금합니다.
There was a problem hiding this comment.
해당 메소드가 등록된 todo를 더블클릭해서 수정하고 엔터 누르면 호출되는 메소드입니다. 그런데 수정한 todo를 API를 통해 갱신해야 하는데 해결을 못해서 주석으로 남겼습니다 ㅠㅠ
There was a problem hiding this comment.
todo가 수정되어도 값은 남아있어서 잘 동작한다고 생각했었는데 API에는 반영이 안되는군요. API에도 수정, 추가가 한번에 되는 Method는 없는 것 같네요. 호석님은 생각지 못한 기능을 잘 찾아내시는 것 같아요. 😄
| import { api } from "./utils/api.js"; | ||
| import { loadTodos } from "./utils/localStorage.js"; | ||
|
|
||
| const init = async () => { |
There was a problem hiding this comment.
다른 분들도 init 함수를 많이 사용하시던데 저는 아직 생소해서요. init을 따로 설정할 때의 장점이 궁금합니다.
There was a problem hiding this comment.
보통 init이 초기화를 의미하는 용도로 사용할 것 같습니다.(확실하지 않네요) init이나 initialize 같은 함수명으로 작업을 진행하면 조금 더 명시적으로 가장 먼저 실행되야 될 것 들을 모아 둘 수 있습니다. 그런데 저도 여기서만 init이란 함수명으로 관리하고 다른 component에서는 이벤트리스너나 this.render 식으로 그냥 실행시키고 있네요. ㅎㅎ
eastjun-dev
left a comment
eastjun-dev
left a comment
There was a problem hiding this comment.
mission001도 그렇고, 호석님 코드가 기능과 역할에 맞게끔 잘 분리되어 있어서 코멘트를 남길게 많지 않네요 ㅎㅎ
1가지만 강조해서 피드백 드리고 싶은거는 조건문의 중첩을 최대한 지양하기 입니다.
if문안에 다른 조건문들이 들어가기 시작하면, 코드가 복잡해지고 한 함수가 한 가지가 넘는 역할을 하는 경우들이 쉽게 발생할 수 있어요. 그래서 조건문의 중첩되는 경우가 발생된다면 혹시 분리할 수는 없을지 살펴봐주시면 더 좋을 것 같습니다. 너무 고생하셨어요 😄 ❤️
| } | ||
| }); | ||
|
|
||
| $target.addEventListener("keydown", e => { |
There was a problem hiding this comment.
현재는 keydown 이벤트가 발생하고 나서 무슨 동작이 이루어지는지 모든 코드를 읽어봐야 알 수 있는 상태인데요. 함수를 따로 분리해서 함수 네이밍으로 kewdown 이벤트 발생시 무슨 동작을 하는지 명시적으로 보여주는건 어떨까요~?
There was a problem hiding this comment.
전체적으로 함수로 만들지 않고 if문 아래 내용을 onKeydownInEditMode로 만들어서 분리했습니다. 감사합니다!
mission002/hsna7024/js/TodoList.js
Outdated
| }); | ||
|
|
||
| $target.addEventListener("keydown", e => { | ||
| if (e.target.classList.contains(classNameMap.EDIT)) { |
There was a problem hiding this comment.
if문에 if문이 중첩되는 형태는 코드를 이해하는데 더 어려움을 겪게 하고, 나중에 다른 로직이 추가 될 때 코드의 복잡성이 쉽게 높아질 수 있어 저도 매번 주의하려고 하는 부분인데요.
위 코드의 경우에는
아래와 같이 예외인 경우에 함수가 종료되게끔 해주시면 코드의 depth가 줄어들며서 가독성이 더 좋아질 것 같아요 :)
if (!e.target.classList.contains(classNameMap.EDIT)) {
return
}
There was a problem hiding this comment.
넵 말씀하신 내용 반영했습니다. 예외적인 부분을 앞으로 오도록 코딩해야겠네요. 감사합니다!
mission002/hsna7024/js/TodoList.js
Outdated
| if (e.target.classList.contains(classNameMap.EDIT)) { | ||
| const { id } = e.target.closest("li").dataset; | ||
| if (e.key === keyMap.ENTER && e.target.value) { | ||
| const index = todos.findIndex(todo => { |
There was a problem hiding this comment.
위 코드는 아래와 같이 한줄로 깔끔하게 줄일 수 있답니다 😄
const index = todos.findIndex(todo => todo._id === id);
There was a problem hiding this comment.
한줄로 깔끔하게 줄일 수 있군요. 피드백 반영했습니다. 감사합니다!
mission002/hsna7024/js/main.js
Outdated
| const init = async () => { | ||
| const todos = loadTodos() || (await api.getTodos(USERNAME)); | ||
|
|
||
| const app = new App({ |
There was a problem hiding this comment.
app변수를 사용하는 곳이 없는데 선언하신 이유가 있으실까요?
There was a problem hiding this comment.
없습니다 ㅠㅠ app과 todoInput 변수 삭제했습니다. 감사합니다.
Add onEdit, offEdit and refactoring code.
3 participants
미션내용
#7
진행상황
소감
2번째 미션 올립니다.
첫번째 미션 코드에 지난 스터디 코드 참고해서 진행했습니다.
해결 못한 부분은 더블클릭하여 Todo 내용을 수정했을 때 수정된 내용을 API에 반영하지 못했습니다.
해당 부분은 구현 못하고 주석처리하여 남겨 놓았습니다.
그리고 첫번째 미션 때 todo 데이터에 onEdit이란 항목을 추가해서 작업을 했었는데 데이터 형식을 맞추기 위해 해당 항목을 삭제하고 동준님 코드 참고하여 수정했습니다. (classList.toggle('editing'))
현재는 복습도 필요할거 같아서 5기 세션에 참가도 할까 고민중입니다. ㅎㅎ
화이팅!