Skip to content

Refactor/network service abstraction#76

Merged
gomminjae merged 2 commits intodevelopfrom
refactor/network-service-abstraction
Mar 28, 2026
Merged

Refactor/network service abstraction#76
gomminjae merged 2 commits intodevelopfrom
refactor/network-service-abstraction

Conversation

@gomminjae
Copy link
Copy Markdown
Owner

No description provided.

gomminjae and others added 2 commits March 28, 2026 15:31
- NetworkService 프로토콜 도입, MoyaNetworkService로 Moya 캡슐화
- Feature Data 모듈 11개에서 import Moya 완전 제거
- AppDIContainer/CompositionRoot를 NetworkService 기반으로 전환
- BrandDetailView 이미지 비율 유지 (scaledToFill + overlay 패턴)
- KFImage 다운샘플링 크기 최적화 (표시 크기 × scale로 통일)
- BookmarkCard scaledToFill 누락 수정
- NewletterRow/NewsletterDetailRow 중복 onFailure 제거

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request implements a major architectural shift from horizontal layers to vertical slices by decomposing the monolithic Domain and Data modules into feature-specific sub-targets. It also introduces a NetworkService abstraction to decouple repositories from Moya and optimizes image downsampling across various views. Feedback was provided to re-add removed .onFailure handlers for KFImage in the Explore feature to ensure a consistent user experience when image loading fails.

Comment on lines 72 to 76
.placeholder {
// 로딩 중 표시
RoundedRectangle(cornerRadius: 10)
.fill(Color.gray.opacity(0.2))
.frame(width: 56, height: 56)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The .onFailure handler for KFImage was removed. This could lead to a poor user experience if an image fails to load, as there is no longer a custom failure placeholder. Please consider re-adding it to provide a fallback view for when the image fails to load.

Suggested change
.placeholder {
// 로딩 중 표시
RoundedRectangle(cornerRadius: 10)
.fill(Color.gray.opacity(0.2))
.frame(width: 56, height: 56)
}
.placeholder {
RoundedRectangle(cornerRadius: 10)
.fill(Color.gray.opacity(0.2))
.frame(width: 56, height: 56)
}
.onFailure { _ in
Image(systemName: "photo")
.font(.system(size: 24))
.foregroundColor(Color.gray.opacity(0.5))
.frame(width: 56, height: 56)
.background(Color.gray.opacity(0.1))
.clipShape(RoundedRectangle(cornerRadius: 10))
}

Comment on lines 28 to 32
.placeholder {
// 로딩 중 표시
RoundedRectangle(cornerRadius: 10)
.fill(Color.gray.opacity(0.2))
.frame(width: 56, height: 56)
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The .onFailure handler for KFImage was removed. This could lead to a poor user experience if an image fails to load, as there is no longer a custom failure placeholder. Please consider re-adding it to provide a fallback view for when the image fails to load.

Suggested change
.placeholder {
// 로딩 중 표시
RoundedRectangle(cornerRadius: 10)
.fill(Color.gray.opacity(0.2))
.frame(width: 56, height: 56)
}
.placeholder {
RoundedRectangle(cornerRadius: 10)
.fill(Color.gray.opacity(0.2))
.frame(width: 56, height: 56)
}
.onFailure { _ in
Image(systemName: "photo")
.font(.system(size: 24))
.foregroundColor(Color.gray.opacity(0.5))
.frame(width: 56, height: 56)
.background(Color.gray.opacity(0.1))
.clipShape(RoundedRectangle(cornerRadius: 10))
}

@gomminjae gomminjae merged commit b2e5734 into develop Mar 28, 2026
2 checks passed
@gomminjae gomminjae deleted the refactor/network-service-abstraction branch March 28, 2026 13:01
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.

1 participant