[B2WIN] 2차 코드리뷰

Heera1·2023년 6월 9일
0

[멘토링] B2WIN

목록 보기
2/2

1. type 관리는 어떻게 해야하는지?

타입을 설정하는 interface를 모두 한 파일로 모아서 util 폴더에 typeCollection 으로 모아두었는데 멘토님께서 보시고 아래와 같이 조언해주셨다.

  • 한 파일로 묶는 건 좋지만 다른 곳에서 사용하지 않는 type이라면 사용하는 폴더 최상단에 그냥 적는 게 관리하기 편할 것 같다. 다른 곳에서 사용되는 type이라면 묶어두는 것도 좋다. (좀 사람 by 사람의 문제인 거 같다고 말씀하시기도 했다.)

  • api 응답 type과 view의 props type은 분리하자. (모아두는 파일을 분리하는 것도 포함되고, 아래와 같이 propsType을 apiRes 에 가져와 쓰는 것도 포함된다. 둘을 분리하지 않는 경우라면 같이 써도 상관은 없지만 그런 경우는 정말 드물다.) (ex: propsType을 후에 절대 수정하지 않는 경우라던가)

export interface apiRes{
	data: propsType [];
	ex1: string;
	ex2: number;
	...
}
    
export interface propsType{
	props1: ...
}

2. array를 사용했는데 이게 맞는지 모르겠다.

category 같은 걸 만들 때 array를 사용해서 만들었는데... 솔직히 썩 좋은 방법은 아니라는 생각이 들어서 멘토님께 여쭤봤다.

  • enum을 사용해 보자.
  • 로그인의 경우 아래와 같이 코드를 작성했었는데 멘토님께서 다른 방법을 제시해주셨다. 덕분에 인덱스 시그니처라는 개념을 새롭게 알게되어 적용해보았다. (참고1, 참고2)
//before //login.tsx
const signin = () => {
    const user = userArr.find(
      (user) => user.email === userId && user.password === password
    );

    if (user === undefined) {
      alert("찾을 수 없는 사용자입니다.");
      return;
    }
    alert("로그인 완료!");
    setIsLogin(true);
    navigator("/");
    localStorage.setItem(
      "accessToken",
      `${process.env.REACT_APP_ACCESS_TOKEN}`
    );
    return user;
  };

//arrayCollection.tsx
export const userArr= [
  { email: "codestate1", password: "12345678" },
  { email: "codestate2", password: "87654321" },
];


//after //login.tsx
interface loginType {
  [key: string]: string;
}

  const signIn = () => {
    const hash: loginType = {
      codestate1: "12345678",
      codestate2: "87654321",
    };
    const user = hash[userId] === password ? true : false;

    if (!user) {
      console.log(`userId: ${hash[userId]} // password: ${password}`);
      alert("사용자를 찾을 수 없습니다.");
      return;
    } else {
      console.log(`userId: ${hash[userId]} // password: ${password}`);
      alert("로그인 완료!");
      setIsLogin(true);
      navigator("/");
      localStorage.setItem(
        "accessToken",
        `${process.env.REACT_APP_ACCESS_TOKEN}`
      );
    }
    return user;
  };


//arrayCollection.tsx
... 코드 지움

전에 작성했던 코드는 사용자가 입력한 userId와 password를 useState로 저장하고, userArray와 같은 지 비교하는 코드였다.

수정한 코드는 hash의 사용자가 입력한 userId가 있다면, 사용자가 입력한 password와 해당하는 value값이 같을 때 true, 틀리면 false를 리턴한다. 사용자가 입력한 userId가 hash에 없다면 undefined를 리턴한다.

3. 코드가 길어지면 보기 나쁜 코드?

  • 코드가 길다는 것에 너무 신경쓰지 말자. 코드가 길다고 나쁜 코드이란 법도 없고, 보기 힘든 코드라는 법도 없다. 코드의 길이보단 다른 기준을 먼저 신경쓰자.
  • 예를들면, 하나의 함수가 너무 많은 책임을 가지고 있진 않는지? 라던가. 관심사 분리가 가장 중요하다.

4. mapping 함수를 사용해 보자.

  • enum value를 받아와서 내가 원하는 string으로 return 할 수 있다. 반대로 reverse mapping도 가능하다. (참고)

5. 함수의 파라미터의 순서에 의존하게 하지 말자.

  • apiCollection.tsx 파일을 만들고 거기에 api 호출 함수들을 모아두고 있다. 뉴스 기사를 가져올 때 파라미터로 보낼 값이 많았는데 작성한 코드는 함수의 파라미터의 순서에 의존하고 있었다.
  • params의 타입을 정리해주고, api 호출 함수에 params로 정리한 것을 타입으로 지정해줬다. 그리고 api를 호출하는 페이지 news.tsx에서 객체로 params를 보낸다. (객체의 value와 key가 같으면 중복하여 작성하지 않고 생략할 수 있는 것을 이용)
//before //apiCollection.tsx
export const getNewsDataAPI = async (
  q: string,
  date: string,
  country: string,
  page_size: number,
  offset: number,
  sort: string,
  topic: string,
  signal: any
) => {
  try {
    const res = await axios.get(
      `https://api.newscatcherapi.com/v2/search?q=${q}&from=${date}&countries=${country}&page_size=${page_size}&page=${offset}&sort_by=${sort}&topic=${topic}`,
      {
        headers: {
          "x-api-key": `${process.env.REACT_APP_API_KEY}`,
        },
        signal,
      }
    );
    return res;
  } catch (err: unknown) {
    return console.error(err);
  }
};

//after //apiCollection.tsx
type GetNewsDataApiParams = {
  keyword: string;
  date: string;
  country: string;
  pageSize: number;
  offset: number;
  sort: string;
  topic: string;
  signal: any;
};

export const getNewsDataAPI = async ({
  keyword,
  date,
  country,
  pageSize,
  offset,
  sort,
  topic,
  signal,
}: GetNewsDataApiParams) => {
  try {
    const res = await axios.get(
      `https://api.newscatcherapi.com/v2/search?q=${keyword}&from=${date}&countries=${country}&page_size=${pageSize}&page=${offset}&sort_by=${sort}&topic=${topic}`,
      {
        headers: {
          "x-api-key": `${process.env.REACT_APP_API_KEY}`,
        },
        signal,
      }
    );
    return res;
  } catch (err: unknown) {
    return console.error(err);
  }
};

//news.tsx
const getData = async () => {
        try {
          const res = await getNewsDataAPI({
            keyword,
            date,
            country,
            pageSize,
            offset,
            sort,
            topic,
            signal,
          });
          if (signal.aborted) return;
          const resData = (res as AxiosResponse<any, any>).data.articles;
          const resInfo = (res as AxiosResponse<any, any>).data.total_datas;
          setDataInfo(resInfo);
          setNewsData(resData);
          setIsLoading(true);
        } catch (err: unknown) {
          if (signal.aborted) return;
          console.error(err);
          setIsLoading(true);
        }
      };

6. Pagenation UI표현 코드와 상태관리 하는 것을 분리해보자.

  • 나는 멘토님의 말씀을 pagenation 코드에서 사용하는 useState 부분을 zustand를 사용해 관리해보는 걸로 이해했는데 이게 맞는지는 모르겠다.
  • 일단 내가 만든 pagenation 코드는 아래와 같다.
// before
import { useState } from "react";
interface PaginationProps {
  totalPage: number;
  curPage: number;
  setCurPage: (curPage: number) => void;
  pageCount: number;
}

export default function Pagenation({
  totalPage,
  curPage,
  setCurPage,
  pageCount,
}: PaginationProps) {
  const [pageGroup, setPageGroup] = useState(Math.ceil(curPage / pageCount));
  let lastPage = pageGroup * pageCount;
  if (lastPage > totalPage) {
    lastPage = totalPage;
  }

  let firstPage = lastPage - (pageCount - 1);
  if (pageCount > lastPage) {
    firstPage = 1;
  }

  const pagination = () => {
    let arr = [];
    for (let i = firstPage; i <= lastPage; i++) {
      arr.push(
        <button
          key={i}
          className={`pagination-but ${i === curPage ? "bg-gray-500" : ""}`}
          onClick={() => setCurPage(i)}
        >
          {i}
        </button>
      );
    }
    return arr;
  };

  const minusPage = () => {
    if (curPage === firstPage) {
      setPageGroup(pageGroup - 1);
      setCurPage((pageGroup - 1) * 10);
      return;
    } else if (curPage !== firstPage) {
      setCurPage(curPage - 1);
      return;
    }
  };

  const plusPage = () => {
    if (curPage === lastPage) {
      setPageGroup(pageGroup + 1);
      setCurPage(curPage + 1);
      return;
    }
    if (curPage !== lastPage) {
      setCurPage(curPage + 1);
      return;
    }
  };

  return (
    <div className="flex">
      <button
        className="pagination-but"
        onClick={minusPage}
        disabled={curPage === 1}
      >
        &lt;
      </button>
      {pagination()}
      <p className="px-2">...</p>
      <button
        className={`pagination-but font-bold ${
          totalPage === curPage ? "bg-gray-500" : ""
        }`}
        onClick={() => setCurPage(totalPage)}
      >
        {totalPage}
      </button>
      <button
        className="pagination-but"
        onClick={plusPage}
        disabled={curPage === lastPage && curPage === totalPage}
      >
        &gt;
      </button>
    </div>
  );
}

//사용하는 곳 //news.tsx
const [dataInfo, setDataInfo] = useState(1);//총 몇개의 페이지가 있는지 
const [offset, setOffset] = useState(1);//현재 선택하고 있는 페이지

...생략
   <Pagenation
      totalPage={dataInfo}
      curPage={offset}
      setCurPage={setOffset}
      pageCount={10}
   />
 
 //after //news.tsx
    const { dataInfo, setDataInfo, offset, setOffset } = usePagenation();

//PagenationState.tsx
import { create } from "zustand";

interface PagenationType {
  dataInfo: number;
  setDataInfo: (dataInfo: number) => void;
  offset: number;
  setOffset: (offset: number) => void;
}

export const usePagenation = create<PagenationType>((set) => ({
  dataInfo: 1,
  setDataInfo: (input) => set(() => ({ dataInfo: input })),
  offset: 1,
  setOffset: (input: number) => set(() => ({ offset: input })),
}));

profile
웹 개발자

0개의 댓글