싸가지 있게 또두리스트 리팩토링하기.

싸가지 있게 또두리스트 리팩토링하기.

'청소광 브라이언' 보다가 더러운 코드 상태 찔려서 리팩토링한 과정을 기록해봤습니다.

·

9 min read

나는 안다. 내가 얼마나 싸가지인지..ㅋㅋㅋㅋㅋ 1년 간의 경력을 쌓으면서, 가장 마음에 걸렸던건 나는 과연 협업하기에 좋은 코드를 고민하고 짤 줄 아는 사람인가? 에 대한 것이었다. 그리고 얼마 전.. 브라이언 선배님의 절규하는 소리를 듣다가 나도 이제 청소를 해야겠다는 생각에 지금까지 짰던 코드들을 리팩토링하는 시간을 가져봤다.

그 전에 다시 한 번.. 브라이언 형님 보고 마음 다 잡기..


🤔 어떻게 기준을 잡을래?

‘협업하기 좋은 코드’라는 말은 되게 현실적으로 확 와 닿지는 않는다. 내가 아무리 잘 짰다고 하더라도, 또 누군가에게는 분석하고 어렵게 파악해야하는 코드가 될 수 있으니까. 그래서 이 기준점에 대해서는 우선 이전에 포스팅 했던 내용들을 많이 떠올리려 했다. TOSS SLASH 에서 발표했던 내용들이었는데, 주요 내용은 다음과 같다.

  • 하나의 함수는 한 가지 역할만 하자

  • 함수의 세부 구현 단계가 제각각이지 않도록 하자

  • 하나의 목적을 가진 코드로 응집 시키자

  • 추상화의 레벨을 맞춰주자

추가적으로 생각해본 기준은 내가 지금까지 개발을 하면서 느꼈던 것들을 위주로 잡아봤다.

  • 개인 프로젝트더라도 어느 정도의 컨벤션을 맞추자

  • 작성해나가는 것들은 명확한 의도와 목적이 있도록 하자

  • 굳이 어렵게 짜는 것보다 편하게 읽힐 수 있는 것들을 고민하자

  • 불필요한 props 는 피해보자 - 컴포넌트 간 의존성을 줄이고 싶음

음 사실 여러 기준들을 고려하면서 짜봤는데, 기능이 그리 많지는 않아서 모든 것을 담지는 못했을 수도 있다. 하지만 고치면 고칠 수록 조금씩 나아지던 코드들을 정리하며 앞으로 어떻게 고려하며 작업하면 좋을지에 대해 자취를 남겨보고 싶었다.


📒 또두리스트.

뭔가 프론트엔드 개발자라면 최고의 입문 과제가 “TodoList” 가 아닐까 싶다. 나도 개발을 시작하면서 정말 많은 투두리스트를 만들었었는데, 이제는 뭔가 카운트도 못할 것 같다. 하지만 이렇게 간단해 보이는 친구도 매번 새로 만들 때 마다 새롭게 컴포넌트 설계를 해보게 되고 다른 시도들을 하게 되는데.. 간단한 것은 없는 것 같다.

어쨌든 이번에는 “TodoList”의 가장 기본적인 폼이라 할 수 있는 위와 같은 프로젝트를 만들어봤다. 참 이것저것 많이 시도해봤는데.. 지금부터 고민했던 것들에 대해서 끄적여봐야겠다.

😶‍🌫️ 뭔가 정리되지 않은 느낌.

처음 리스트를 구성할 때, 위 리스트는 돌려쓰면 좋겠다는 생각이 들어서 하나의 컴포넌트로 분리하려 했다. 분리를 하면서 고려했던 점은 두 가지였다.

  1. title 영역의 UI가 비슷하기 때문에 props 를 활용하자.

  2. 내부의 리스트 영역은 각각의 역할이 다르기 때문에 돌려쓴다는 개념을 버리고 싶다.

    • TODO 영역은 항목 수정이 가능함 → 포함하는 로직이 많아질 것

    • DONE 영역은 항목 수정이 불가능 → 삭제 / 취소 버튼만 있으면 됨

그래서 아래와 같이 처음에는 작성했었다. <ListSection /> 이라는 하나의 템플릿 같은 컴포넌트를 만들고 list 배열은 children 으로 내려줘서 템플릿 내부에 들어가는 것들에 자유도를 높였다. 음 크게 문제는 없었던 것 같은데, 뭔가 눈에 잘 안들어왔다.

function TodoPage() {
  const [todoList, setTodoList] = useGetTodo();

  const todoData = todoList.filter((value) => value.done === false);
  const doneData = todoList.filter((value) => value.done === true);

  return (
    <>
      <TodoForm setTodoList={setTodoList} />
      <ListSection title="✋🏻 TODO" loading={loading}>
        <ul>
          {notArchivedList.map((value) => (
            <li key={value.entityId}>
              <ListItem data={value} setList={setList} />
            </li>
          ))}
        </ul>
      </ListSection>
      <ListSection title="👩🏻‍🌾 DONE" loading={loading}>
        <ul>
          {archivedList.map((value) => (
            <li key={value.entityId}>
              {getListItem(value.name, value.description)}
            </li>
          ))}
        </ul>
      </ListSection>
    </>
  );
}

<TodoForm/> 의 경우에는 하나의 컴포넌트로 완전히 분리된 반면에 List 의 경우는 두 개의 파일에서 각각 나뉘어져있다 보니 조금 어지러웠다. 심지어 여기에서 각각의 리스트에 버튼도 달아줘야 하는데, 그렇게 되면 버튼은 어떤 경우는 ListItem 이라는 컴포넌트 내부에 위치해야하고 어떤 경우는 최상단인 Page 에서 관리되어야 했다.

그래서 아래와 같이 리팩토링을 해봤다. 이 단계에서 리팩토링을 할 때 중점을 둔 것은 하위 컴포넌트들의 추상화된 레벨을 최대한 일치시키는 것이었다. 지금은 뭔가 FormList 의 추상화 레벨 사이에 이질감이 느껴졌다.

function TodoPage() {
  const [todoList, setTodoList] = useGetTodo();

  return (
    <UI.Layout>
      <TodoContext.Provider value={{ todoList, setTodoList }}>
        <TodoForm />
        <TodoList category="active" />
        <TodoList category="done" />
      </TodoContext.Provider>
    </UI.Layout>
  );
}

이번에 리팩토링한 코드에서는 TodoPage 내부의 FormList 컴포넌트를 봤을 때 관련 로직들이 해당 컴포넌트 내부에 위치하고 있다는 것을 한 눈에 알아볼 수 있었다. List 컴포넌트와 관련한 로직과 컴포넌트들이 여러 파일에서 관리되고 있지 않아서, 다음 스텝의 로직을 작성하기에도 편리했다.


😎 함수명 예쁘게 지어주기 그리고 예쁘게 나눠주기.

음 이번엔 함수를 좀 정리해봤다. 이 단계에서는 두 가지 기준을 잡아봤다.

  • 함수 하나는 하나의 역할만 할 것

  • 함수명이 직관적일 것

이 두 가지 기준으로 아래 코드를 봤을 때, 어떤 것들이 개선되면 좋았을까?

function TodoForm() {
    // ...

  const onChange = (event: React.ChangeEvent<HTMLInputElement>) => {
    const value = event.target.value;
    const id = event.target.id;
    if (!value || value.trim() === '') {
      setError((prev) => ({ ...prev, [key]: true }));
      return;
    }

    setError((prev) => ({ ...prev, [key]: false }));
    setFormItem((prev) => ({ ...prev, [id]: value }));
  };

  const onSubmit = async (event: React.FormEvent<HTMLFormElement>) => {
    event.preventDefault();
    const response = await TodoProvider.createTodo({
      id: data.id,
      title: formItem.title,
      content: formItem.content,
      done: data.done,
    });
    setTodoList(response);
  };

  return (
    <UI.Form onSubmit={handleSubmit}>
      {/* ... */}
    </UI.Form>
  );
}

export default TodoForm;

먼저 유효성 검사를 하는 함수와 FormItem 의 상태를 관리하는 함수를 구분해줬다. 뭔가 별 것 아니어 보이면서도 역할이 섞여있는 느낌이라, 조금 더 각각의 역할이 직관적으로 보였으면 좋겟다는 생각을 했다.

그리고 다음으로는 함수의 이름을 변경했는데, 위 함수들은 특정 이벤트를 직접적으로 발생시키는 역할을 하면서 각자의 역할들이 있으니 handle${역할} 의 모습으로 수정해줬다.

function TodoForm() {
    // ...
    const handleValidateForm = (event: React.ChangeEvent<HTMLInputElement>) => {
    const {value, id} = event.target;

    if (!value || value.trim() === '') {
      setError((prev) => ({ ...prev, [id]: true }));
      return;
    }
    setError((prev) => ({ ...prev, [id]: false }));
  };

  const handleChange = (event: React.ChangeEvent<HTMLInputElement>) => {
    const {value, id} = event.target;

    setFormItem((prev) => ({ ...prev, [id]: value }));
  };

  const handleSubmit = async (event: React.FormEvent<HTMLFormElement>) => {
    event.preventDefault();
    const response = await TodoProvider.createTodo({
      title: formItem.title,
      content: formItem.content,
    });
    setTodoList(response);
  };

  return (
    <UI.Form onSubmit={handleSubmit}>
      {/* ... */}
    </UI.Form>
  );
}

export default TodoForm;

코드가 조금 길어지긴 했는데, 읽기에는 더 편해진 것 같다.

  • onChange (유효성 검사 + formValue 관리)

    → handleValidateForm : 유효성 검사

    → handleChange : formValue 관리

  • onSubmit (폼 등록)

    → handleSubmit

함수의 이름을 지을 때, prefix 를 handle 로 잡는 것과 on 으로 잡는 것에 대해 검색을 좀 해봤다. 컴포넌트의 props 로 이벤트가 연결됨을 의미한다면 on 을 prefix 로 가져가고, 이벤트가 발생했을 때 호출되는 실제 함수를 의미한다면 handle 를 가진다는 레퍼런스를 봐서 이렇게 기준을 잡게 되었다.


🎁 props 최소화해주기.

컴포넌트의 추상화 레벨을 맞추면서, 또 나름 그 역할에 맞게 나눠주다보니 뎁스가 깊어지는 경우가 발생했다. 그런데 최상단에 있는 stateaction 을 가장 아래에 있는 녀석이 사용하는 경우가 있어서 컴포넌트를 깊게 나눠준 것에 대해 고민을 해봤다.

하지만 이유 있는 분리였고.. 👀 다시 합치기엔 내부 로직 파악이 더 어려워질 것 같았다. 그래서 props 를 깊게 내려주기 보다 Context API를 사용하는 쪽을 고려했다.

// 컴포넌트 구조
TodoPage
    ㄴ TodoForm
    ㄴ TodoList
         ㄴ TodoListItem

setTodoList 라는 setter<TodoPage> 에 위치해있는데, <TodoListItem> 컴포넌트에서 사용해야해서 처음에는 모두 props 로 내려줬었다. 하지만 이렇게 props 가 여러 단계에서 이어지게 된다면 추후 유지보수할 때 난이도가 점점 어려워졌다. 이것 뿐만아니라 다른 것들도 공유를 해줘야하기도 했다.

그래서 그 체인을 좀 끊어주면 좋겠어서 아래와 같이 Context API를 적용해줬다. 기존에는 아래와 같이 하위 컴포넌트에서 필요로하는 것들을 모두 직접 내려주고 있었다.

function TodoPage() {
  const [todoList, setTodoList] = useGetTodo();
  const [loading, setLoading] = useState<boolean>(false);
  const todoData = todoList.filter((value) => value.done === false);
  const doneData = todoList.filter((value) => value.done === true);

  return (
    <UI.Layout>
      <TodoForm setTodoList={setTodoList} setLoading={setLoading}/>
      <TodoList category="active" data={todoData} setTodoList={setTodoList} loading={loading}/>
      <TodoList category="done" data={doneData} setTodoList={setTodoList} loading={loading}/>
    </UI.Layout>
  );
}

export default TodoPage;

그리고 공유하고자 하는 친구들을 TodoContext 로 묶어서 Provider 로 랩핑해 전달해주는 식으로 리팩토링을 해줬다. 이 작업으로 props가 변경될 때 마다 파일 하나씩 찾아가서 변경해줘야하는 어려움도 덜었다. 물론 ContextAPI 가 가지는 문제점도 있기 때문에 이런 것들을 잘 고려해서 적절하게 사용해주면 좋겠다.

interface TodoContextType {
  todoList: TodoEntity[];
  setTodoList: React.Dispatch<React.SetStateAction<TodoEntity[]>>;
  loading: boolean;
}

export const TodoContext = createContext<TodoContextType | null>(null);
function TodoPage() {
  const [todoList, setTodoList, initLoading] = useGetTodo();

  return (
    <UI.Layout>
      <TodoContext.Provider value={{ todoList, setTodoList, loading: initLoading }}>
        <TodoForm />
        <TodoList category="active" />
        <TodoList category="done" />
      </TodoContext.Provider>
    </UI.Layout>
  );
}

export default TodoPage;

음, 추가적으로 context 사용처에서는 useContext 를 사용하면 값에 접근해서 사용할 수 있지만, 그 값이 null 일 경우의 예외처리를 위해 아래와 따로 커스텀훅을 만들어서 사용해줬다. 예전에 벨로퍼트님의 Veltrends 라이브 방송 때 유심히 봐뒀던 건데 이렇게 써보는구나! 감사합니당

import { useContext } from 'react';
import { TodoContext } from '..';

export function useTodoContext() {
  const context = useContext(TodoContext);
  if (!context) throw new Error('TodoContext 가 없어용');
  return context;
}
import { useTodoContext } from '../context/useTodoContext';

function TodoForm() {
  const { setTodoList } = useTodoContext();

    // ...
  return (
    <UI.Form className="todo_form" onSubmit={handleSubmit}>
      {/* ... */}
    </UI.Form>
  );
}

🌐 Loading 처리는 어떻게?

데이터를 다루다보면 에러, 로딩 케이스에 대한 조건부 렌더링을 해줘야하는 경우가 생기는데 그럴 때 마다 늘어나는 삼항 연산자들로 뭔가 어질어질해질 때가 있다. 물론 이게 나쁜건 아닌데, 막상 마주치면 조금 더 명확하면 좋겠다라는 욕심은 늘 생기는 것 같다.

아래의 경우도 로딩 중에는 스피너를 띄우고, 로딩이 끝나면 리스트가 렌더링이 되도록 작성되었다. 그래서 이번엔 리액트의 <Suspense> 를 떠올리며 <Loading> 컴포넌트를 만들어봤다. 이 투두리스트는 실제로 데이터 fetching 을 하고 있는 경우가 아니었어서 그냥 수작업으로 만들었다.

function TodoList(props: TodoListProps) {
  const { todoList, loading } = useTodoContext();
    // ...

  return (
    <UI.ListLayout>
      <h3>{isActiveList ? '✋🏻 TODO' : '👩🏻‍🌾 DONE'}</h3>
            {loading ? (
                    <Spinner/>
                ) : (
            <ul>
              {isEmptyList && <h4>is Empty!</h4>}
              {listData.map((value) => (
                <li key={value.id} id={value.id}>
                  <UI.ListItemBox>
                    <TodoListItem data={value} />
                  </UI.ListItemBox>
                </li>
              ))}
            </ul>
                )}
    </UI.ListLayout>
  );
}

아래와 같이 loading 여부에 따라 렌더링 시켜주는 컴포넌트를 만들어줬다. 이제 로딩 처리가 필요한 컴포넌트를 Loading 컴포넌트로 묶어주기만 하면 된다.

interface LoadingProps extends PropsWithChildren {
  loading: boolean;
}

const Loading = (props: LoadingProps) => {
  const { children, loading } = props;
  return <>{loading ? <Spinner /> : children}</>;
};

export default Loading;

음 훨씬 보기가 편해졌다. 물론 Suspense 와 구조는 다르다. Loading 컴포넌트의 경우에는 로딩 상태를 받아서 정해진 UI 를 보여주는 식으로 설계한 것이기 때문에! 음 여기서 로딩 상태의 UI 를 유연하게 가고 싶다면 그 부분만 조금 더 손 봐주면 좋지 않을까 싶다.

쨌든, 로딩 상태에 따른 렌더링을 아래와 같이 작업해주니 조금 더 정돈된 느낌도 들고 의미도 눈에 잘 들어와서 좋았다.

function TodoList(props: TodoListProps) {
  const { todoList, loading } = useTodoContext();
    //...
  return (
    <UI.ListLayout>
      <h3>{isActiveList ? '✋🏻 TODO' : '👩🏻‍🌾 DONE'}</h3>
      <Loading loading={loading}>
        <ul>
          {listData.map((value) => (
            <TodoListItem key={value.id} data={value} />
          ))}
        </ul>
      </Loading>
    </UI.ListLayout>
  );
}

👧🏻 의미를 파악하기 편하게.

이번엔 지금까지 개발을 하면서 어려웠던 점들을 돌아보면서 적용해본 것이다. 리액트로 개발을 해주다보면 조건부 렌더링 관련해서 각각의 조건을 return 문 안에서 써주는 경우가 많은데, 이 조건은 간단한 연산일 수도 있고 복잡한 연산일 수도 있다. 그런데 이런 조건들을 return 문 안에서 사용하다보면 UI 만 파악하고 싶은데 조건들에 대한 의미들을 파악하러 다시 올라가야되는 경우도 생겨 어려울 때가 많았다.

그래서 음 뭔가 이 코드들이 어떤걸 의미 하는지에 대해 파악이 쉽고 잘 전달이 되면 좋겠다는 생각이 들어서 최대한 이름을 지어줬다. 예를 들면, 아래와 같이 작성을 한다면 UI를 파악하는 곳에서 각 조건들의 의미에 대해서 생각을 해줘야한다. 지금 예제는 굉장히 간단한 케이스지만 서비스가 커질수록 더 어려워질 것이다.

function TodoList(props: TodoListProps) {
  const isActiveList = props.category === 'active';
  const isEmptyList = listData.length === 0;

  return (
    <UI.ListLayout>
      <h3>{props.category === 'active' ? '✋🏻 TODO' : '👩🏻‍🌾 DONE'}</h3>
      <ul>
        {listData.length === 0 && <h4>is Empty!</h4>}
        {...}
      </ul>
    </UI.ListLayout>
  );
}

이런 조건들에 대해 상단 로직을 관리하는 쪽에서 어떤 것을 의미하는지 이름을 지어주고, 이를 활용해서 UI 를 그려준다면 로직은 로직이 있는 부분에서 관리할 수 있고 UI를 파악할 때는 그 의미에 따른 변화만 파악할 수 있어서 좋다고 생각해서 아래와 같이 적용을 해줬다.

function TodoList(props: TodoListProps) {
  const isActiveList = props.category === 'active';
  const isEmptyList = listData.length === 0;

  return (
    <UI.ListLayout>
      <h3>{isActiveList ? '✋🏻 TODO' : '👩🏻‍🌾 DONE'}</h3>
      <ul>
        {isEmptyList && <h4>is Empty!</h4>}
        {...}
      </ul>
    </UI.ListLayout>
  );
}

👩🏻‍🌾 정리

막상 어떤 것들을 적용했는지 적어보니 별 것 없어서 머쓱하기도 한데ㅋㅋㅋ 그래도 이런 작은 것들을 많이 고민해보고 수정해본 경험이 나에게 큰 거름이 되줄 것이라고 생각하며.. 🙂 태초의 모습보다는 많이 읽기 편해진 나의 또두리스트야 고맙다!

물론 더 좋은 방법들도 있었을 것이다. 그리고 또 여러 경험들을 쌓다보면 또 다른 설계를 가지고 와서 새롭게 짜볼 수 있을 것이다. 그래도 아직 1년, 나에게는 쌓아갈 시간들이 아직 많고 계속 더 좋은 것들을 고민해보고 싶다. 그러니 혹시 이 글을 보고 더 좋은 방법이 있다면 많이 의견을 주셨으면 좋겠습니다. 😉


🥰Special Thanx to.

리팩토링한 내용을 가지고 주변 분들께 코드리뷰 요청을 드렸었는데, 시간 내어서 봐주신 Kevin, 진성님, 헤이즐님 정말 감사드립니다. 🥰 감사한 마음을 담아 추상화 레벨 관련해서 써보려고 그렸던 피카츄 그림을 선물로 드립니다..