본문 바로가기
항해99/온보딩 스터디

[항해99] 온보딩 스터디 - 2주차 5일 TIL

by junvely 2023. 3. 18.

🦄 온보딩 스터디 2주차 5일 TIL

✨03/17 : 목표 : Todo List 코드리뷰 정리

오늘은 어제 실습한 Todo List 리팩토링에 대해 기술 매니저님께서 코드 리뷰 해 주신 부분들을 정리해 보았다. 


📒Todo List 코드리뷰 정리

01 코드리뷰 정리

 

1. 불필요한 updateTodo를 생략하고, todos = [ ]로 바로 생성하기 

=> todos = [ ... ] 자체만으로도 새로운 배열이 생성된 것이기 때문에,  updateTodo를 재사용할 것이 아니라면 따로 만들 필요가 없다. 

기존 코드)

const addTodo = () => {
  const newTodo = {
    id: getRandomKey(), 
    todo: todoInput.value,
    done: false,
  };

  const updateTodo = [...todos, newTodo]; //updateTodo 생략
  todos = updateTodo;
  renderTodos();
};

♻️ 리팩토링)

const addTodo = () => {
  todos = [
    ...todos,
    {
      id: getRandomKey(),
      todo: todoInput.value,
      done: false,
    },
  ];
  renderTodos();
};

 

2. 변수의 네이밍, !연산자로 done 여부에 따른 done상태 변경 코드 줄이기, 불필요한 스프레드 연산자(...)

기존 코드)

const modifyTodo = (event) => {
  const target = event.target.parentElement; //1. target은 예약어, target이 무엇인지 정확하지 않고, 반복되며 가독성이 떨어짐
  const updateTodo = todos.map((todo) => {
    if (todo.id === Number(target.dataset.id)) {
      if (todo.done) { //2. done의 여부에 따라 done상태를 변경해줌 => 조건문 사용하지 않고, !todo.done으로 코드 줄일 수 있다.
        return {
          ...todo,
          done: false,
        };
      } else {
        return {
          ...todo,
          done: true,
        };
      }
    }
    return { ...todo }; // 객체에 새로 담지 않고 그냥 todo를 리턴 => 이미 새로운 배열이기 때문
  });
  todos = updateTodo;
  renderTodos();
};

1) 변수의 네이밍 : target은 이미 자바스크립트에서 사용되는 예약어이고, target이 무엇인지 정확하지 않아 한눈에 알아보기 어렵다. 이런 target이 반복되면서 가독성이 떨어진다. => li로 수정,

modifyTodo는 Todo를 수정한다는 의미인데, 무엇을 수정할지에 대해 정확하지 않아 Todo의 내용을 수정하는 것과 혼돈이 있을 수 있다. => done은 토글이기 때문에 tpggleComplateTodo로 변경

2) done의 여부에 따라 done의 상태를 true/false로 변경 : 굳이 조건문이나 반복되는 코드를 길게 작성하지 않아도 !연산자를 사용하여 boolean값을 반대로 조정할 수 있다. => done : !todo.done 으로 수정

3) return { ...todo} : 이미 파괴적인 방법이기 때문에 todo자체가 객체인데 굳이 새 객체에 {...todo}를 담을 필요 없이 바로 todo를 리턴하면 된다. => 근데 이렇게 하면 todo는 기존의 배열을 참조하고 있기 때문에 값을 변경하면 기존값도 변경된다.

♻️ 리팩토링)

const toggleCompleteTodo = (event) => {
  const li = event.target.parentElement; //target => li로 수정
  todos = todos.map((todo) => {
    if (todo.id === Number(li.dataset.id)) {
      return {
        ...todo,
        done: !todo.done, // done 여부를 간단하게 변경
      };
    }
    return todo; // {...todo}를 todo로 변경
  });
  renderTodos();
};

 

 

3. 추가적으로 궁금했던 부분들

- e와 event의 차이

addEventListener에서 매개변수 콜백의 자리에 우리가 실행하고 싶은 함수를 넣으면, 콜백에서 이미 자동적으로 이벤트를 함수에 전달해 준다. 따라서 e와 event 등 매개변수의 네이밍은 필수는 아니지만 우리가 이벤트를 받고 사용한다는 것을 인식할 수 있도록 넣어주는 것이며, e와 event 또한 매개변수 네이밍도 우리가 지정해 줄 수 있다. => 결과적으로 상관 없음. 차이는 없다.

ul.addEventListener("click", handleClickEvent); //event를 명시하지 않아도, 이미 event는 전달됐다.

const toggleCompleteTodo = (event) => { //매개변수로 받아주기만 하면 된다.

 

- 엄격모드('use strict') 를 꼭 사용해야 할까?

"use strict";

바닐라 자바스크립트를 사용할 때는 항상 엄격모드를 사용했었다. 이는 개발자의 비정상적인 실수들을 방지할 수 있는데(선언하지 않은 변수에 값 할당이라던지, 호출이라던지..) class를 사용할 때나 React에서는 사용하지 않았었다. React 자체에서 엄격모드를 이미 지원하고 있기 때문이었다. eslint에서 구체적인 컨벤션으로 조정도 가능하고...

결론적으로는 바닐라 자바스크립트를 사용할 때 엄격모드를 사용하면 개발자의 비정상적인 실수들을 방지할 수 있어 좋지만(어차피 React등의 라이브러리에서도 엄격모드로 개발하니..) 사실 실무에서 바닐라 자바스크립트로 개발할 일은 거의 없다고 봐야하기 때문에(React나 vue등 다양한 라이브러리를 이용) 엄격모드는 자바스크립트를 배우는 동안만 사용하면 될 것 같다. React등의 라이브러리를 사용하는 이유 자체가 규모가 큰 프로젝트나 코드를 구조적으로 관리하기에 굉장히 편하고 성능 개선에도 좋고 등등 편리하고 이점이 많기 때문이다. 바닐라 자바스크립트로 그런 프로젝트들을 리액트와 동일하게 관리할 수 있다면 굉장히 대단한...? 코드구조를 굉장히 체계적으로 잘 짜시는 개발자가 아닐까...?