Conversation
Home Feature를 완전한 수직 슬라이스로 전환: - HomeDomain: HomeArticle, HomeNewsletter 등 컨텍스트별 모델 + Repository/UseCase 프로토콜 - HomeData: DTO, Repository 구현체 (MoyaProvider 직접 사용) - Home Sources: import Domain → import HomeDomain, 공유 타입 → Home 전용 타입 - CompositionRoot: HomeArticleRepositoryImpl/HomeNewsletterRepositoryImpl 직접 생성 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1. VM 캐시 미초기화: resetCache() 구현 + Notification으로 로그아웃/탈퇴 시 호출
2. 탈퇴 실패→온보딩 이동: onChange(of: withdrawSuccess)로 성공 시에만 화면 전환
3. EditNicknameView .constant("") 버그: initialNickname 파라미터로 변경, VM/UserInfoStore에서 실제 닉네임 전달
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces the Home feature module, including its domain models, data repositories, and use cases, while also implementing a caching mechanism for the MyPage view model to handle user data resets. The review feedback highlights a consistency issue in the HomeNewsletter model initializer, suggests improving the robustness of the custom JSON decoding logic in HomeArticleDTO, and recommends using type-safe static constants for Notification names instead of string literals.
| public init(id: Int, brandName: String, imageUrl: String, publicationCycle: String) { | ||
| self.id = id | ||
| self.brandName = brandName | ||
| self.imageUrl = imageUrl | ||
| self.publicationCycle = publicationCycle | ||
| } |
There was a problem hiding this comment.
The id and publicationCycle properties are declared as optionals, but the initializer requires non-optional values. This is inconsistent and forces callers (like DTO mappings) to provide default values (e.g., 0 or ""), which can be misleading and hide data issues. The initializer should be updated to accept optionals to match the property declarations.
| public init(id: Int, brandName: String, imageUrl: String, publicationCycle: String) { | |
| self.id = id | |
| self.brandName = brandName | |
| self.imageUrl = imageUrl | |
| self.publicationCycle = publicationCycle | |
| } | |
| public init(id: Int?, brandName: String, imageUrl: String, publicationCycle: String?) { | |
| self.id = id | |
| self.brandName = brandName | |
| self.imageUrl = imageUrl | |
| self.publicationCycle = publicationCycle | |
| } |
| public init(from decoder: Decoder) throws { | ||
| if let today = try? decoder.container(keyedBy: TodayKeys.self), today.contains(.id) { | ||
| self.id = try today.decode(Int.self, forKey: .id) | ||
| self.articleTitle = try today.decode(String.self, forKey: .title) | ||
| self.status = try today.decode(String.self, forKey: .status) | ||
| self.publishDate = try today.decodeIfPresent(Int.self, forKey: .publishDate) | ||
|
|
||
| if today.contains(.newsletter) { | ||
| let n = try today.nestedContainer(keyedBy: NewsletterKeys.self, forKey: .newsletter) | ||
| self.brandName = (try? n.decode(String.self, forKey: .brandName)) ?? "" | ||
| self.imageUrl = try? n.decode(String.self, forKey: .imageUrl) | ||
| } else { | ||
| self.brandName = "" | ||
| self.imageUrl = nil | ||
| } | ||
| } else { | ||
| let flat = try decoder.container(keyedBy: FlatKeys.self) | ||
| self.id = try flat.decode(Int.self, forKey: .articleId) | ||
| self.articleTitle = try flat.decode(String.self, forKey: .articleTitle) | ||
| self.brandName = try flat.decode(String.self, forKey: .brandName) | ||
| self.imageUrl = try flat.decodeIfPresent(String.self, forKey: .imageUrl) | ||
| self.status = try flat.decode(String.self, forKey: .status) | ||
| self.publishDate = nil | ||
| } | ||
| } |
There was a problem hiding this comment.
The custom decoding logic relies on try? which can suppress important decoding errors, making debugging difficult. This is especially true for the top-level check if let today = try? decoder.container(keyedBy: TodayKeys.self). A more robust approach would be to use do-catch blocks to handle different JSON structures explicitly. This would improve error diagnostics and make the code easier to understand and maintain.
For example, you could attempt to decode using TodayKeys and, upon failure, fall back to decoding with FlatKeys, logging the error from the first attempt for debugging purposes.
|
|
||
|
|
||
| // 캐시된 뷰모델 초기화 | ||
| NotificationCenter.default.post(name: .init("ResetMypageCache"), object: nil) |
There was a problem hiding this comment.
Using string literals for Notification names is error-prone. It's a best practice to define a static constant for the notification name in an extension of Notification.Name. This provides type safety and avoids typos.
For example:
extension Notification.Name {
static let resetMypageCache = Notification.Name("ResetMypageCache")
}Then you can use NotificationCenter.default.post(name: .resetMypageCache, object: nil). This should be applied consistently where this notification is posted or observed.
- DerivedData 캐시 추가로 증분 빌드 활성화 (변경된 모듈만 재컴파일) - SwiftLint 바이너리 캐시로 매 실행 시 다운로드 제거 - xcodebuild에 -derivedDataPath 명시 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
No description provided.