Skip to content

fix: FilteredAuthors의 3중 map 구조 개선#819

Merged
yuminnnnni merged 1 commit into
githru:mainfrom
yuminnnnni:fix/816-refactor-filtered-authors
Aug 9, 2025
Merged

fix: FilteredAuthors의 3중 map 구조 개선#819
yuminnnnni merged 1 commit into
githru:mainfrom
yuminnnnni:fix/816-refactor-filtered-authors

Conversation

@yuminnnnni

Copy link
Copy Markdown
Member

Related issue

#816

Result

  • FilteredAuthors 컴포넌트 내 3중 map 구조를 flatMap과 Set을 활용하여 중복 제거와 배열 평탄화를 적용함
  • 코드 가독성이 향상됨

Work list

  • selectedClusters에서 저자 배열들을 flatMap과 flat으로 1차원 배열로 변환
  • Set을 사용해 중복 저자명을 제거
  • uniqueAuthorNames 배열을 기반으로 Author 컴포넌트 map 렌더링 방식 적용
  • 기존 3중 map 구조 완전 제거 및 리팩토링 완료

Discussion

  • 저자 이름 중복 문제(동명이인 등)는 기존과 동일하게 이름을 기준으로만 처리되고 있습니다.
  • 따라서 향후 저자 식별에 고유 ID를 활용하는 방향을 검토해보면 좋을 것 같습니다!

@yuminnnnni yuminnnnni self-assigned this Aug 4, 2025
@yuminnnnni
yuminnnnni requested a review from a team as a code owner August 4, 2025 10:59
@yuminnnnni yuminnnnni added the fix label Aug 4, 2025
@ytaek

ytaek commented Aug 4, 2025

Copy link
Copy Markdown
Contributor

Discussion

  • 저자 이름 중복 문제(동명이인 등)는 기존과 동일하게 이름을 기준으로만 처리되고 있습니다.
  • 따라서 향후 저자 식별에 고유 ID를 활용하는 방향을 검토해보면 좋을 것 같습니다!

해당 건은
#803, #773, #523 와도 연관이 있을 것 같습니다!

@ytaek ytaek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Refactoring은 말 그대로 기능은 그대로 인데 구조를 바꾸는 것이고,
이를 가장 안전하게 작업할 수 있는 방법은 바로 Test 라고 생각합니당!
(red-green-refactor: https://velog.io/@harib0/TDD%EC%9D%98-%ED%95%B5%EC%8B%AC-%EC%9B%90%EC%B9%99-Red-Green-Refactor)

해당 component나 로직을 위한 Test를 덧붙이는 task를 담번에 수행하시면 어떨까요?
(시작하기 전에 이슈로 만들고, 할당한 뒤 해주세요!!!)

코드는 넘 깔끔해져서 좋습니다!! LGGGGTM!

Comment on lines +14 to +16
const uniqueAuthorNames = Array.from(
new Set(selectedClusters.flatMap((cluster) => cluster.summary.authorNames.flat()))
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍👍👍👍👍 완전 깔끔해졌네요!!

@SingTheCode SingTheCode left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

가독성 측면에서 너무 좋아졌네요! 고생하셨습니다ㅎㅎ
개인적으로 저는 코드 수정할 때 저를 못 믿는 편이라 기존 요구사항에 대한 테스트 코드를 꼭 추가하고 테스트가 통과하는 것을 확인한다음 리팩토링을 하는 편이에요! 다음 작업에 테스트코드도 포함되면 너무 좋을 것 같습니다ㅎㅎ

Comment on lines +23 to +29
uniqueAuthorNames.map((authorName) => (
<Author
key={authorName}
name={authorName}
src={authSrcMap[authorName]}
/>
))}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

너무 깔끔해졌네요ㅎㅎ 어떤 의미인지 한눈에 파악이 되어서 좋습니다ㅎㅎ

@yuminnnnni

Copy link
Copy Markdown
Member Author

네엡 감사합니다!
이번엔 로컬에서만 돌려보고 PR에는 포함하지 못했지만, 다음 작업 때 해당 컴포넌트/로직에 대한 테스트 코드도 꼭 추가하겠습니다 💪

@yuminnnnni
yuminnnnni merged commit 8913327 into githru:main Aug 9, 2025
2 checks passed
@ytaek

ytaek commented Aug 11, 2025

Copy link
Copy Markdown
Contributor

해당 건은 #803, #773, #523 와도 연관이 있을 것 같습니다!

@yuminnnnni 님, 혹시 위 이슈들 중에 해결된 부분이 있다면 해당 이슈들도 close 해주시면 좋을 것 같습니다!!

@yuminnnnni

Copy link
Copy Markdown
Member Author

@ytaek 앗 제가 해당 이슈들은 이 PR에서 해결하지 못해서 일단 이대로 남겨두겠습니다 😂..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants