20221208 TIL 리팩토링을 조금 더 편하게

2주에 걸쳐 과제 구현을 완료했다!

 

과제를 완료했기 때문에 어제 작업했던 페이지네이션 컴포넌트 리팩토링을 진행하였다.
이때까지 리팩토링을 할 때 리팩토링을 원래 파일에서 하다보면 혼란스러웠던 경우가 많았다.

그리고 기존 파일에서 작업할 때 작업을 하는데도 어려움이 느껴졌었기 때문에

리팩토링을 제대로 하는 경우에 다른 방식으로 접근을 해야겠다는 생각을 했었고,

예전에 홀맨님께 조언을 구했었다.

그런데 그동안 리팩토링을 제대로 할 일이 많지 않았기 때문에 그 방식을 한동안 까먹고 있었다.

 

오늘은 어제 작성한 무지막지한 페이지네이션 컴포넌트 코드를 제대로 리팩토링을 해야됐는데,

어떻게 하면 조금 편하게 리팩토링을 할 수 있을까 고민하는 중에

홀맨님이 말씀해주신 방식이 떠올랐다.

바로 기존 파일을 복사해서 복사한 파일에서 리팩토링을 진행하는 것이었다.

테스트 코드는 이미 있기 때문에 안심하고 리팩토링을 진행할 수 있었고, 파일을 복사해서 중복되는 로직을 분리했다.

 

어제 생각해본 대로 결국 페이지네이션 컴포넌트에서 쓰일 값들을 배열로 뽑아내기만 하면

컴포넌트를 매우 작게 만들 수 있기 때문에 페이지네이션 컴포넌트에서 쓰일 값들(페이지 값들)을 뽑아내는 함수를 만드는 것부터 시작했다.

두 페이지 사이 값이 2이상 차이나는 경우 ... 으로 표시를 해야했는데, 

리액트에서 map을 돌리는 경우 고유한 key가 필요하다.

...는 최대 두 개가 있을 수 있고, 페이지 값들은 양수이기 때문에 key를 -1과 -2를 주었고,

여러 번의 리팩토링 끝에 아래와 같이 리팩토링할 수 있었다.

동기분의 리팩토링 중의 코드를 보고 복잡해보인다는 말을 듣고 rangeOf 메서드를 분리할 수 있었다.

지금도 깔끔하진 않지만 동기분의 말씀 덕분에 메서드를 하나 더 분리할 수 있어 조금 더 깔끔해진 것 같다.

const rangeOf = ({ size, start = 0 }) => [...Array(size).keys()].map((i) => i + start);

const pageValues = ({ totalPages, currentPage }) => {
  if (totalPages < 8) {
    return [...rangeOf({ size: totalPages })];
  }

  if (currentPage < 5) {
    return [...rangeOf({ size: 5 }),
      { key: -1, value: '...' },
      totalPages - 1];
  }

  if (totalPages - currentPage < 4) {
    return [0,
      { key: -1, value: '...' },
      ...rangeOf({ size: 5, start: totalPages - 5 })];
  }

  return [0,
    { key: -1, value: '...' },
    ...rangeOf({ size: 3, start: currentPage - 2 }),
    { key: -2, value: '...' },
    totalPages - 1];
};

export default function Pagination({ url, totalPages, currentPage = 1 }) {
  const navigate = useNavigate();

  const handleClickPrevious = () => {
    navigate(`${url}?page=${currentPage - 1}`);
  };

  const handleClickNext = () => {
    navigate(`${url}?page=${Number(currentPage) + 1}`);
  };

  if (!totalPages) {
    return null;
  }

  const pages = pageValues({
    totalPages: Number(totalPages),
    currentPage: Number(currentPage),
  });

  return (
    <ul>
      <li>
        <button
          type="button"
          title="previous"
          name="previous"
          disabled={currentPage < 2}
          onClick={handleClickPrevious}
        >
          previous
        </button>
      </li>
      {
        pages
          .map((page) => (
            typeof page === 'number'
              ? (
                <li key={page}>
                  <Link
                    to={`${url}?page=${page + 1}`}
                    selected={Number(currentPage) === page + 1}
                  >
                    {page + 1}

                  </Link>
                </li>
              ) : (
                <li key={page.key}>
                  <p>
                    {page.value}
                  </p>
                </li>
              )
          ))
      }
      <li>
        <button
          type="button"
          title="next"
          name="next"
          disabled={currentPage > totalPages - 1}
          onClick={handleClickNext}
        >
          next
        </button>
      </li>
    </ul>
  );
}

항상 복잡해 보이면 분리할 수 있는 메서드가 있는지 꼭 확인하자.

그리고 이번에 리팩토링할 때 복사를 해서 리팩토링하는 것이 매우 편함을 느꼈다.

테스트 코드가 있어서 마음놓고 리팩토링을 할 수 있었고, 기존 코드도 이미 있기 때문에 마음을 놓고 리팩토링을 할 수 있었던 것 같다.

앞으로도 리팩토링할 때 기존 파일을 복사해서 수정해서 테스트가 통과되면

기존 파일을 리팩토링한 파일로 대체하는 방식으로 작업하면 좋을 것 같다!