인생 첫 리팩토링을 해보았다

김용현·2023년 12월 12일

LESSER 개발일지

목록 보기
2/7
post-thumbnail

1. 문제 상황

저는 여러 팀원들과 "Lesser"라는 프로젝트를 개발하고 있었습니다. 이 때, 개발 중이던 일부 코드를 저와 다른 팀원들이 여러 곳에서 재사용해야 했지만, 중간 발표를 위해 "당장의 화면을 보여주는 것"이 급했던 터라 해당 코드는 구조나 가독성을 신경쓰지 못한 채 개발되었습니다.

때문에 화면을 띄우고 서버에 데이터를 보내는 것은 훌륭히 해내고 있지만, 이 코드를 읽고 필요한 부분을 다른 팀원들이 쉽게 꺼내 사용하기 위해서는 길고 복잡하게 얽힌 코드들을 풀어내고 분리하는 과정이 필요했습니다. 그렇기에 저는 이 코드들을 풀어내는 인생 첫 "리팩토링"에 수행해보았습니다.

2. 문제 상황 분석

기존 코드를 보고 어째서 해당 코드가 읽고 재사용하기 어려운지에 대해 원인을 분석해보았습니다.

  1. 하나의 함수 및 컴포넌트에서 여러 개의 업무를 한번에 처리하기 위한 구조로 코드를 설계하였습니다. 다양한 업무를 처리하기 위해 자연스럽게 수 많은 조건문(if)로 구성 되었고, 각각의 조건에 따라서 화면이 구성되거나 함수가 동작하는지를 일일이 기억하며 판단하기 어려웠습니다.

    • 예를 들어 "useBlock"이라는 기존의 커스텀 훅은 "에픽", "스토리", "태스크"라는 요소를 생성하고 수정하는 모든 기능을 수행하고 있습니다. 뿐만 아니라, 생성하거나 수정하기 위해 Input 요소를 토글하는 기능도 함께 담당하고 있습니다.

      // useBlock.tsx
      const useBlock = ({ block, setBlock, epicIndex, storyIndex, taskIndex }: BlockOptions) => {
        const [newFormVisible, setNewFormVisibility] = useState<boolean>(false);
        const [updateFormVisible, setUpdateFormVisibility] = useState<boolean>(false);
        const formRef = useRef<HTMLFormElement>(null);
      	...
      }
    • 많은 기능을 한번에 처리하기 위해 useBlock은 조건을 분기하기 위해 어디에서 사용하고, 어떤 기능이 필요한지를 Argument로 전달받아야 합니다. 그렇기 때문에 제가 아닌 다른 개발자들은 useBlock이 어떻게 동작하는 지를 이해하기 위해선, 전달한 인자값을 기억하고, 수 많은 if 문을 파헤쳐야만 했습니다.
      (useBlock의 핵심 수행 코드 내에 if 문은 총 9개가 존재했습니다)

      // useBlock.tsx
      const useBlock = ({ block, setBlock, epicIndex, storyIndex, taskIndex }: BlockOptions) => {
      	...
      	if (...) {
      	...
      		} else {
                const updatedEpics = block.epicList.map((epic, index) => {
                  if (index === epicIndex) {
                    const updatedStories = epic.storyList.map((story, index) => {
                      if (index === storyIndex) {
                        return {
                          ...story,
                          title: blockTitle!,
                        };
                      }
                      return story;
                    });
      
                    return {
                      ...epic,
                      storyList: updatedStories,
                    };
                  }
                  return epic;
                });
  2. 여러 역할을 조건문으로 분기하여 동작하는 코드를 작성했기 때문에, 기존의 코드와 기능이 비슷한 새로운 기능을 개발하기 위해서는 기존의 코드에 새로운 조건문을 추가하는 방식을 사용해야 합니다.

    • 상기의 useBlock을 예로 들어, 만약 다른 팀원이 "에픽"과 유사한 요소를 생성하는 기능이 갑자기 필요해졌다고 상상해봅시다. 아마, 해당 팀원은 useBlock에 새로운 조건문을 추가한 후에야 useBlock을 사용할 수 있었을 것입니다. 매번 새롭게 호출할 때마다 내부 코드를 수정해야 하는 상황은 재사용성의 관점에서 좋지 못한 코드일 것입니다.
  3. 동일한 기능/형태를 가진 코드를 하나의 컴포넌트로 만들지 않고, 유사한 형태의 코드로 반복되는 부분들이 있었습니다.

    • Epic과 Story를 생성하는 버튼은 "서버에 새로운 데이터를 생성한다", "동일한 형태의 버튼 구조를 렌더링한다"는 공통점을 가지고 있습니다. 유일한 차이점은 서버에 생성 요청을 보내기 위한 "url"만 다르다는 것이지만, 해당 부분을 별도의 컴포넌트로 분리하여 재사용하고 있지 못했습니다.
    // BacklogPage.tsx
    // 에픽 생성 버튼
    <button
      className={`flex w-full py-[0.313rem] mt-4 rounded-md text-center justify-center bg-house-green font-bold text-true-white`}
      onClick={handleAddBlockButtonClick}
    >
      <PlusIcon color="text-true-white" />
      {`Epic 생성하기`}
    </button>
    
    // EpicBlock.tsx
    // 스토리 생성 버튼
    <button
      className="flex w-full py-[0.313rem] rounded-md text-center justify-center bg-accent-green font-bold text-true-white"
      onClick={handleAddBlockButtonClick}
    >
      <PlusIcon color="text-true-white" />
      {`Story 생성하기`}
    </button>

3. 해결 시도 방법

이번 리팩토링의 가장 큰 관점은 "해당 코드는 어떤 일을 해야 하는지"에 대해 "관심사"로 정의하고, 관심사를 기준으로 코드를 분리하고 재사용하도록 새롭게 설계해 보았습니다.

  1. "관심사"란 무엇인가요?

    • 여기서 "관심사"는 해당 코드가 어떤 일에 "관심"을 가지고 수행하는지를 기준으로 정의하였습니다.
    • 하나의 코드에는 하나의 관심, 즉 책임을 가지도록 정의했습니다. 이렇게 정의한다면 다른 팀원이 제 코드를 읽을 때, 하나의 흐름을 가지고 코드를 쉽게 이해할 수 있을 것이라 판단했습니다.
  2. 가장 먼저 하나의 코드에 집중된 많은 "관심사"를 정의하고, 분리하여 별도의 컴포넌트, 커스텀 훅으로 만들었습니다.

    • 기존의 useBlock의 코드를 살펴보면, "에픽", "스토리", "태스크"를 POST, PUT하는 기능과 해당 POST, PUT 데이터를 사용자로부터 입력받기 위해 Input을 토글하는 기능을 "관심사"로 가지고 있습니다.

    • useBlock의 각각의 관심사들을 분리하여 새로운 커스텀 훅을 만들어 보았습니다. Input을 토글하는 기능을 useToggleButton로, "에픽"과 "스토리"를 생성하는 기능을 usePostBackog로 "에픽"과 "스토리"를 새로운 데이터로 갱신하는 기능은 useUpdateBacklog로 분리하여 새로운 훅들을 만들어보았습니다.
      useBlock의 많은 기능들을 분리

      useBlock의 많은 기능들을 분리	
  1. 분리한 "관심사"에서 유사한 기능들이 있다면 새롭게 묶어 컴포넌트로 생성해보았습니다.

    • "에픽과 스토리를 생성하는 버튼"은 "서버에 새로운 데이터를 생성한다", "동일한 형태의 버튼 구조를 렌더링한다"는 공통점과 "서버의 다른 url로 요청을 보낸다”와 “색상이 다르다”는 차이점이 있습니다. 이러한 공통점에 주목하여, 요청을 보내는 API url과 배경 색상을 prop으로 전달 받는 형태의 코드로 만들어 재사용할 수 있도록 해보았습니다.

      // PostButton.tsx
      const PostButton = ({ title, placeholder, color, url, id }: PostButtonProps) => {
        const { postForm, toggleButton, postFormRef, getPostTitle } = useToggleButton();
        const getBody = () => {
          return { parentId: id, title: getPostTitle() };
        };
        const { handleClick } = usePostBacklog(url, getBody, toggleButton);
      
        return (
          <>
            {!postForm ? (
              <button
                onClick={toggleButton}
                className={`flex w-full py-1 rounded-md text-center justify-center ${color} font-bold text-true-white`}
              >
                <PlusIcon color="text-true-white" />
                <span>{title}</span>
              </button>
            ) : (
              <form ref={postFormRef} className="flex w-full gap-1 py-1" onSubmit={handleClick}>
                <input
                  type="text"
                  className="w-full border outline-starbucks-green font-normal ps-2"
                  placeholder={placeholder}
                />
                <button type="submit">
                  <CheckIcon color="text-true-white" backgroundColor="bg-starbucks-green" />
                </button>
                <button type="button" onClick={toggleButton}>
                  <ClosedIcon color="text-true-white" backgroundColor="bg-error-red" />
                </button>
              </form>
            )}
          </>
        );
      };
      
      export default PostButton;
  2. "관심사"는 다르지만, 공통된 UI를 활용한다면? custom hook과 View의 분리를 통해 재사용성을 높이자.

    • Task를 "생성하는 모달"과 "수정하는 모달"은 새로운 태스크를 "생성"하는 것과 기존의 태스크를 "수정"하는 서로 다른 관심사를 가지고 있다. 하지만 이 때 사용자에게 보여주는 화면은 동일한 컴포넌트를 사용하고 있습니다. 단지, defaultValue가 있는가, 없는가 차이일 뿐.

    • 그렇다면 비즈니스 로직이 존재하지 않고 화면을 구성하기만 하는 TaskForm 컴포넌트를 구성. TaskForm 컴포넌트 호출을 기반으로 별도의 비즈니스 로직(Patch, Post)을 가지는 별개의 모달(TaskUpdateModal, TaskPostModal)을 생성하여, 화면 구성 부분을 재사용할 수 있지만 별도의 관심사에 따라 별개의 컴포넌트를 생성하였습니다.

      // TaskForm.tsx
      
      const TaskForm = ({ handleSubmit, close, setNewTaskManager, formRef, defaultData = null }: TaskFormProps) => {
        const { detail, toggleDetail, detailRef } = useDropdownToggle();
        const { username, setNewUsername, resetUsername } = useTaskUsername(Number(defaultData?.userId));
        const handleDropdownClick = ({ currentTarget }: React.MouseEvent<HTMLButtonElement>) => {
          setNewTaskManager(Number(currentTarget.id));
          setNewUsername(Number(currentTarget.id));
          toggleDetail();
        };
        const handleDropdownResetClick = () => {
          setNewTaskManager(null);
          resetUsername();
          toggleDetail();
        };
      
        return (
          <div className="fixed top-0 left-0 bg-black w-screen h-screen bg-opacity-30 flex justify-center items-center">
            <form
              className="w-[31.875rem] h-[33.75rem] rounded-md bg-white p-6 text-house-green flex flex-col justify-between"
              onSubmit={handleSubmit}
              ref={formRef}
            >
              <p className="text-l font-bold">Task</p>
              <TaskInputLayout title="업무 내용" description="Story를 구현하기 위해 필요한 업무를 작성합니다" htmlFor="title">
                <input
                  className="w-full py-2 px-2.5 border rounded-sm border-starbucks-green outline-starbucks-green text-s"
                  type="text"
                  id="title"
                  name="title"
                  placeholder="어떤 업무를 수행할 예정인가요?"
                  defaultValue={defaultData?.title}
                />
              </TaskInputLayout>
              <TaskInputLayout title="인수 조건" description="Task를 완료하기 위한 조건을 작성합니다." htmlFor="condition">
                <textarea
                  className="w-full py-2 px-2.5 resize-none border rounded-sm border-starbucks-green outline-starbucks-green text-s "
                  rows={4}
                  id="condition"
                  name="condition"
                  placeholder={
                    '예시 조건)\n' +
                    '몇 개의 테스트 코드를 통과해야 합니다\n' +
                    '사전에 작성한 예상 유저 시나리오와 비교하여 동작을 확인합니다'
                  }
                  defaultValue={defaultData?.condition}
                />
              </TaskInputLayout>
              <TaskInputLayout title="담당자" description="Task를 수행할 멤버를 선정합니다" htmlFor="userId">
                <div className="relative">
                  <div className="w-[9.375rem] py-2 px-2.5 border rounded-sm border-starbucks-green outline-starbucks-green text-s flex items-center">
                    <p className="w-full">{username}</p>
                    <button
                      type="button"
                      onClick={() => {
                        toggleDetail();
                      }}
                    >
                      {detail ? <ChevronUpIcon size={16} /> : <ChevronDownIcon size={16} />}
                    </button>
                  </div>
                  {detail && (
                    <MemberDropdown
                      ref={detailRef}
                      setNewTaskManager={handleDropdownClick}
                      resetTaskManager={handleDropdownResetClick}
                    />
                  )}
                </div>
              </TaskInputLayout>
              <TaskInputLayout
                title="Task Point"
                description="Task를 완료하기 위해 소요되는 시간을 예상합니다"
                htmlFor="point"
              >
                <div className="flex w-[9.375rem] pr-3 items-center border rounded-sm border-starbucks-green justify-between">
                  <input
                    className="w-full py-2 px-2.5 text-s outline-none"
                    type="number"
                    id="point"
                    name="point"
                    defaultValue={defaultData?.point}
                    min={0}
                  />
                  <p className="font-bold text-starbucks-green">Point</p>
                </div>
              </TaskInputLayout>
      
              <div className="flex justify-end gap-3">
                <button
                  type="button"
                  className="border rounded-md border-starbucks-green w-14 py-1.5 font-bold text-starbucks-green text-s"
                  onClick={close}
                >
                  취소
                </button>
                <button type="submit" className="rounded-md w-14 py-1.5 bg-starbucks-green font-bold text-true-white text-s">
                  확인
                </button>
              </div>
            </form>
          </div>
        );
      };
      
      export default TaskForm;
      // TaskPostModal.tsx
      const TaskPostModal = ({ parentId, close }: TaskPostModalProps) => {
        const formRef = useRef<HTMLFormElement>(null);
        const { taskManagerId, setNewTaskManager } = useTaskManager();
        const getBody = () => {
          if (!formRef.current) return { parentId, title: '', userId: '', point: 0, condition: '' } as TaskPostBody;
          return [...formRef.current.querySelectorAll('input'), formRef.current.querySelector('textarea')].reduce(
            (acc: TaskPostBody, cur: HTMLInputElement | HTMLTextAreaElement | null) => {
              if (!cur) return acc;
              const newData = {
                ...acc,
                [cur.id]: cur.id === 'point' ? Number(cur.value) : cur.value,
              };
              return newData;
            },
            { parentId, userId: taskManagerId } as TaskPostBody,
          );
        };
        const queryClient = useQueryClient();
        const { mutateAsync } = useMutation({
          mutationFn: async () => {
            return await api.post('/backlogs/task', getBody());
          },
          onSuccess: () => {
            queryClient.invalidateQueries({ queryKey: ['backlogs'] });
          },
        });
      
        const handleSubmit = async (e: React.FormEvent) => {
          e.preventDefault();
          await mutateAsync();
          close();
        };
      
        return (
          <>
            <TaskForm close={close} handleSubmit={handleSubmit} formRef={formRef} setNewTaskManager={setNewTaskManager} />
          </>
        );
      };
      
      export default TaskPostModal;
      
      // TaskUpdateModal
      import { useMutation, useQueryClient } from '@tanstack/react-query';
      import { useRef } from 'react';
      import { api } from '../../../apis/api';
      import TaskForm from '../taskForm/TaskForm';
      import { TaskModalProps } from './TaskModal';
      import { ReadBacklogTaskResponseDto } from '../../../types/backlog';
      import useTaskManager from '../../../hooks/pages/backlog/useTaskManager';
      
      const TaskUpdateModal = ({ close, id, title, userId, point, condition }: TaskModalProps) => {
        const formRef = useRef<HTMLFormElement>(null);
        const { taskManagerId, setNewTaskManager } = useTaskManager(!userId ? null : Number(userId));
        const getBody = () => {
          if (!formRef.current) return { id, title, userId, point, condition };
          return [...formRef.current.querySelectorAll('input'), formRef.current.querySelector('textarea')].reduce(
            (acc: ReadBacklogTaskResponseDto, cur: HTMLInputElement | HTMLTextAreaElement | null) => {
              if (!cur) return acc;
              const newData = {
                ...acc,
                [cur.id]: cur.id === 'point' ? Number(cur.value) : cur.value,
              };
              return newData;
            },
            { id, userId: taskManagerId } as ReadBacklogTaskResponseDto,
          );
        };
        const queryClient = useQueryClient();
        const { mutateAsync } = useMutation({
          mutationFn: async () => {
            return await api.patch('/backlogs/task', getBody());
          },
          onSuccess: () => {
            queryClient.invalidateQueries({ queryKey: ['backlogs'] });
          },
        });
      
        const handleSubmit = async (e: React.FormEvent) => {
          e.preventDefault();
          await mutateAsync();
          close();
        };
      
        return (
          <>
            <TaskForm
              close={close}
              handleSubmit={handleSubmit}
              setNewTaskManager={setNewTaskManager}
              formRef={formRef}
              defaultData={{ id, title, userId: taskManagerId, point, condition }}
            />
          </>
        );
      };
      
      export default TaskUpdateModal;

4. 결론

  • "관심사"라는 기준으로 코드를 분리하면 자연스레 하나의 "파일" 안에는 "하나의 주제"만을 가지도록 의도했고 코드의 주제를 "함수명", "변수명"으로 최대한 이해할 수 있기 위해 표현하고자 했습니다. 이 덕분에, 다른 팀원들이 해당 코드들을 읽고 이해하며, 재사용하기에 좋은 코드로 만들 수 있었다고 생각합니다.
  • "관심사"라는 의미를 고민하며, 사람들이 만들어가고 저희가 배우는 많은 코드 작성법들의 주요한 의미 중 하나는 "얼마나 다른 사람들과 코드로 쉽게 소통할 수 있을까?"라는 것을 익힐 수 있었습니다. 제가 만든 코드를 누군가는 재사용해야 하고, 누군가는 유지보수를 해야할 것입니다. 또 다른 팀원들은 버스 팩터를 위해 제 코드를 완벽하게 숙지해야 하구요. 그렇다면 제가 얼마나 쉽게 읽을 수 있는 코드를 작성하느냐에 다른 사람들의 업무 효율성이 강하게 의존하고 있는 상황으로 볼 수 있는 것입니다. 그렇기에 많은 사람들은 변수명, 함수명을 짓고 파일의 아키텍쳐와 코드의 길이에 민감하게 신경쓰는 이유를 이해할 수 있었습니다.
  • 하지만, 아직 리팩토링이 완벽하게 이루어진 것은 아닙니다. 파일의 구조 간 일관성이 마땅히 존재하지 않기 때문에, 어떤 컴포넌트 안에는 비즈니스 로직이 존재하고, 어떤 컴포넌트는 커스텀 훅을 통해 분리하는 등 불규칙성이 아직 코드 안에 남아있습니다. 아직 많은 리팩토링, 좋은 코드를 본 경험이 부족하기 때문에 제 안에 나름의 리팩토링 규칙을 만들지 않은 탓이라고 생각이 듭니다. 이번, 네이버 부스트 캠프의 6주간의 과정을 마무리하고 나면 팀원들과 어떻게 코드를 구성해야 서로가 더 읽기 쉬울지 함께 이야기 해보며 저 나름의 규칙을 차근차근 쌓아보아야 겠습니다.
profile
함께 일하고 싶은 개발자가 되기위해 노력 중입니다.

0개의 댓글