-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[#17] CollectionView 및 ViewModel 리팩토링 #18
base: main
Are you sure you want to change the base?
Conversation
letskooo
commented
Jan 2, 2025
•
edited
Loading
edited
- 기존 CollectionView를 CompositionalLayout과 Diffable Data Source를 도입하여 리팩토링을 진행했습니다.
- 뷰의 계층에 맞춰 뷰모델을 계층화하였습니다.
- 뷰에서 쓰이는 뷰모델을 각각 정의하고 최상위 뷰모델에서 이를 메소드로 반환하는 방식으로 재구조화하였습니다.
public var newPostData: [Post]? | ||
public var weeklyTopPostData: [Post]? | ||
public var customizedPostData: [Post]? | ||
public var editorRecommendationPostData: [Post]? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 저는 HomeSection의 케이스가 늘어날 때마다 HomePostData의 프로퍼티가 늘어나는 것이 비효율적일 수 있다는 의미로 말씀해주신 것으로 이해를 했습니다. HomeSection 열거형 타입의 경우 앱 내 로직에서 사용되는 개념이고, HomePostData의 경우는 외부 API로부터 데이터를 매핑하는 DTO입니다. 현재 구조에서 HomePostData의 프로퍼티를 고정적으로 두지 않고 HomeSection의 케이스에 따라서 동적으로 형성되도록 한다면 기존 HomePostData는 쓰이지 않게 될 것 같습니다.
하지만 현재 구조에서 enum으로 동적으로 데이터를 매핑하는 것보다는 DTO를 따로 두는 것이 더 효율적일 수 있다는 생각을 했습니다. 제 의견에 대해서 멘토님의 피드백이 궁금합니다!
아니면 혹시 연관값으로 현재 문제를 해결할 수 있는 방법을 제가 캐치하지 못한 것인지도 궁금합니다.
// post는 제네릭 타입으로 주어진 HomePostViewTemplate의 인스턴스를 의미 | ||
collectionView, indexPath, post in | ||
/// 섹션의 순서를 가져온 배열. 섹션의 순서는 사용자마자 다를 수 있음. | ||
let section = HomeSection.allCases[indexPath.section] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
말씀해주신 부분에서 가변적으로 동작하지 않는 오류를 발견했습니다!
HomeSection.allCases로 고정하는 것이 아니라 homeCVVM의 sectionIndex로 section을 가져오도록 수정하겠습니다!
func setHomeData(homePostData: HomePostViewTemplateGroup) { | ||
if let sectionIndex = homePostData.sectionIndex { | ||
self.sectionIndex = sectionIndex | ||
for sectioin in sectionIndex { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 sectionIndex보다는 sectionOrder라는 변수명으로 전체적으로 적용하겠습니다!
withReuseIdentifier: HomeContentsSectionHeaderView.identifier, | ||
for: indexPath) as! HomeContentsSectionHeaderView | ||
let section = HomeSection.allCases[indexPath.section] | ||
switch section { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
위 코멘트에도 언급하였듯이 HomeSectionTemplate이라는 새로운 뷰전용 enum을 만들고 연관값을 적용시켜 코드를 개선시켜보았습니다!
Quality Gate failedFailed conditions |