인디버디 리팩토링하기 (2)

Home 리팩토링 - 2

이전 포스트에서 Home컴포넌트를 1차적으로 리팩토링을 진행했다. 하지만 그 때 한 부분은 리팩토링을 하는 부분이 적었다고 볼 수 있다. 오늘부터 진행할 부분은 손 댈 곳이 정말 많다고 생각한다. 이번 프로젝트를 하면서도 느꼈고 하고 나서도 느낀 것인데 시간에 쫓겨 급하게 코드를 작성하게 되면 재사용은 물론이고 의미를 한 번에 알기 어려운 코드가 짜여지게 된다. 내 실력을 너무 믿지 않는다. 자신감이 없는 것이 아니라 누구나 한 번에 좋은 코드를 짜기 어렵고 심지어 나는 개발 경력이 오래되지 않았기 때문에 더 그럴 것이다. 그래서 코드를 짜면서 많은 고민을 하는 습관을 들였다. 이제는 그 고민을 시간 안에 녹여내는 것을 연습해야 한다.

Home 하위 컴포넌트

GameContainer

오늘 리팩토링 할 부분은 GameContainer 컴포넌트다. 컴포넌트 명만 보면 어떤 컴포넌트인지 알 수 없다.(여기서 벌써 컴포넌트 명이 불분명하다는 문제가 있는 것일까..)

위 부분이다. 크게 보면 2개의 부분으로 나눌 수 있다. 인기, 신규 탭을 보여주는 네비게이션 바과 게임 카드와 페이지네이션이 보여지는 부분이다. 또 게임 카드들과 페이지네이션도 분리할 수 있다.

위 작업을 위해 내가 짠 코드를 봐보자. 코드가 굉장히 지저분하고 길 것이니 미리 주의하자.

리팩토링 전

const GameContainer = () => {
  const user = useSelector((s: RootState) => s.user);
  const [games, setGames] = useState<GameType[]>([]);
  const [curPage, setCurPage] = useState(1);
  const [pageInfo, setPageInfo] = useState<PageInfoType>({
    page: 1,
    size: 14,
    totalPage: 1,
    totalSize: 1
  });

  const [tabInd, setTabInd] = useState<number>(0);

  const apiRef = [
    '/api/games/?filter=POPULAR',
    '/api/games/?filter=NEW',
    `/api/members/${user.memberId}/creategame`,
    `/api/members/${user.memberId}/mygame`
  ];

  const onClickHandler = (i: number) => () => {
    setTabInd(i);
  };
  const onPageClickHandler = (page: number) => {
    setCurPage(page);
  };

  useEffect(() => {
    if (tabInd === 0 || tabInd === 1) {
      (async () => {
        try {
          const res = await axios.get(
            `${process.env.REACT_APP_API_URL}${apiRef[tabInd]}&page=${
              curPage !== pageInfo.page ? curPage : 1
            }&size=${pageInfo.size}`
          );
          setGames(res.data.data);
          setPageInfo(res.data.pageInfo);
          // isTabIndChanged() && setCurPage(1);
          curPage === pageInfo.page && setCurPage(1);
        } catch (err) {
          console.error(err);
        }
      })();
    } else {
      (async () => {
        try {
          const res = await axios.get(
            `${process.env.REACT_APP_API_URL}${apiRef[tabInd]}`
          );
          const newPageInfo = {
            ...pageInfo,
            page: curPage !== pageInfo.page ? curPage : 1,
            totalSize: res.data.data.length,
            totalPage: Math.ceil(res.data.data.length / 14)
          };

          const startInd = (newPageInfo.page - 1) * newPageInfo.size;
          const endInd = startInd + newPageInfo.size;
          setGames(res.data.data.slice(startInd, endInd));
          setPageInfo(newPageInfo);
          // isTabIndChanged() && setCurPage(1);
          curPage === pageInfo.page && setCurPage(1);
        } catch (err) {
          console.error(err);
        }
      })();
    }
  }, [tabInd, curPage]);

  return (
    <StyledWrapper>
      <StyledContainer>
        <FilterBar onClickHandler={onClickHandler} tabInd={tabInd} />
        <StyledCardContainer>
          {games.length === 0 && (
            <StyledEmptyItem>등록된 게임 채널이 없습니다.</StyledEmptyItem>
          )}
          {games.map((game) => (
            <HomeGameCard key={game.gameId} {...game} />
          ))}
        </StyledCardContainer>
        {games.length > 0 && (
          <StyledPagination>
            <Pagination
              activePage={curPage}
              itemsCountPerPage={pageInfo.size}
              totalItemsCount={pageInfo.totalSize}
              pageRangeDisplayed={5}
              prevPageText={'‹'}
              nextPageText={'›'}
              onChange={onPageClickHandler}
            />
          </StyledPagination>
        )}
      </StyledContainer>
    </StyledWrapper>
  );
};

이렇게 보면 짧아 보이지만 실제 모듈 import 부분과 스타일드 컴포넌트 정의 부분을 합치면 더 보기 싫어진다.

문제라고 생각되는 부분

이제 위 코드에서 문제라고 생각되는 부분을 봐보자. 제일 먼저 보이는 것은 useEffect에서 데이터를 패칭하는 부분이다. 이건 백엔드에서 넘겨주는 데이터가 페이지네이션으로 구현되는 부분이 있고 그냥 한 번에 전체 데이터를 넘겨주는 것이 있어서 분기 처리를 했는데, 그래도 내부에 중복되는 코드가 많이 발생한다는 문제가 있다.

(async () => {
  try {
    const res = await axios.get(
      `${process.env.REACT_APP_API_URL}${apiRef[tabInd]}&page=${
      curPage !== pageInfo.page ? curPage : 1
      }&size=${pageInfo.size}`
    );
    setGames(res.data.data);
    setPageInfo(res.data.pageInfo);
    // isTabIndChanged() && setCurPage(1);
    curPage === pageInfo.page && setCurPage(1);
  } catch (err) {
    console.error(err);
  }
})();

위 코드와 아래 코드를 비교해보자.

(async () => {
  try {
    const res = await axios.get(
      `${process.env.REACT_APP_API_URL}${apiRef[tabInd]}`
    );
    const newPageInfo = {
      ...pageInfo,
      page: curPage !== pageInfo.page ? curPage : 1,
      totalSize: res.data.data.length,
      totalPage: Math.ceil(res.data.data.length / 14)
    };

    const startInd = (newPageInfo.page - 1) * newPageInfo.size;
    const endInd = startInd + newPageInfo.size;
    setGames(res.data.data.slice(startInd, endInd));
    setPageInfo(newPageInfo);
    // isTabIndChanged() && setCurPage(1);
    curPage === pageInfo.page && setCurPage(1);
  } catch (err) {
    console.error(err);
  }
})();

api 요청 주소와 페이지네이션을 직접 구현하는 부분을 제외하면 동일하다는 것을 알 수 있다. 동일한 부분은 당연히 함수로 뺄 수 있다는 것이다.

또한 현재 인기, 신규, 생성, 팔로우 필터링을 하는 방식에도 문제가 있다.

const [tabInd, setTabInd] = useState<number>(0);

const apiRef = [
  '/api/games/?filter=POPULAR',
  '/api/games/?filter=NEW',
  `/api/members/${user.memberId}/creategame`,
  `/api/members/${user.memberId}/mygame`
];

위와 같은 방식으로 필터링 탭 상태를 인덱스로 관리하고, 그에 맞는 주소를 인덱스로 접근하는 데 이는 어떤 탭인지 명확하게 알 수 없고 각 탭마다 요청 주소가 어떻게 연결되는 지 알기 어렵다고 생각한다.

문제는 요청 주소가 각각 다르고 생성, 팔로우 요청 주소의 경우 user.memberId가 필요해 외부로 빼기가 어려워 보인다.

다음은 렌더링 부분이다.

return (
  <StyledWrapper>
    <StyledContainer>
      <FilterBar onClickHandler={onClickHandler} tabInd={tabInd} />
      <StyledCardContainer>
        {games.length === 0 && (
          <StyledEmptyItem>등록된 게임 채널이 없습니다.</StyledEmptyItem>
        )}
        {games.map((game) => (
          <HomeGameCard key={game.gameId} {...game} />
        ))}
      </StyledCardContainer>
      {games.length > 0 && (
        <StyledPagination>
          <Pagination
            activePage={curPage}
            itemsCountPerPage={pageInfo.size}
            totalItemsCount={pageInfo.totalSize}
            pageRangeDisplayed={5}
            prevPageText={'‹'}
            nextPageText={'›'}
            onChange={onPageClickHandler}
            />
        </StyledPagination>
      )}
    </StyledContainer>
  </StyledWrapper>
);

이 역시 컴포넌트의 의미 레벨이 맞지 않는다고 생각했다. 위 코드를 보면 필터바는 컴포넌트로 분리가 되었지만, 게임 카드들을 보여주는 부분은 분리되어 있지 않다. 또한 페이지네이션도 게임 카드를 보여주는 부분에 합쳐줄 수 있을 것 같다. 단, 이때의 문제점은 프롭스로 넘겨받을 상태가 많아진다는 것이다.

리팩토링

탭 상태 관리

먼저 탭의 상태를 인덱스가 아닌 글자를 가지도록 변경한다.

const [tabInd, setTabInd] = useState<string>('인기');

이후 API 주소를 외부로 추출한다.

// filterTabList.ts
export const getFilterPath = (memberId: number): { [key: string]: string } =>
  memberId === -1
    ? {
        인기: '/api/games/?filter=POPULAR',
        신규: '/api/games/?filter=NEW'
      }
    : {
        인기: '/api/games/?filter=POPULAR',
        신규: '/api/games/?filter=NEW',
        생성: `/api/members/${memberId}/creategame`,
        팔로우: `/api/members/${memberId}/mygame`
      };

주소가 memeberId를 필요로하기 때문에 함수로 만들었다. key를 인자로 받아 요청 주소를 리턴하게 만들지 않은 이유는 이 함수가 반환하는 객체의 키값 전체를 FilterBar 컴포넌트에서 사용하기 때문이다.

const FilterBar = ({
  onClickHandler,
  tabInd
}: {
  onClickHandler: (i: number) => () => void;
  tabInd: number;
}) => {
  const user = useSelector((s: RootState) => s.user);

  return (
    <StyledContainer>
      <StyledFilterTabContainer>
        <StyledItem onClick={onClickHandler(0)} selected={tabInd === 0}>
          인기
        </StyledItem>
        <StyledItem onClick={onClickHandler(1)} selected={tabInd === 1}>
          신규
        </StyledItem>
        {user.memberId !== -1 && (
          <>
            <StyledItem onClick={onClickHandler(2)} selected={tabInd === 2}>
              생성
            </StyledItem>
            <StyledItem onClick={onClickHandler(3)} selected={tabInd === 3}>
              팔로우
            </StyledItem>
          </>
        )}
      </StyledFilterTabContainer>
    </StyledContainer>
  );
};

이 컴포넌트도 조금 리팩토링 해줄 것이다.

const FilterBar = ({
  onClickHandler,
  tab
}: {
  onClickHandler: (i: string) => () => void;
  tab: string;
}) => {
  const user = useSelector((s: RootState) => s.user);
  const tabs = Object.keys(getFilterPath(user.memberId));

  return (
    <StyledContainer>
      <StyledFilterTabContainer>
        {tabs.map((tabName) => (
          <StyledItem
            key={tabName}
            onClick={onClickHandler(tabName)}
            selected={tab === tabName}
          >
            {tabName}
          </StyledItem>
        ))}
      </StyledFilterTabContainer>
    </StyledContainer>
  );
};

const apiPath = useMemo(() => getFilterPath(user.memberId), [user]);
(async () => {
  try {
    const res = await axios.get(
      `${process.env.REACT_APP_API_URL}${apiPath[tab]}&page=${
      curPage !== pageInfo.page ? curPage : 1
      }&size=${pageInfo.size}`
    );
    setGames(res.data.data);
    setPageInfo(res.data.pageInfo);
    // isTabIndChanged() && setCurPage(1);
    curPage === pageInfo.page && setCurPage(1);
  } catch (err) {
    console.error(err);
  }
})();

위와 같이 주소를 받는 부분이 변경되었다.

API 요청 부분

필터 탭의 상태에 따라 useEffect에서 데이터를 패칭하는 부분을 수정했다.

export const makeNewPageInfo = (
  data: GameType[],
  pageInfo: PageInfoType,
  curPage: number
): [number, number, PageInfoType] => {
  const newPageInfo = {
    ...pageInfo,
    page: curPage !== pageInfo.page ? curPage : 1,
    totalSize: data.length,
    totalPage: Math.ceil(data.length / 14)
  };

  const startInd = (newPageInfo.page - 1) * newPageInfo.size;
  const endInd = startInd + newPageInfo.size;
  return [startInd, endInd, newPageInfo];
};

우선 페이지네이션 기능을 직접 만들어야 하는 경우를 위한 함수를 추출했다.

const onSuccess = ({
  data
}: { data: { data: GameType[]; pageInfo?: PageInfoType }; }) => {
  if (data.pageInfo) {
    setGames(data.data);
    setPageInfo(data.pageInfo);
  } else {
    const [startInd, endInd, newPageInfo] = makeNewPageInfo(
      data.data,
      pageInfo,
      curPage
    );
    setGames(data.data.slice(startInd, endInd));
    setPageInfo(newPageInfo);
  }

  curPage === pageInfo.page && setCurPage(1);
};

이후 데이터 패칭 요청이 성공했을 때 실행할 함수를 분리했다. 여기서 넘어오는 데이터의 data.pageInfo가 있는지 여부에 따라 상태 변경 함수에 들어갈 값을 다르게 주었다.

useEffect(() => {
  let url = '';
  if (tab == '인기' || tab == '신규') {
    url = `${process.env.REACT_APP_API_URL}${apiPath[tab]}&page=${
    curPage !== pageInfo.page ? curPage : 1
  }&size=${pageInfo.size}`;
  } else {
    url = `${process.env.REACT_APP_API_URL}${apiPath[tab]}`;
  }

  getData(url, onSuccess, (error) => {
    console.error(error);
  });
}, [tab, curPage]);

마지막으로 useEffect의 내용을 다음과 같이 수정했다. getData 함수를 이용해서 데이터 패칭 부분을 숨겼고, 주소는 따로 만들어 넘겨주었다.

랜더링 부분

마지막으로 랜더링 부분을 리팩토링 했다.

// GaemCardList.tsx
const GameCardList = ({ games }: { games: GameType[] }) => {
  return (
    <StyledCardContainer>
      {games.length === 0 && (
        <StyledEmptyItem>등록된 게임 채널이 없습니다.</StyledEmptyItem>
      )}
      {games.map((game) => (
        <GameItem key={game.gameId} {...game} />
      ))}
    </StyledCardContainer>
  );
};

게임 카드를 랜더링 하는 부분을 컴포넌트로 분리했다.

// GameCardPagination.tsx
const GameCardPagination = ({
  curPage,
  gameLength,
  pageInfo,
  onPageClickHandler
}: {
  curPage: number;
  gameLength: number;
  pageInfo: PageInfoType;
  onPageClickHandler: (page: number) => void;
}) => {
  if (gameLength <= 0) return <></>;
  return (
    <StyledPagination>
      <Pagination
        activePage={curPage}
        itemsCountPerPage={pageInfo.size}
        totalItemsCount={pageInfo.totalSize}
        pageRangeDisplayed={5}
        prevPageText={'‹'}
        nextPageText={'›'}
        onChange={onPageClickHandler}
      />
    </StyledPagination>
  );
};

페이지네이션 부분도 분리했다. 이제 완성된 모습을 확인하면 아래와 같다.

return (
  <StyledWrapper>
    <StyledContainer>
      <FilterBar onClickHandler={onTabClickHandler} tab={tab} />
      <GameCardList games={games} />
      <GameCardPagination
        gameLength={games.length}
        curPage={curPage}
        pageInfo={pageInfo}
        onPageClickHandler={onPageClickHandler}
        />
    </StyledContainer>
  </StyledWrapper>
);

랜더링부분이 많이 줄었고 각 컴포넌트에 기능을 위임하여 분리하였다.