[10기 곽창섭] TodoList with CRUD #218
Conversation
ChangSubKwak
commented
Jul 12, 2021
ChangSubKwak
commented
- 기능 구현에만 집중
- 리팩토링, 코드 분리, 클린코드 지향 작업 못하였음
| itemHTML += "</div>"; | ||
| itemHTML += "<input class='edit' value='" + todoObj.value + "' />"; | ||
| itemHTML += "</li>"; | ||
|
|
There was a problem hiding this comment.
백틱(`)으로 템플릿 리터럴을 사용을 추천드립니다😸
이를 사용하면, 기존의 따옴표 방식에서는, + 연산자를 사용하지 않고
않에 표현식을 작성할 수 있습니다 ㅎㅎ
| for (let i = 0 ; i < todoData.length ; i++) { | ||
| addTodoItem(todoData[i], true); | ||
| } |
There was a problem hiding this comment.
배열순회를 목적으로 하신다면 for문보다는 map이나 foreach의 사용을 추천드립니다ㅎㅎ
| let todoView = document.getElementById("todo-list"); | ||
| let todoData = JSON.parse(localStorage.getItem('todoData')) ?? []; |
There was a problem hiding this comment.
변하는 값이 아니라면 let 보다는 const를 사용하는게 좋습니다! let은 나중에 변수 할당해야할 필요가 있거나 등의 꼭 필요한 이유가 있는 경우 아니고서야 기본적으로 const를 사용한다고 보시면됩니다. 이유는 계속해서 한개의 변수에 다른 값을 할당하면 디버깅이나 유지보수 측면에서 추적이 힘들 수 있다고 알고있습니다!
| for (let i = 0 ; i < todoData.length ; i++) { | ||
| addTodoItem(todoData[i], true); | ||
| } |
There was a problem hiding this comment.
for 보다는 array method나 for in for of를 사용하는 것도 좋아보입니다!
| type : todoObj.type, | ||
| value : todoObj.value, | ||
| }); | ||
| localStorage.setItem('todoData', JSON.stringify(todoData)); |
There was a problem hiding this comment.
setitem getitem같은걸 유틸로 분리해서 사용하면 좋을 것 같아요!
| if (e.key === "Enter" && newTodoTitle.value !== '') { | ||
| let todoObj = { | ||
| isComplete : "false", | ||
| type : "view", | ||
| value : newTodoTitle.value, | ||
| } | ||
| addTodoItem(todoObj, false); | ||
| newTodoTitle.value = ''; | ||
| } |
There was a problem hiding this comment.
&&로 체크하기 보다는 거짓 조건을 먼저 달아서 함수를 끝내는 것도 방법입니다
if (newTodoTitle.value === '') return
if (e.key === "Enter") {
#code
}There was a problem hiding this comment.
네, 동의합니다. 장후님 말씀하신 대로의 코드를 지향하는데, 놓친 부분이에요.
| document.addEventListener("dblclick", (e) => { | ||
| let li = e.target.parentElement.parentElement; | ||
| li.classList.add("editing"); | ||
| li.addEventListener('keyup', (e2) => { |
| e.target.setAttribute("false", ""); | ||
| e.target.removeAttribute("checked"); |
There was a problem hiding this comment.
attribute를 사용할 때
e.target.checked = true
e.target.checked = false이런식으로 사용해도 좋을 것 같아요!
| let itemHTML = ""; | ||
| itemHTML += "<li id='" + id + "' class='" + isComplete + "'>"; | ||
| itemHTML += "<div class='" + todoObj.type + "'>"; | ||
| itemHTML += "<input class='toggle' type='checkbox' id='" + id + "' " + isChecked + " />"; | ||
| itemHTML += "<label class='label'>" + todoObj.value + "</label>"; | ||
| itemHTML += "<button class='destroy' id='" + id + "'></button>"; | ||
| itemHTML += "</div>"; | ||
| itemHTML += "<input class='edit' value='" + todoObj.value + "' />"; | ||
| itemHTML += "</li>"; |
There was a problem hiding this comment.
`<li >
<div class="view">
<input class="toggle" type="checkbox" data-node-id=${todo.idx} />
//중략
</li>`이런식으로 백틱()` 을 이용해서 한번에 기입하는 것도 좋을 것 같아요 ㅎㅎ
ChangSubKwak
changed the title
