Skip to content

Conversation

@ohinhyuk
Copy link
Contributor

@ohinhyuk ohinhyuk commented Oct 28, 2025

작업 사항

기존에 서버에서 가져온 데이터 이름을 대부분 data (default 값)으로 사용함.
이름이 구체적이지 않아 코드 상에서 어떤 데이터인지 파악이 어려움.
데이터의 주제에 맞는 데이터 명으로 구체화하여 코드 가독성을 증가시킨다.

추가로 isLoading, refetch도 해당 주제에 맞게 구체화 함.

작업 단계

  1. e2e 테스트 코드 작성
    리팩터링 할 때, 기존 서버 호출이 잘 유지 되는 지 확인할 수 있어야 함.
    따라서 페이지 접근 후 화면이 잘 출력되는 지 확인하는 테스트 코드들을 작성함
  • 테스트 위치: e2e-test/pages/*/page.spec.ts
  1. 리팩터링 진행
    서버에서 가져온 data, isLoading, refetch 모두 주제에 맞는 이름으로 구체화

Example

AS-IS

const { data, isLoading } = useQuery(['AllTeamRanks'], getAllTeamRanks, {
      cacheTime: 10 * 60 * 1000,
      refetchOnWindowFocus: false,
   });
const topThreeTeams = useMemo(() => {
      if (!data) return [];  // 어떤 데이터 인지 알기 위해 스크롤 올려서 확인 해야 함.
      return data.teams.slice(0, 3).map((team) => ({
         ...team,
         members: team.members.map((name) => maskName(name)),
      }));
   }, [data]);
if (isLoading) {  // 어떤 로딩인지 알기 위해 스크롤 올려서 확인 해야 함.
      return <WaveLoading />;
}

TO-BE

const { data: teamRanksData, isLoading: isTeamRanksLoading } = useQuery(['AllTeamRanks'], getAllTeamRanks, {
      cacheTime: 10 * 60 * 1000,
      refetchOnWindowFocus: false,
   });
const topThreeTeams = useMemo(() => {
      if (!teamRanksData) return [];  // 데이터 명 구체화
      return teamRanksData.teams.slice(0, 3).map((team) => ({
         ...team,
         members: team.members.map((name) => maskName(name)),
      }));
}, [teamRanksData]);
if (isTeamRanksLoading) {  // 로딩 명 구체화
      return <WaveLoading />;
}

관련 이슈

close #161

@ohinhyuk ohinhyuk requested a review from minzziPark October 28, 2025 15:58
@coderabbitai
Copy link

coderabbitai bot commented Oct 28, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

};

export const readReportDetail = async (reportId: number): Promise<Report> => {
export const readOneReport = async (reportId: number): Promise<Report> => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One을 강조하신 이유가 있으실까요? 있다면 상관없지만 없다면 readRepord라고 해도 괜찮을 것 같습니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

좋은 의견입니다!

Comment on lines 16 to 19
data: studyEnrolleesData,
refetch: studyEnrolleesRefetch,
isLoading: isStudyEnrolleesLoading,
} = useQuery(['readEnrollees'], readEnrollees, {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

studyEnrollees와 그냥 enrollee가 함께 사용되고 있는데, 도메인이 스터디이기 때문에 enrollee로 통일하는 것은 어떻게 생각하시나요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

studyEnrollee가 더 구체적인 것 같고, 현재 enrollee 보다 studyEnrollee가 더 많이 사용되고 있습니다.
studyEnrollee로 통일하는 걸로 하겠습니다!

export default function StudyEnrollmentPage() {
const { data: myStudyEnrollment, isLoading } = useQuery('getMyStudyEnrollment', getMyGroup);
export default function StudyApplicationPage() {
const { data: myStudyEnrollment, isLoading: isMyStudyEnrollmentLoading } = useQuery(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const { data: myStudyEnrollment, isLoading: isMyStudyEnrollmentLoading } = useQuery(
const { data: myStudyEnrollmentData, isLoading: isMyStudyEnrollmentLoading } = useQuery(

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영하였습니다.

const [debouncedSearchTerm] = useDebounce(searchTerm, 250);

const { data: searchResults } = useQuery(['searchCourse', debouncedSearchTerm], () =>
const { data: searchCourseResults } = useQuery(['searchCourse', debouncedSearchTerm], () =>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const { data: searchCourseResults } = useQuery(['searchCourse', debouncedSearchTerm], () =>
const { data: searchCourseResultsData } = useQuery(['searchCourse', debouncedSearchTerm], () =>

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

꼼꼼하게 봐주셨군요. 반영하였습니다.

- 변수명 통일 및 구체화
@minzziPark
Copy link
Collaborator

@ohinhyuk 님, 고생 많으셨습니다! 자잘한 코드 수정이긴 하지만 전체적으로 통일성은 높아진 것 같습니다!ㅎㅎ

@ohinhyuk ohinhyuk merged commit 718b6a2 into dev-refactoring Nov 6, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants