Design/#18 home#24
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughHome 관련 도메인·유스케이스·리포지토리·서비스·네트워크 추가, Home UI를 컬렉션뷰·디프러블 데이터소스로 전면 재구성, Main/PopularTravel/Search/Setting RIBs 모듈 추가 및 루트 라우팅을 TabBar→Main으로 전환, 여러 DSKit 컴포넌트·에셋 및 유틸 확장 추가/정리. Changes
Sequence Diagram(s)sequenceDiagram
participant VC as HomeViewController
participant I as HomeInteractor
participant UC as HomeUsecase
participant Repo as HomeRepository
participant Svc as HomeService
participant Net as Networks
rect rgba(200,200,255,0.5)
VC->>I: viewDidLoad / reloadBtnTapped
I->>UC: fetchMyTripInfo(), fetchCategoryList(), fetchPopularTripList(), fetchRecommendTripList()
UC->>Repo: repository.fetch...
Repo->>Svc: homeService.get...
Svc->>Net: Moya request (HomeAPI)
Net-->>Svc: DTO 응답
Svc-->>Repo: DTO 전달
Repo-->>UC: 도메인 모델 반환
UC-->>I: 결과 전달
I->>VC: update(with: [HomeSectionModel])
VC->>VC: applySnapshot() -> UI 렌더링
end
sequenceDiagram
participant RootI as RootInteractor
participant RootR as RootRouter
participant MainB as MainBuilder
participant MainR as MainRouter
participant Tab as TabBarRouter
rect rgba(200,255,200,0.5)
RootI->>RootR: attachMain()
RootR->>MainB: build()
MainB-->>RootR: MainRouter
RootR->>MainR: attach main as root
MainR->>Tab: attachTabBar() / attach children (Follow, PopularTravel, Search, Setting)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@Projects/Data/Sources/Adapter/TokenProviderAdapter.swift`:
- Around line 20-23: The accessToken() function currently returns a hardcoded
JWT string; restore the original token retrieval by calling the token repository
(uncomment or reimplement tokenRepository.get(.accessToken)) in
TokenProviderAdapter.accessToken(), remove the embedded token from source, and
ensure fallback/optional handling remains (return String?); for tests, switch to
reading the token from an environment variable or a gitignored config and update
any callers of accessToken() if needed.
In `@Projects/Data/Sources/Transform/TripResponse`+.swift:
- Around line 14-29: In TripResponse.toDomain() the countryCode is incorrectly
set to the country name (countryCode: $0.country) which must be an ISO 3166-1
alpha-2 code; update the mapping in TripResponse.toDomain() to convert the
incoming country name to its 2-letter ISO code (or use the DTO's ISO field if
available) by calling the conversion helper (e.g., a new/existing
countryNameToISO or similar extension) before assigning to countryCode so
downstream extensions like String.toKoreanCountryName()/toFlag() receive the
expected ISO code.
In `@Projects/Domain/Sources/Model/Home/TripInfo.swift`:
- Around line 21-40: The initializer for TripInfo declares countryCode but never
stores it, causing the value to be dropped and breaking callers like
RecommendInfoCell that expect a 2-letter ISO code from
model.country.toFlag()/toKoreanCountryName(); either add a stored property
(e.g., let countryCode: String) to the TripInfo struct/class and assign
self.countryCode = countryCode in the public init (update any codable/Equatable
implementations accordingly), or if country already holds the 2-letter code
remove the redundant countryCode parameter and update callers to use country
consistently—verify RecommendInfoCell usage (model.country.toFlag()) after you
make the change.
In `@Projects/Features/HomeFeature/Sources/HomeInteractor.swift`:
- Around line 107-142: The fetchHomeData Task can race with subsequent calls
because the Task's completion always sets fetchDataTask = nil; fix by capturing
the current Task reference at the start of the Task closure (e.g. let
currentTask = fetchDataTask) and only clear fetchDataTask if it still equals
that captured reference (check identity, e.g. if fetchDataTask === currentTask {
fetchDataTask = nil }); apply this check in both the success and catch paths so
rapid reloadBtnTapped() calls don't let an earlier Task overwrite the active
fetchDataTask; see fetchHomeData, fetchDataTask and reloadBtnTapped for where to
apply the change (same pattern as PopularTravelInteractor).
In `@Projects/Features/HomeFeature/Sources/HomeViewController.swift`:
- Around line 277-280: The update(with:) method in HomeViewController must
ensure UI updates run on the main thread: wrap the call to applySnapshot(with:)
inside a DispatchQueue.main.async to prevent crashes when the interactor emits
on a background thread; mirror the pattern used for setLoading and showErrorView
(which are already main-thread protected) and update PopularTravelViewController
if present to match this behavior.
In
`@Projects/Features/HomeFeature/Sources/Views/CollectionView/SectionKind/HomeSectionKind.swift`:
- Around line 17-25: The headerTitle getter in HomeSectionKind currently
hardcodes a user name for the recommendedTrip case; update it so the string uses
a dynamic user name (e.g., inject or fetch the current user's displayName)
instead of the literal "나혜주님" by changing the recommendedTrip branch in var
headerTitle to format with the runtime user name (or call a helper like
UserManager.current.name) and ensure localization/escaping as needed; if you
cannot wire the user model right now, replace the literal with a clear TODO
comment in HomeSectionKind::headerTitle indicating the value must be replaced
with the logged-in user's name and reference the recommendedTrip case.
In `@Projects/Features/MainFeature/Sources/MainViewController.swift`:
- Around line 44-46: The method popRootViewController's name promises popping to
the root but its body calls popViewController (one level); decide intent and
fix: if you want to go to the root, change the implementation in
MainViewController to call popToRootViewController(animated:) instead of
popViewController(animated:); if the intended behavior is only to pop one level,
rename popRootViewController to popViewController(animated:) (and update any
callers) so the method name matches the implementation.
In
`@Projects/Features/PopularTravelFeature/Sources/PopularTravelInteractor.swift`:
- Around line 97-128: The current fetchData() can race when multiple calls
cancel and replace fetchDataTask because the Task body sets fetchDataTask = nil;
remove that in-task clearing to avoid losing the active task reference. Instead,
create the Task into a local constant, assign fetchDataTask = task before
awaiting, and do not set fetchDataTask = nil inside the Task body; let
willResignActive() (or the single centralized cleanup) be the sole place that
cancels/clears fetchDataTask to ensure the active Task isn't accidentally
dropped (refer to fetchData(), fetchDataTask, and willResignActive()).
In
`@Projects/Features/PopularTravelFeature/Sources/PopularTravelViewController.swift`:
- Around line 135-145: The navigationBar.trailingButtonDidTap event is being
subscribed to twice, causing listener?.searchBtnTapped() to fire twice; remove
the duplicate subscription block so there is only one subscribe(with: self) {
owner, _ in owner.listener?.searchBtnTapped() } call and its corresponding
.disposed(by: disposeBag) for navigationBar.trailingButtonDidTap (keep reference
to navigationBar, trailingButtonDidTap, listener?.searchBtnTapped(), and
disposeBag when locating the duplicate).
- Around line 195-198: The update(with:) implementation in
PopularTravelViewController (conforming to PopularTravelPresentable) calls
applySnapshot from potentially a background Task and must be dispatched to the
main thread; modify the update(with:) method to wrap the applySnapshot(with:)
call inside DispatchQueue.main.async so NSDiffableDataSourceSnapshot updates
always run on the main thread (consistent with how setLoading and showErrorView
are already protected).
In
`@Projects/Features/PopularTravelFeature/Sources/Views/CollectionView/CompositionalLayout/PopularTravelCompositionalLayout.swift`:
- Around line 87-103: The emptyLayout() function is creating an
NSCollectionLayoutGroup with an invalid initializer; replace the direct
NSCollectionLayoutGroup(layoutSize:) call by creating a concrete group via a
factory like NSCollectionLayoutGroup.horizontal(layoutSize:subitems:) or
.vertical(layoutSize:subitems:) and pass the previously created `item` as the
subitems array (e.g., subitems: [item]) so the group is constructed correctly;
update the `group` creation in emptyLayout() to use one of these factory
methods.
In `@Projects/Features/SearchFeature/Sources/SearchViewController.swift`:
- Around line 34-40: The override of viewDidLoad in SearchViewController is
missing a call to super; update the viewDidLoad() implementation to call
super.viewDidLoad() (typically as the first statement) before invoking
setStyle(), setUI(), setLayout(), bindKeyboard(), and setupActions() so UIKit's
lifecycle behavior is preserved.
In `@Projects/Features/SettingFeature/Sources/SettingViewController.swift`:
- Around line 127-137: The .toggle branch in tableView(_:didSelectRowAt:) is
dead because the UISwitch in SettingMenuCell intercepts taps; remove or stop
relying on the .toggle handling there and instead handle toggle events in the
UISwitch .valueChanged handler inside SettingMenuCell (or via a delegate/closure
exposed by SettingMenuCell) and call listener?.didTapMenu(item:) (or a new
listener?.didToggleMenu(item:isOn:)) from that handler; update SettingMenuCell
to expose a switchValueChanged callback (or delegate method) and wire it in
SettingViewController when configuring cells, keeping
tableView(_:didSelectRowAt:) only for selectable types like .icon.
- Around line 32-38: The override of viewDidLoad in SettingViewController is
missing a call to super.viewDidLoad(); add super.viewDidLoad() as the first
statement inside the viewDidLoad() implementation so the UIViewController
lifecycle is preserved before calling setStyle(), setUI(), setLayout(),
setDelegate(), and setupActions().
In
`@Projects/Features/SettingFeature/Sources/UITableView/Cells/SettingMenuCell.swift`:
- Around line 56-59: The toggle switch currently only updates its visual state
in SettingMenuCell (case .toggle) so taps never trigger
tableView(_:didSelectRowAt:); add a UIControl.Event.valueChanged target-action
to toggleSwitch (e.g., in the cell initializer or in configureForMenuItem) that
calls a handler on the cell which in turn invokes listener?.didTapMenu(item:
currentItem) (or the appropriate method) to perform the toggle logic; ensure the
handler reads toggleSwitch.isOn and uses the cell's associated menu item (stored
property) to notify the listener, and keep selectionStyle = .none and
toggleSwitch.isHidden logic as-is.
In `@Projects/Modules/Networks/Sources/Extensions/MoyaProvider`+Async.swift:
- Around line 117-140: The success check currently uses HTTP status (200...299)
but your other `request` method treats success by inspecting `baseResponse.code
== "2000"`, causing inconsistent behavior; update this async handler (in
MoyaProvider+Async.swift) to first decode the response into BaseResponse<T>
(using response.map(BaseResponse<T>.self)), then check `baseResponse.code ==
"2000"`: if true, return `baseResponse.data` or throw `NetworkError.noData`; if
false, throw `NetworkError.unknown(baseResponse.message ?? "알 수 없는 오류가
발생했습니다.")`; keep the ErrorResponse fallback only if BaseResponse decoding fails
and use `response.map(ErrorResponse.self)` to extract message, and ensure all
continuation.resume calls are used consistently (resume(returning:) or
resume(throwing:)).
- Around line 110-112: NetworkLogger.logRequest is being called inside the
request(target) completion closure (in the request callback) which logs after
the request was sent; move the call to NetworkLogger.logRequest(target) so it
executes immediately before entering withCheckedThrowingContinuation (i.e., call
NetworkLogger.logRequest just prior to invoking
withCheckedThrowingContinuation/request for the same target), mirroring the
pattern used in the other methods; update references around
withCheckedThrowingContinuation, request(target) and NetworkLogger.logRequest to
ensure logging happens before the async continuation starts.
🟡 Minor comments (23)
Projects/Features/SearchFeature/Sources/SearchViewController.swift-20-20 (1)
20-20:⚠️ Potential issue | 🟡 MinorSwiftLint 경고: 포맷팅 수정 필요
SwiftLint에서 콤마 뒤 공백 및 여는 중괄호 앞 공백 관련 경고가 발생합니다.
🔧 수정 제안
-final class SearchViewController: UIViewController, SearchPresentable, SearchViewControllable{ +final class SearchViewController: UIViewController, SearchPresentable, SearchViewControllable {Projects/Modules/DSKit/Sources/Component/CategoryChipCell.swift-30-35 (1)
30-35:⚠️ Potential issue | 🟡 Minor
prepareForReuse에서attributedText를 초기화해야 합니다.
configure메서드에서titleLabel.setText()를 통해attributedText를 설정하지만,prepareForReuse에서는.text = nil로 초기화하고 있습니다. UIKit에서.text와.attributedText는 별개의 프로퍼티이므로,.text = nil로는 이전에 설정된attributedText가 완전히 클리어되지 않을 수 있습니다.🐛 수정 제안
override public func prepareForReuse() { super.prepareForReuse() iconView.image = nil - titleLabel.text = nil + titleLabel.attributedText = nil }Projects/Features/HomeFeature/Sources/Views/Cells/HomeHeaderView.swift-52-57 (1)
52-57:⚠️ Potential issue | 🟡 Minor
titleLabel에 trailing 제약 조건이 없습니다.현재
leading과directionalVerticalEdges만 설정되어 있어, 긴 텍스트가 뷰 영역을 넘어갈 수 있습니다.numberOfLines = 2와lineBreakMode = .byTruncatingTail이 적용되어 있지만, trailing 제약이 없으면 intrinsic content size가 뷰 너비를 초과할 수 있습니다.제안
titleLabel.snp.makeConstraints { $0.leading.equalToSuperview() + $0.trailing.lessThanOrEqualToSuperview() $0.directionalVerticalEdges.equalToSuperview() }Projects/Modules/Networks/Sources/TargetType/HomeAPI.swift-28-29 (1)
28-29:⚠️ Potential issue | 🟡 Minor
getMyTrip의 path가 빈 문자열입니다.빈 path는 base URL 자체로 요청을 보내게 됩니다. "임시" 코드로 표시되어 있지만, 실수로 프로덕션에 포함될 경우 의도하지 않은 엔드포인트로 요청이 전송됩니다. TODO 주석이나
fatalError로 명시적으로 표시하는 것을 권장합니다.Projects/Features/HomeFeature/Sources/Views/Component/HomeBannerUpCommingView.swift-12-12 (1)
12-12:⚠️ Potential issue | 🟡 Minor클래스명 오타:
HomeBannerUpCommingView→HomeBannerUpcomingView"UpComming"은 "Upcoming"의 오타입니다. 파일명과 클래스명 모두 수정이 필요합니다. 이 이름이 다른 파일에서 참조되기 전에 조기 수정하는 것이 좋습니다.
Projects/Features/HomeFeature/Sources/Views/Component/HomeBannerUpCommingView.swift-34-52 (1)
34-52:⚠️ Potential issue | 🟡 Minor
prepareForReuse()에서imageView.backgroundColor초기화 누락
configure에서 URL 파싱 실패 시imageView.backgroundColor = .systemGray5를 설정하지만,prepareForReuse()에서는 이를 초기화하지 않습니다. 재사용 시 이전 셀의 회색 배경이 남을 수 있습니다.🔧 수정 제안
func prepareForReuse() { imageView.kf.cancelDownloadTask() titleLabel.text = nil dateLabel.text = nil dDayLabel.text = nil imageView.image = nil + imageView.backgroundColor = .clear }Projects/Features/HomeFeature/Sources/Views/Component/HomeBannerOnGoingView.swift-63-72 (1)
63-72:⚠️ Potential issue | 🟡 Minor
prepareForReuse()에서imageView.backgroundColor초기화 누락
configure에서 URL 실패 시.systemGray5로 설정하지만 reuse 시 초기화되지 않습니다.HomeBannerUpCommingView와 동일한 문제입니다.🔧 수정 제안
func prepareForReuse() { imageView.kf.cancelDownloadTask() titleLabel.text = nil dateLabel.text = nil placeLabel.text = nil durationLabel.text = nil imageView.image = nil + imageView.backgroundColor = .systemGray6 iconImageView.image = nil transportLabel.text = nil }Projects/Modules/DSKit/Sources/Component/PopularInfoCell.swift-27-28 (1)
27-28:⚠️ Potential issue | 🟡 Minor
super.init(frame: .zero)— 전달받은frame파라미터를 무시하고 있습니다.
init(frame:)에서 전달받은frame을 사용하지 않고.zero를 전달하면, collection view의 레이아웃 시스템이 초기 프레임을 올바르게 설정하지 못할 수 있습니다.🐛 수정 제안
override public init(frame: CGRect) { - super.init(frame: .zero) + super.init(frame: frame)Projects/Modules/Networks/Sources/Service/HomeService.swift-23-28 (1)
23-28:⚠️ Potential issue | 🟡 Minor기본
MoyaProvider의 기본값 제거 권장
HomeService는 현재 DI 패턴을 통해 항상 인증된 provider가 주입되고 있어 실제 보안 위험은 낮습니다. 다만 생성자의 기본값MoyaProvider<HomeAPI>()는 인증 플러그인이 없으므로, 명시적으로 제거하여 provider 의존성을 필수로 만드는 것이 좋습니다. 이렇게 하면 의도하지 않은 unauthenticated provider 사용을 컴파일 시점에 방지할 수 있습니다.Projects/Domain/Sources/Model/Home/TripCategory.swift-14-14 (1)
14-14:⚠️ Potential issue | 🟡 Minor오타:
viedoType→videoType공개 API의 프로퍼티명에 오타가 있습니다. 이 이름은
ProgramResponse+.swift의toDomain()등 여러 곳에서 사용되므로, 조기에 수정하는 것이 좋습니다.✏️ 수정 제안
- public let viedoType: VideoType + public let videoType: VideoType - public init(id: Int, creator: String, viedoType: VideoType) { + public init(id: Int, creator: String, videoType: VideoType) { self.id = id self.creator = creator - self.viedoType = viedoType + self.videoType = videoType }Projects/Features/HomeFeature/Sources/Views/CollectionView/CompositionalLayout/HomeCompositionalLayout.swift-148-155 (1)
148-155:⚠️ Potential issue | 🟡 Minor
createRecommendedTripSection에서contentInsets값이 일관성 없이 사용되고 있습니다.다른 섹션에서는
.adjusted/.adjustedH를 사용하지만, 여기서는top: 24, leading: 24, trailing: 24에 raw 값이 사용되고bottom만81.adjustedH입니다. 의도적인 설계가 아니라면 다른 섹션과 동일하게 적용해 주세요.🔧 제안 수정
- section.contentInsets = .init(top: 24, leading: 24, bottom: 81.adjustedH, trailing: 24) + section.contentInsets = .init(top: 24.adjustedH, leading: 24.adjusted, bottom: 81.adjustedH, trailing: 24.adjusted)Projects/Modules/Networks/Sources/Extensions/MoyaProvider+Async.swift-109-109 (1)
109-109:⚠️ Potential issue | 🟡 Minor메서드명 오타:
asyncThowsRequest→asyncThrowsRequest
Thows는Throws의 오타입니다. 이 메서드는HomeService에서 호출되므로 모든 call site도 함께 수정해야 합니다.🐛 제안 수정
- func asyncThowsRequest<T: Decodable>(_ target: Target) async throws -> T { + func asyncThrowsRequest<T: Decodable & Sendable>(_ target: Target) async throws -> T {Projects/Features/HomeFeature/Sources/Views/Cells/RecommendInfoCell.swift-33-42 (1)
33-42:⚠️ Potential issue | 🟡 Minor
prepareForReuse()가 누락되어 셀 재사용 시 이전 데이터가 표시될 수 있습니다.
UICollectionViewCell이 재사용될 때 이전 셀의 썸네일 이미지와 텍스트가 잠시 보이는 문제가 발생할 수 있습니다. 특히 Kingfisher의 비동기 이미지 로딩 시 이전 이미지가 플리커 현상을 유발합니다.prepareForReuse 추가 제안
required init?(coder: NSCoder) { fatalError("init(coder:) has not been implemented") } +override func prepareForReuse() { + super.prepareForReuse() + thumbnailView.kf.cancelDownloadTask() + thumbnailView.image = nil +} + // MARK: - ConfigureProjects/Domain/Sources/Interface/Home/HomeServiceProtocol.swift-9-23 (1)
9-23:⚠️ Potential issue | 🟡 MinorDomain 레이어의 주석 처리된 파일을 삭제하세요.
Projects/Domain/Sources/Interface/Home/HomeServiceProtocol.swift의 전체 내용이 주석 처리되어 있습니다. 실제로 사용 중인HomeServiceProtocol은Projects/Modules/Networks/Sources/Service/HomeService.swift에서 정의되고 있으며,HomeRepository.swift와HomeServiceFactory.swift에서 이 네트워크 모듈의 프로토콜을 참조하고 있습니다.Domain 레이어의 주석 처리된 파일은 dead code이므로 삭제하시기 바랍니다.
Projects/Features/PopularTravelFeature/Sources/Models/PopularTravelPresentationModel.swift-17-21 (1)
17-21:⚠️ Potential issue | 🟡 Minor
viedoType→videoType오타입니다.
viedoType은videoType의 오타입니다. 도메인 모델TripCategory에서 비롯되어 PopularTravelPresentationModel, HomePresentationModel, 그리고 관련 View 파일들에 전파되어 있습니다. 퍼블릭 API가 확정되기 전에 도메인 모델부터 일괄 수정하는 것을 권장합니다.Projects/Features/HomeFeature/Sources/Models/HomePresentationModel.swift-36-40 (1)
36-40:⚠️ Potential issue | 🟡 Minor오타:
viedoType→videoType.도메인 모델에서 시작된 오타가 프레젠테이션 모델까지 전파되고 있습니다. 전체 코드베이스에서 일괄 수정이 필요합니다.
Projects/Features/HomeFeature/Sources/Models/HomePresentationModel.swift-64-64 (1)
64-64:⚠️ Potential issue | 🟡 Minor오타:
toPresention()→toPresentation().Public API 이름에 오타가 있습니다. 이후 수정하면 breaking change가 되므로 지금 수정하는 것이 좋습니다.
🐛 수정 제안
- func toPresention() -> HomePresentationModel.Banner { + func toPresentation() -> HomePresentationModel.Banner {Projects/Features/PopularTravelFeature/Sources/Views/CollectionView/CompositionalLayout/PopularTravelCompositionalLayout.swift-14-27 (1)
14-27:⚠️ Potential issue | 🟡 Minor
self가 nil일 때 레이아웃 provider가nil을 반환합니다.
[weak self]캡처로 인해self가 해제된 경우createCategorySection()등이nil을 반환할 수 있습니다.UICollectionViewCompositionalLayout의 section provider가nil을 반환하면 크래시가 발생할 수 있습니다.guard let self를 추가하여emptyLayout()을 fallback으로 사용하는 것이 안전합니다.🛡️ 수정 제안
return UICollectionViewCompositionalLayout { [weak self] sectionIndex, _ in + guard let self else { + // fallback: 빈 레이아웃을 인라인으로 생성 + let size = NSCollectionLayoutSize(widthDimension: .fractionalWidth(1), heightDimension: .fractionalHeight(1)) + let item = NSCollectionLayoutItem(layoutSize: size) + let group = NSCollectionLayoutGroup.horizontal(layoutSize: size, subitems: [item]) + return NSCollectionLayoutSection(group: group) + } guard let sectionKind = PopularTravelSectionKind(rawValue: sectionIndex) else { - return self?.emptyLayout() + return self.emptyLayout() } switch sectionKind { case .category: - return self?.createCategorySection() + return self.createCategorySection() case .popularTrip: - return self?.createPopularTripSection() + return self.createPopularTripSection() } }Projects/Features/HomeFeature/Sources/HomeViewController.swift-202-202 (1)
202-202:⚠️ Potential issue | 🟡 Minor오타:
createRecommedTripCellRegistration→createRecommendTripCellRegistration.
Recommed는Recommend의 오타로 보입니다. 이 메서드가 정의된 Registration 파일에서도 함께 수정이 필요합니다.Projects/Features/PopularTravelFeature/Sources/PopularTravelInteractor.swift-140-147 (1)
140-147:⚠️ Potential issue | 🟡 Minor
Int(trip.id) ?? 0은 파싱 실패 시 잘못된 리소스로 라우팅될 수 있습니다.
trip.id가 유효한 정수 문자열이 아닌 경우 ID0으로 폴백되어 존재하지 않는 리소스에 대한 상세 화면으로 이동하게 됩니다. 이는HomeInteractor(Line 164-166)에서도 동일한 패턴이 사용되고 있습니다.
id타입을 도메인 모델 단계에서Int로 정의하거나, 파싱 실패 시 early return을 고려하세요.Projects/Features/MainFeature/Sources/MainBuilder.swift-24-24 (1)
24-24:⚠️ Potential issue | 🟡 MinorSwiftLint: 쉼표 뒤 공백 누락.
PopularTravelDependency,SearchDependency사이에 공백이 없습니다.🔧 수정
-final class MainComponent: Component<MainDependency>, FollowDetailDependency, PopularTravelDependency,SearchDependency, SettingDependency, TabBarDependency { +final class MainComponent: Component<MainDependency>, FollowDetailDependency, PopularTravelDependency, SearchDependency, SettingDependency, TabBarDependency {Projects/Features/HomeFeature/Sources/Views/CollectionView/Registration/HomeRegistration.swift-48-48 (1)
48-48:⚠️ Potential issue | 🟡 Minor메서드 이름 오타:
createRecommedTripCellRegistration→createRecommendTripCellRegistration
Recommed에서n이 빠져 있습니다. 이 메서드를 호출하는 모든 곳도 함께 수정해 주세요.🔤 수정 제안
- func createRecommedTripCellRegistration() -> UICollectionView.CellRegistration<RecommendInfoCell, HomePresentationModel.RecommendedTrip> { + func createRecommendTripCellRegistration() -> UICollectionView.CellRegistration<RecommendInfoCell, HomePresentationModel.RecommendedTrip> {Projects/Features/HomeFeature/Sources/Views/CollectionView/Registration/HomeRegistration.swift-24-30 (1)
24-30:⚠️ Potential issue | 🟡 Minor프로퍼티 이름 오타:
viedoType→videoType로 수정 필요
viedoType은videoType의 오타입니다. 이 오타는 모델 정의뿐만 아니라 여러 파일에 걸쳐 존재합니다:
TripCategory(Domain 모델)HomePresentationModel.CategoryPopularTravelPresentationModel.Category- 데이터 변환 및 뷰 등록 파일들
전체 코드베이스에서 일관되게 수정해야 합니다.
🧹 Nitpick comments (64)
Projects/Domain/Sources/Model/Travel/Recommendation.swift (1)
12-35: 주석 처리된 코드 대신 파일을 삭제하세요.
Recommendation구조체 전체가 주석 처리되어 있어, 이 파일에는 실질적인 코드가 없습니다. 버전 관리(git)가 히스토리를 보존하므로 주석으로 남길 필요가 없습니다. PR 설명에서 언급된 새로운 도메인 모델(TripCategory,TripInfo등)로 대체되었다면, 이 파일 자체를 삭제하는 것이 바람직합니다.임시로 유지해야 한다면, 최소한
// TODO:주석으로 정리 시점과 이유를 명시해 주세요.Projects/Domain/Sources/Model/Travel/PopularTrip.swift (1)
11-47: 주석 처리된 코드를 제거하고 파일을 삭제하세요.
PopularTrip과TripCategory가 다른 위치로 이동/대체된 상태에서, 기존 코드를 주석으로 남겨두는 것은 불필요합니다. Git 히스토리가 이전 코드를 보존하고 있으므로, 주석 처리된 코드 블록과 함께 이 파일 자체를 삭제하는 것이 좋습니다. 빈 파일(혹은 주석만 있는 파일)이 프로젝트에 남아 있으면 혼란을 줄 수 있습니다.Projects/Core/Sources/Extensions/UIKit+/UICollectionViewCell+.swift (1)
12-14: SwiftLint 경고: 콜론 간격 및 암시적 반환 수정SwiftLint에서 두 가지 경고가 발생합니다:
- 콜론은 식별자 바로 뒤에 붙여야 합니다 (
cellIdentifier : String→cellIdentifier: String).- 단일 표현식 getter에서는
return을 생략하는 것이 좋습니다.♻️ 수정 제안
- static var cellIdentifier : String { - return String(describing: self) - } + static var cellIdentifier: String { + String(describing: self) + }Projects/Core/Sources/Extensions/UIKit+/UITableViewCell+.swift (1)
12-14: SwiftLint 경고: 암시적 반환 선호단일 표현식 getter에서는
return키워드를 생략하는 것이 Swift 관용적 스타일입니다.♻️ 수정 제안
static var cellIdentifier: String { - return String(describing: self) + String(describing: self) }Projects/Core/Sources/Extensions/UIKit+/UICollectionReusableView+.swift (1)
12-14: SwiftLint 경고: 콜론 간격 및 암시적 반환 수정
UICollectionViewCell+.swift와 동일한 SwiftLint 경고가 발생합니다.♻️ 수정 제안
- static var reusableViewIdentifier : String { - return String(describing: self) - } + static var reusableViewIdentifier: String { + String(describing: self) + }Projects/Features/TabBarFeature/Sources/Components/NDGLTabItem.swift (1)
37-41: 선택/비선택 상태의 색상 일관성 확인
setup()에서 타이틀 색상을white로 고정 설정하고,updateState()에서는 아이콘 tintColor만 상태에 따라 변경하고 있습니다. 현재는 타이틀이 선택 상태에서만 표시되므로(isHidden = !isTabSelected, line 89) 문제가 없지만, 향후 비선택 상태에서도 타이틀을 표시하게 되면 흰색 텍스트가 의도치 않게 보일 수 있습니다.현재 동작에는 문제 없으므로 참고 사항입니다.
Also applies to: 88-99
Projects/Modules/DSKit/Sources/Extensions/UIKit+/UILabel+.swift (1)
37-41:lineBreakMode를.byTruncatingTail로 하드코딩하면 멀티라인 레이블이 깨질 수 있습니다.
NSMutableParagraphStyle의 기본lineBreakMode는.byWordWrapping이고,attributedText를 설정하면 paragraph style의 값이 UILabel의 속성보다 우선합니다. 이 변경으로setText를 사용하는 모든 레이블이 말줄임 처리되므로,numberOfLines > 1인 멀티라인 레이블에서 텍스트가 잘릴 수 있습니다.
alignment파라미터처럼lineBreakMode도 파라미터로 분리하는 것을 권장합니다.♻️ 제안: lineBreakMode를 파라미터로 추가
func setText( _ style: UIFont.NDGL, text: String, color: UIColor, - alignment: NSTextAlignment = .left + alignment: NSTextAlignment = .left, + lineBreakMode: NSLineBreakMode = .byTruncatingTail ) { var attributes = style.attributes if let paragraphStyle = attributes[.paragraphStyle] as? NSMutableParagraphStyle { paragraphStyle.alignment = alignment - paragraphStyle.lineBreakMode = .byTruncatingTail + paragraphStyle.lineBreakMode = lineBreakMode attributes[.paragraphStyle] = paragraphStyle }Projects/Core/Sources/Extensions/Foundation+/String+.swift (1)
28-33:toFlag()와의 일관성을 위해uppercased()처리를 추가하세요.
toFlag()는 내부적으로uppercased()를 호출하여 소문자 입력도 처리하지만,toKoreanCountryName()은 그렇지 않습니다.Locale.localizedString(forRegionCode:)가 일반적으로 대소문자를 구분하지 않지만, 명시적으로 처리하는 것이 더 안전하고 일관성 있습니다.♻️ 제안 수정
func toKoreanCountryName() -> String { - guard self.count == 2 else { return "알 수 없음"} + guard self.count == 2 else { return "알 수 없음" } let locale = Locale(identifier: "ko_KR") - return locale.localizedString(forRegionCode: self) ?? "알 수 없음" + return locale.localizedString(forRegionCode: self.uppercased()) ?? "알 수 없음" }Projects/Modules/DSKit/Sources/Component/NDGLErrorView.swift (3)
87-90:containerStackView의 수직 중앙 정렬 로직이 취약할 수 있습니다.
$0.centerY.equalTo(button.snp.top).dividedBy(2)는 superview의 좌표 원점이 y=0이라는 가정 하에만 정상 동작합니다.NDGLErrorView가 safe area inset이 있는 컨테이너에 포함되거나, 상단에 offset이 적용된 경우 의도대로 중앙 정렬되지 않습니다.보다 안정적인 방법으로는, button 상단까지의 영역에 대해 명시적으로 center를 잡는 것을 고려해 주세요.
♻️ 제안: UILayoutGuide 활용
+ private let centerGuide = UILayoutGuide()
setLayout()내에서:+ addLayoutGuide(centerGuide) + centerGuide.snp.makeConstraints { + $0.top.leading.trailing.equalToSuperview() + $0.bottom.equalTo(button.snp.top) + } + containerStackView.snp.makeConstraints { $0.centerX.equalToSuperview() - $0.centerY.equalTo(button.snp.top).dividedBy(2) + $0.centerY.equalTo(centerGuide.snp.centerY) }
83-85: 이미지 크기에adjustedH만 사용하고 있습니다.
size는 width와 height를 모두 설정하므로, 학습된 DSKit 스케일링 가이드라인에 따르면 width는adjustedW/adjusted, height는adjustedH를 사용해야 합니다. 정사각형 이미지라면 한쪽 기준으로 통일하는 것도 합리적이지만, 가이드라인 일관성을 위해 width/height를 분리하는 것을 고려해 주세요.♻️ 제안
imageView.snp.makeConstraints { - $0.size.equalTo(140.adjustedH) + $0.width.equalTo(140.adjusted) + $0.height.equalTo(140.adjustedH) }Based on learnings: "In the NDGL iOS design system, ensure all components use the defined scaling extensions: adjustedH scales by height ratio, and adjustedW and adjusted scale by width ratio. Spacing between components should consistently use horizontal spacing with adjustedW/adjusted and vertical spacing with adjustedH."
39-61: 타이틀, 서브타이틀, 버튼 텍스트가 하드코딩되어 있습니다.현재 에러 메시지("정보를 불러올 수 없어요", "인터넷 연결 확인 후 다시 시도해 주세요")와 버튼 텍스트("다시 시도")가 고정되어 있어, 다양한 에러 상황(예: 404, 권한 오류 등)에서 재사용이 어렵습니다. 범용 에러 뷰로 활용하려면 이니셜라이저 파라미터로 텍스트를 주입받는 구조를 고려해 주세요.
Projects/Features/SearchFeature/Sources/SearchViewController.swift (1)
98-125: 키보드 애니메이션 duration을 notification에서 가져오는 것을 권장현재 하드코딩된
0.3초 대신, 시스템이 제공하는UIResponder.keyboardAnimationDurationUserInfoKey값을 사용하면 키보드 애니메이션과 더 자연스럽게 동기화됩니다.♻️ keyboardWillShow 예시
.subscribe(onNext: { [weak self] notification in guard let keyboardFrame = notification.userInfo?[UIResponder.keyboardFrameEndUserInfoKey] as? NSValue else { return } let keyboardHeight = keyboardFrame.cgRectValue.height + let duration = notification.userInfo?[UIResponder.keyboardAnimationDurationUserInfoKey] as? Double ?? 0.3 self?.containerView.snp.updateConstraints { $0.bottom.equalToSuperview().inset(keyboardHeight) } - UIView.animate(withDuration: 0.3) { + UIView.animate(withDuration: duration) { self?.view.layoutIfNeeded() } })Projects/Features/SearchFeature/Sources/SearchInteractor.swift (1)
37-45: 불필요한 override 메서드 (SwiftLint 경고)
didBecomeActive()와willResignActive()는 현재super호출만 하고 있어 SwiftLintunneeded_override경고가 발생합니다. TODO가 구현될 때까지는 제거해도 무방하지만, 향후 비즈니스 로직 추가를 위한 placeholder라면 유지해도 괜찮습니다.Projects/Modules/DSKit/Sources/Component/CategoryChipCell.swift (2)
62-84: 하드코딩된 spacing 값과borderColor의 동적 테마 대응 미비.
- Line 80:
spacing = 4가 하드코딩되어 있습니다. DSKit의 스케일링 규칙에 따라 수평 간격에는.adjusted또는.adjustedW를 사용해야 합니다.- Line 66:
borderColor를CGColor로 설정하면 다크모드 전환 시 자동으로 갱신되지 않습니다.traitCollectionDidChange에서 업데이트하거나layoutSubviews에서 재할당하는 처리가 필요합니다.♻️ 수정 제안
spacing 수정:
stackView.do { $0.axis = .horizontal - $0.spacing = 4 + $0.spacing = 4.adjusted $0.alignment = .center $0.isUserInteractionEnabled = false }borderColor 동적 대응 (클래스 본문에 추가):
override public func traitCollectionDidChange(_ previousTraitCollection: UITraitCollection?) { super.traitCollectionDidChange(previousTraitCollection) if traitCollection.hasDifferentColorAppearance(comparedTo: previousTraitCollection) { contentView.layer.borderColor = DSKitAsset.Colors.black200.color.cgColor } }Based on learnings: "spacing between components should consistently use horizontal spacing with adjustedW/adjusted and vertical spacing with adjustedH."
104-127: 하드코딩된 padding 값과 SwiftLint 경고.
- Line 123:
.none케이스의horizontalPadding이23.5로 하드코딩되어 있어 화면 크기에 따라 스케일링되지 않습니다..youtube/.tv케이스처럼.adjusted를 적용해야 합니다.- Line 107: SwiftLint에서
tvenum case 이름이 3자 미만이라는 경고가 발생합니다.♻️ 수정 제안
fileprivate var horizontalPadding: CGFloat { switch self { case .none: - return 23.5 + return 23.5.adjusted case .youtube, .tv: return 14.adjusted } }Based on learnings: "replace any hard-coded spacing with these extensions and verify visual consistency."
Projects/Features/SettingFeature/Sources/UITableView/SettingCellItem.swift (1)
32-33: 알림 토글 상태가 하드코딩되어 있습니다.
.notification의isOn: true가 하드코딩되어 있어 실제 사용자의 알림 설정 상태를 반영하지 않습니다.UserDefaults나 도메인 레이어로부터 실제 상태를 주입받는 것이 좋습니다.Projects/Features/SettingFeature/Sources/UITableView/Cells/SettingMenuCell.swift (1)
44-48:prepareForReuse에서toggleSwitch상태 초기화 누락.
titleLabel과detailLabel은 초기화하고 있지만,toggleSwitch.isOn상태는 초기화하지 않습니다.configure에서 매번 설정하긴 하지만, 방어적으로 리셋하는 것이 좋습니다.♻️ 수정 제안
override func prepareForReuse() { super.prepareForReuse() titleLabel.text = nil detailLabel.text = nil + toggleSwitch.isOn = false }Projects/Features/SettingFeature/Sources/SettingInteractor.swift (2)
35-41:super만 호출하는 불필요한 오버라이드를 제거하세요.
didBecomeActive()와willResignActive()가super만 호출하고 있으므로 제거해도 됩니다. SwiftLintunneeded_override규칙에서도 경고하고 있습니다.♻️ 수정 제안
- override func didBecomeActive() { - super.didBecomeActive() - } - - override func willResignActive() { - super.willResignActive() - }
48-63:didTapMenu의각 메뉴 항목에 대한 실제 동작이
// TODO:주석을 남겨두면 추적에 도움이 됩니다.Tuist/Package.swift (1)
11-22: 모든 의존성을.framework(동적 프레임워크)로 설정한 이유를 확인해 주세요.동적 프레임워크는 여러 모듈에서 공유 시 중복 심볼 문제를 방지하지만, 앱 실행 시 dyld 로딩 시간이 증가합니다. 의존성 수가 많아지면 콜드 스타트 성능에 영향을 줄 수 있으므로, 실제로 여러 모듈에서 공유되지 않는 라이브러리(e.g.,
Kingfisher,Then)는 static으로 유지하는 것이 유리할 수 있습니다.현재 모듈 구조에서 이 설정이 의도된 것인지 확인 부탁드립니다.
Projects/Modules/Networks/Sources/DTO/Home/ProgramResponse.swift (1)
11-15:Sendable준수 추가를 권장합니다.
BaseResponse는Sendable을 준수하고 있으며, 프로젝트에서 async/throws 네트워크 레이어를 사용하고 있으므로ProgramResponse에도Sendable을 추가하면 Swift 6 strict concurrency에서의 호환성이 향상됩니다.제안
-public struct ProgramResponse: Decodable { +public struct ProgramResponse: Decodable, Sendable {Projects/Domain/Sources/Model/Home/HomeError.swift (1)
11-15: 주석 처리된 코드는 삭제하는 것이 좋습니다.
HomeErrorenum이 전체 주석 처리되어 있습니다. 사용하지 않는 코드라면 파일 자체를 제거하고, 추후 필요 시 git 히스토리에서 복원하는 것이 깔끔합니다. 현재 상태로는 데드 코드로 남게 됩니다.PR 설명에 언급된 임시 조치라면, 머지 후 정리 시 이 파일도 함께 제거해 주세요.
Projects/Features/HomeFeature/Sources/Views/Cells/HomeFooterButtonView.swift (1)
43-71: 하드코딩된 한국어 문자열에 대해 로컬라이제이션을 고려해 주세요.
"여행 따라가기 더보기"문자열이 코드에 직접 하드코딩되어 있습니다. 향후 다국어 지원 계획이 있다면Localizable.strings등으로 분리하는 것이 좋습니다. 현재 단계에서 불필요하다면 무시하셔도 됩니다.Projects/Features/HomeFeature/Sources/Views/Cells/HomeHeaderView.swift (1)
28-38:prepareForReuse에서attributedText를 nil로 설정하는 것이 더 정확합니다.
configure에서는setText를 통해attributedText를 설정하지만,prepareForReuse에서는text를 nil로 설정하고 있습니다. UIKit에서text를 nil로 설정하면attributedText도 함께 초기화되므로 동작상 문제는 없지만, 일관성을 위해attributedText = nil로 맞추는 것이 명확합니다.Projects/Features/TabBarFeature/Sources/TabBarViewController.swift (1)
189-191:UIImpactFeedbackGenerator를 미리prepare()호출 후 사용하는 것을 권장합니다.현재 매 탭 전환마다 새 인스턴스를 생성하고 바로
impactOccurred()를 호출합니다.prepare()를 먼저 호출하면 햅틱 응답 지연이 줄어듭니다.Projects/Domain/Sources/Model/Home/MyTripSummary.swift (1)
27-51:Schedule은 너무 범용적인 이름입니다.
Schedule이 public Domain 모듈에 정의되어 있어 다른 모듈이나 프레임워크와 이름 충돌 가능성이 높습니다.TripSchedule등 도메인 컨텍스트를 포함한 이름을 권장합니다.Projects/Features/HomeFeature/Sources/Views/Component/HomeBannerOnGoingView.swift (1)
40-61:configure메서드의 파라미터가 7개로 과다합니다.SwiftLint에서도 경고하는 것처럼, 파라미터가 5개를 초과합니다. 전용 뷰 모델 struct로 묶는 것을 권장합니다.
♻️ 리팩토링 제안
+struct HomeBannerOnGoingViewModel { + let title: String + let date: String + let transportIcon: UIImage? + let transport: String + let duration: String + let place: String + let imageUrl: String +} + -func configure( - title: String, - date: String, - transportIcon: UIImage?, - transport: String, - duration: String, - place: String, - imageUrl: String -) { +func configure(with viewModel: HomeBannerOnGoingViewModel) {Projects/Modules/Networks/Sources/TargetType/HomeAPI.swift (1)
46-67:getPopularTripList과getRecommendTripList의 query parameter 구성 로직이 중복됩니다.
page/size파라미터 처리 로직이 동일합니다. 필요 시 헬퍼로 추출할 수 있지만, 현재 규모에서는 허용 가능합니다.Projects/Modules/DSKit/Sources/Component/PopularInfoCell.swift (1)
96-106: 하드코딩된 spacing 값 — DSKit 스케일링 확장 함수 사용이 필요합니다.
infoStackView.spacing = 4(수직 스택)와textContainerStackView.spacing = 4(수평 스택)에서 하드코딩된 spacing이 사용되고 있습니다. DSKit 컴포넌트의 일관된 스케일링을 위해 각각.adjustedH와.adjusted를 적용해야 합니다. Line 73의cornerRadius = 6도 마찬가지입니다.♻️ 수정 제안
thumbnailView.do { - $0.layer.cornerRadius = 6 + $0.layer.cornerRadius = 6.adjusted $0.clipsToBounds = trueinfoStackView.do { $0.axis = .vertical - $0.spacing = 4 + $0.spacing = 4.adjustedH $0.alignment = .leading } textContainerStackView.do { $0.axis = .horizontal - $0.spacing = 4 + $0.spacing = 4.adjusted $0.alignment = .center }Based on learnings: "In the NDGL iOS design system (Projects/Modules/DSKit/Sources/Component), ensure all components use the defined scaling extensions: adjustedH scales by height ratio... adjustedW and adjusted scale by width ratio... replace any hard-coded spacing with these extensions."
Projects/Modules/Networks/Sources/Service/HomeService.swift (1)
13-21: 임시 프로토콜 및 반환 타입에 대한 정리 계획이 필요합니다.
getMyTrip() -> Int는 임시 구현으로 표시되어 있고, 프로토콜 자체도 "API 나오기 전 임시"로 주석 처리되어 있습니다. API가 확정된 후 적절한 도메인 모델(예:MyTripSummary)을 반환하도록 업데이트가 필요합니다.이 임시 항목들을 추적하기 위한 이슈를 생성할까요?
Projects/Features/MainFeature/Sources/MainInteractor.swift (2)
47-49: 빈willResignActive오버라이드를 제거하세요.
super.willResignActive()만 호출하는 빈 오버라이드입니다. SwiftLint에서도unneeded_override로 경고하고 있습니다.♻️ 수정 제안
- override func willResignActive() { - super.willResignActive() - }
55-57: 디버그 주석 "이건 뭐임"을 정리해주세요.
followDetailDidAddTrip은 현재 no-op 상태이며, 의도를 설명하는// TODO:주석으로 교체하거나 구현을 추가해야 합니다.♻️ 수정 제안
func followDetailDidAddTrip(title: String, startDate: Date, endDate: Date) { - // 이건 뭐임 + // TODO: 여행 추가 로직 구현 필요 }Projects/Features/MainFeature/Project.swift (1)
8-10: SwiftLint: import 정렬 순서 경고.SwiftLint에서 import 정렬 순서 경고(
sorted_imports)가 발생하고 있습니다.♻️ 수정 제안
-import ProjectDescription -import ProjectDescriptionHelpers import DependencyPlugin +import ProjectDescription +import ProjectDescriptionHelpersProjects/Modules/Networks/Sources/Base/NetworkError.swift (1)
23-24:noData에러 메시지의 마침표 사용이 다른 케이스와 불일치합니다.
connectionFailed와decodingFailed는 마침표 없이 끝나지만,noData는 마침표("응답 데이터가 없습니다.")로 끝납니다. 사소하지만 일관성을 위해 통일하는 것이 좋습니다.♻️ 수정 제안
case .noData: - return "응답 데이터가 없습니다." + return "응답 데이터가 없습니다"Projects/Data/Sources/DI/HomeServiceFactory.swift (1)
14-17: 네임스페이스 없는 전역 함수 사용.
makeHomeService가 전역 함수로 정의되어 있습니다.HomeServiceFactory같은enum이나struct로 감싸면 네임스페이스 충돌을 방지하고, 다른 팩토리(NetworkProviderFactory등)와 패턴이 일관됩니다.♻️ 네임스페이스 적용 제안
-public func makeHomeService(tokenProvider: TokenProviding) -> HomeServiceProtocol { - let provider: MoyaProvider<HomeAPI> = NetworkProviderFactory.makeAuthenticatedProvider(tokenProvider: tokenProvider) - return HomeService(provider: provider) -} +public enum HomeServiceFactory { + public static func makeHomeService(tokenProvider: TokenProviding) -> HomeServiceProtocol { + let provider: MoyaProvider<HomeAPI> = NetworkProviderFactory.makeAuthenticatedProvider(tokenProvider: tokenProvider) + return HomeService(provider: provider) + } +}Projects/Domain/Sources/Model/Home/VideoType.swift (1)
11-25:case none은Optional.none과 충돌할 수 있습니다.
VideoType?타입에서.none을 사용할 때Optional.none과VideoType.none이 모호해질 수 있습니다.case unknown등 다른 이름을 권장합니다.♻️ 제안
- case none + case unknown public init(rawValue: String) { switch rawValue { case "YOUTUBE": self = .youtube case "TV": self = .tv default: - self = .none + self = .unknown } }Projects/Features/SearchFeature/Project.swift (1)
8-10: SwiftLintsorted_imports: import 순서를 알파벳순으로 정렬해 주세요.SwiftLint가 import 정렬 경고를 보고하고 있습니다.
🔧 제안 수정
-import ProjectDescription -import ProjectDescriptionHelpers import DependencyPlugin +import ProjectDescription +import ProjectDescriptionHelpersProjects/Features/SettingFeature/Project.swift (1)
8-10: SwiftLintsorted_imports: import 순서를 알파벳순으로 정렬해 주세요.
SearchFeature/Project.swift와 동일한 SwiftLint 경고입니다.🔧 제안 수정
-import ProjectDescription -import ProjectDescriptionHelpers import DependencyPlugin +import ProjectDescription +import ProjectDescriptionHelpersProjects/Modules/Networks/Sources/Extensions/MoyaProvider+Async.swift (1)
134-138: 하드코딩된 한국어 에러 메시지
"알 수 없는 오류가 발생했습니다."가 두 곳에 하드코딩되어 있습니다. Localization 또는 상수로 추출하면 유지보수성이 향상됩니다.Projects/Data/Sources/Repository/Home/HomeRepository.swift (1)
21-23:fetchMyTripInfo()가 하드코딩된 더미 데이터를 반환합니다.이 메서드는 실제 네트워크 호출 없이 고정된
MyTripSummary를 반환하고 있습니다. 추후 실제 API 연동 시 교체가 필요합니다.// TODO:주석을 추가하여 임시 코드임을 명시해 주세요.💡 TODO 주석 추가 제안
+ // TODO: 실제 API 연동으로 교체 필요 public func fetchMyTripInfo() async throws -> MyTripSummary? { return MyTripSummary(title: "임시", startDay: .now, endDay: .now, tripSchedule: [Schedule(id: 1, day: 1, placeName: "임시", thumbnailUrl: "", transport: "", estimatedDuration: 2)]) }이 TODO를 추적하기 위한 이슈를 생성해 드릴까요?
Projects/App/Sources/Application/AppComponent.swift (1)
16-19:homeUsecase가 매 접근 시 새 인스턴스를 생성합니다.
tokenProvider는shared { ... }로 감싸 싱글턴처럼 동작하지만,homeUsecase는 일반 computed property여서 접근할 때마다HomeRepository,HomeService, 그리고MoyaProvider가 새로 생성됩니다.homeUsecase가 여러 하위 RIB 컴포넌트에 주입되므로, 불필요한 객체 생성을 줄이기 위해shared로 감싸는 것을 고려해 보세요. 이는tokenProvider와 동일한 패턴을 따르게 됩니다.💡 shared로 감싸는 예시
var homeUsecase: HomeUsecaseProtocol { - let homeRepository = HomeRepository(homeService: makeHomeService(tokenProvider: tokenProvider)) - return HomeUsecase(repository: homeRepository) + shared { + let homeRepository = HomeRepository(homeService: makeHomeService(tokenProvider: tokenProvider)) + return HomeUsecase(repository: homeRepository) + } }Projects/Domain/Sources/Interface/Home/HomeRepositoryInterface.swift (2)
11-11: "임시" 주석을 이슈/TODO로 추적해 주세요.
// MARK: - API 나오기 전 임시주석이 있습니다. API 확정 후 인터페이스가 변경될 수 있으므로, 이를 추적 가능한// TODO:또는 이슈로 등록하는 것을 권장합니다.
11-17: 프로토콜 확장 패턴 재검토 필요 - 무한 재귀는 발생하지 않지만 코드 명확성 개선 권장원래 우려와 달리,
HomeUsecase.swift의 코드는 실제로 무한 재귀를 일으키지 않습니다. 프로토콜 확장 메서드(19-34줄)에서 명시적 인자로fetchPopularTripList(id: id, page: page, size: size)를 호출할 때, 기본값이 있는 확장 메서드가 아닌 프로토콜 요구사항(15줄)으로 디스패치되어HomeUsecase의 구체적 구현(53줄)을 실행합니다.다만 이 패턴은 Swift의 메서드 디스패치 규칙에 의존하므로 유지보수성이 낮습니다. 프로토콜 확장에서 기본값을 제공하는 대신, 구체적 구현에서 직접 기본값을 처리하는 것이 더 명확합니다:
// 현재 패턴 (암묵적, 혼동 유발) extension HomeUsecase: HomeUsecaseProtocol { public func fetchPopularTripList(id: Int?, page: Int?, size: Int?) async throws -> [TripInfo] { try await repository.fetchPopularTripList(id: id, page: page, size: size) } }대신
HomeRepositoryInterface메서드 자체에 기본값을 두거나, 별도의 편의 메서드를 구분하여 사용할 것을 권장합니다.Projects/Features/TabBarFeature/Sources/TabBarInteractor.swift (1)
67-86: HomeListener 이벤트 포워딩이 깔끔합니다.HomeListener의 각 이벤트를 TabBarListener로 위임하는 패턴이 RIBs의 역할 분리 원칙에 부합합니다. 다만
homeDidAddTrip(Line 84-86)은 여전히presenter.switchToTab(at: 2)를 직접 호출하고 있는데, 다른 메서드들과 동일하게 listener를 통한 라우팅으로 통일하는 것이 일관성 측면에서 좋을 수 있습니다.Projects/Features/PopularTravelFeature/Sources/Models/PopularTravelPresentationModel.swift (1)
34-56: 도메인 → 프레젠테이션 매핑 패턴이 적절합니다.
TripCategory와TripInfo에 대한 extension 매핑 메서드가 레이어 간 변환을 깔끔하게 처리합니다. SwiftLint에서implicit_return경고가 발생하므로, 단일 표현식 함수에서return을 생략하는 것을 고려해 주세요.SwiftLint implicit_return 수정
extension TripCategory { func toPopularTravelModel() -> PopularTravelPresentationModel.Category { - return PopularTravelPresentationModel.Category( + PopularTravelPresentationModel.Category( id: self.id, creator: self.creator, viedoType: self.viedoType ) } } extension TripInfo { func toPopularTravelModel() -> PopularTravelPresentationModel.PopularTrip { - return PopularTravelPresentationModel.PopularTrip( + PopularTravelPresentationModel.PopularTrip( id: self.id, title: self.title, thumbnailUrl: self.thumbnailUrl, creator: self.creator, schedule: "\(self.nights)박 \(self.days)일", country: self.country, city: self.city ) } }Projects/Features/HomeFeature/Sources/Views/Cells/HomeBannerCell.swift (2)
153-163: 오타:upComming→upComing(또는upcoming).
HomeBannerType.upComming,HomeBannerUpCommingView등에서 "upcoming"의 철자가 잘못되어 있습니다. 여러 파일에 걸쳐 일관되게 사용되고 있으므로, 지금 수정하면 향후 혼란을 방지할 수 있습니다.
61-94:configure호출 시마다DateFormatter가 새로 생성됩니다.
formatDateRange에서 매번DateFormatter를 생성하는 것은 비용이 큽니다. 셀이 빠르게 재사용될 때 성능에 영향을 줄 수 있습니다.static프로퍼티로 캐싱하는 것을 권장합니다.또한, Line 74와 76에서
} else if가 별도 줄에 위치해 있어 Swift 관례와 다릅니다.♻️ DateFormatter 캐싱 제안
private extension HomeBannerCell { + static let koreanDateFormatter: DateFormatter = { + let formatter = DateFormatter() + formatter.locale = Locale(identifier: "ko_KR") + formatter.dateFormat = "M월 d일" + return formatter + }() + func formatDateRange(start: Date, end: Date) -> String { - let formatter = DateFormatter() - formatter.locale = Locale(identifier: "ko_KR") - formatter.dateFormat = "M월 d일" - + let formatter = Self.koreanDateFormatter let startStr = formatter.string(from: start) let endStr = formatter.string(from: end) return "\(startStr) ~ \(endStr)" }Projects/Features/PopularTravelFeature/Sources/Views/CollectionView/Registration/PopularTravelRegistration.swift (2)
14-28: SwiftLint: 미사용indexPath파라미터를_로 교체하세요.Line 15와 31에서
indexPath가 사용되지 않고 있습니다.♻️ 수정 제안
- return UICollectionView.CellRegistration { cell, indexPath, item in + return UICollectionView.CellRegistration { cell, _, item in
18-24:viedoType오타가 도메인 모델에서 전파되고 있습니다.
chip.viedoType은videoType의 오타입니다. 이 오타는TripCategory,HomePresentationModel.Category,PopularTravelPresentationModel.Category등 여러 레이어에 걸쳐 퍼져 있습니다. 도메인 모델에서 먼저 수정한 후 일괄 변경하는 것이 좋습니다.Projects/Features/HomeFeature/Sources/HomeBuilder.swift (1)
11-12: TODO:import Data임시 의존성이 남아있습니다.PR 설명에서 언급된 임시 의존성입니다. Feature 모듈이 Domain 인터페이스에만 의존해야 하는 원칙에 위배되므로, 머지 후 반드시 제거해야 합니다.
이 임시 의존성 제거를 추적할 이슈를 생성할까요?
Projects/Features/HomeFeature/Sources/Models/HomePresentationModel.swift (1)
42-60:PopularTrip과RecommendedTrip의 필드가 동일합니다.두 struct의 프로퍼티가 완전히 동일합니다(필드 순서만 다름). 현재 단계에서는 별도 타입으로 유지하는 것이 확장성을 위해 합리적일 수 있지만, 향후 분화되지 않는다면 하나의 타입으로 통합하는 것을 고려해보세요.
Projects/Features/PopularTravelFeature/Sources/PopularTravelBuilder.swift (1)
13-15:PopularTravelDependency가homeUsecase에 의존하고 있습니다.PopularTravel 피처가
homeUsecase라는 이름의 의존성을 사용하는 것은 모듈 간 결합도 측면에서 혼란을 줄 수 있습니다. 동일한 usecase를 공유하는 것이 의도적이라면tripUsecase등 보다 범용적인 이름으로 리네이밍하거나, PopularTravel 전용 usecase를 분리하는 것을 고려해보세요.Projects/Features/MainFeature/Sources/MainViewController.swift (2)
59-66:setupDelegate()가UIGestureRecognizerDelegateextension 내에private으로 선언되어 있습니다.
setupDelegate()는 UI 초기화 로직이므로 기존private extension MainViewController(Line 53-57)에 위치하는 것이 더 자연스럽습니다. delegate 설정과 delegate 메서드 구현을 분리하면 응집도가 높아집니다.♻️ 제안
private extension MainViewController { func setStyle() { self.view.backgroundColor = DSKitAsset.Colors.white.color } + + func setupDelegate() { + self.interactivePopGestureRecognizer?.delegate = self + } } extension MainViewController: UIGestureRecognizerDelegate { - private func setupDelegate() { - self.interactivePopGestureRecognizer?.delegate = self - } - public func gestureRecognizerShouldBegin(_ gestureRecognizer: UIGestureRecognizer) -> Bool { viewControllers.count > 1 } }
15-17:MainPresentableListener프로토콜이 비어 있습니다.현재 아무 메서드도 정의되어 있지 않습니다. 향후 MainInteractor와의 통신이 필요한 경우 메서드가 추가될 예정인지 확인이 필요합니다. 의도적으로 비워둔 것이라면 TODO 주석을 추가하는 것을 권장합니다.
Projects/Features/PopularTravelFeature/Sources/PopularTravelInteractor.swift (1)
89-94: 로딩 상태 신호가 중복됩니다.
setupStream의homeDataRelay.map { $0 == nil }구독(Line 89-94)과fetchData()내의presenter.setLoading(true)직접 호출(Line 100)이 동시에 로딩 상태를 설정합니다. 하나의 소스로 통합하면 상태 관리가 더 명확해집니다.Also applies to: 100-101
Projects/Features/MainFeature/Sources/MainBuilder.swift (1)
24-40:followService와travelService가 접근할 때마다 새 인스턴스를 생성합니다.computed property로 구현되어 있어 매번
makeFollowService()/makeTravelService()를 호출합니다. 특히travelService는MoyaProvider와 인증 토큰 설정을 포함하므로, 반복 생성이 비효율적일 수 있습니다. RIBs Component에서는shared패턴을 사용하여 자식 scope 내에서 싱글톤으로 유지하는 것이 일반적입니다.♻️ `shared`를 활용한 인스턴스 공유
-var followService: FollowServiceProtocol { - makeFollowService() -} +var followService: FollowServiceProtocol { + shared { makeFollowService() } +} -var travelService: TravelServiceProtocol { - makeTravelService(tokenProvider: tokenProvider) -} +var travelService: TravelServiceProtocol { + shared { makeTravelService(tokenProvider: tokenProvider) } +}Projects/Features/HomeFeature/Sources/HomeViewController.swift (2)
240-261:supplementaryViewProvider의 fallback으로 빈UICollectionReusableView()를 반환합니다.Line 242에서
HomeSectionKind변환 실패 시 크기가 0인 빈 reusable view를 반환합니다. 이 경우 예상치 못한 레이아웃 문제가 발생할 수 있습니다.nil을 반환하거나assertionFailure를 추가하여 개발 중 문제를 감지하는 것을 권장합니다.
70-148: HomeViewController와 PopularTravelViewController 간 UI 패턴 중복이 상당합니다.
setStyle,setUI,setLayout,bindInteractor,applySnapshot, loading/error 처리 등의 패턴이 거의 동일합니다. 향후 유지보수를 위해 공통 base class 또는 protocol extension으로 추출하는 것을 고려해 볼 수 있습니다.Also applies to: 150-194, 197-261
Projects/Features/HomeFeature/Sources/HomeInteractor.swift (2)
135-138:catch let error에서error가 사용되지 않습니다.에러 로깅이나 디버깅을 위해 사용하지 않는다면
catch만으로 충분합니다. 향후 에러 타입별 처리나 로깅 추가를 고려하세요.♻️ 제안
- } catch let error { + } catch {
159-169:itemSelected의default: break는.bannercase를 무시합니다.현재는 의도적이겠지만, 향후 새로운
HomeItemcase가 추가될 때 컴파일러의 exhaustive switch 경고를 받지 못합니다. 명시적으로.banner를 나열하면 새 case 추가 시 누락을 방지할 수 있습니다.♻️ 제안
func itemSelected(item: HomeItem) { switch item { case .category(let category, _): selectedCategoryRelay.accept(category.id) case .popularTrip(let trip): listener?.homeDidTapFollowDetail(with: Int(trip.id) ?? 0) case .recommendedTrip(let trip): listener?.homeDidTapFollowDetail(with: Int(trip.id) ?? 0) - default: break + case .banner: + break } }Projects/Features/HomeFeature/Sources/Views/CollectionView/Registration/HomeRegistration.swift (1)
55-55: SwiftLint 경고: 콤마 뒤 공백 및 미사용 클로저 파라미터SwiftLint에서 콤마 스페이싱과 미사용 클로저 파라미터 경고가 발생합니다. 사용하지 않는 파라미터는
_로 교체하고, 콤마 뒤에 공백을 추가해 주세요.♻️ 수정 제안
- return UICollectionView.SupplementaryRegistration<HomeHeaderView>(elementKind: UICollectionView.elementKindSectionHeader) { headerView,elementKind,indexPath in + return UICollectionView.SupplementaryRegistration<HomeHeaderView>(elementKind: UICollectionView.elementKindSectionHeader) { headerView, _, indexPath in- return UICollectionView.SupplementaryRegistration<HomeFooterButtonView>(elementKind: UICollectionView.elementKindSectionFooter) { [weak self] footerView,elementKind,indexPath in + return UICollectionView.SupplementaryRegistration<HomeFooterButtonView>(elementKind: UICollectionView.elementKindSectionFooter) { [weak self] footerView, _, _ in
CellRegistration클로저에서도indexPath를 사용하지 않는 경우_로 교체해 주세요.Also applies to: 63-63
Projects/Domain/Sources/UseCase/HomeUsecase.swift (1)
19-34: 프로토콜 기본 구현에서 무한 재귀 위험이 패턴은 Swift에서 프로토콜 메서드에 기본 파라미터를 제공하는 일반적인 방법이지만, 만약
HomeUsecaseProtocol을 채택하는 타입이 필수 메서드(fetchPopularTripList(id:page:size:)등)를 직접 구현하지 않으면, 이 기본 구현이 자기 자신을 호출하여 무한 재귀 → 스택 오버플로가 발생합니다.현재
HomeUsecase는 모든 메서드를 구현하고 있어 안전하지만, 향후 다른 채택자(예: Mock, 테스트 더블)에서 실수할 여지가 있습니다. 주석으로 경고를 남겨두는 것을 권장합니다.💡 주석 추가 제안
public extension HomeUsecaseProtocol { + // ⚠️ 채택 타입은 반드시 프로토콜 필수 메서드를 직접 구현해야 합니다. + // 구현하지 않으면 이 기본 구현이 자기 자신을 호출하여 무한 재귀가 발생합니다. func fetchPopularTripList(Projects/Features/MainFeature/Sources/MainRouter.swift (2)
64-141: attach/detach 패턴이 반복되고 있습니다 — 제네릭 헬퍼 추출 고려
attachFollow,attachPopularTravel,attachSearch,attachSetting은 동일한 패턴(guard nil → build → assign → attachChild → push)을 따르고, detach도 마찬가지(guard let → containsInStack → pop → detachChild → nil)입니다. 공통 로직을 헬퍼 메서드로 추출하면 중복을 줄이고 유지보수가 쉬워집니다.💡 예시: 공통 detach 헬퍼
private func detachChildRouter<R: ViewableRouting>( _ routerRef: inout R?, animated: Bool = true ) { guard let router = routerRef else { return } if viewController.containsInStack(router.viewControllable) { viewController.popRootViewController(animated: animated) } detachChild(router) routerRef = nil }
22-27:popRootViewController메서드명이 실제 동작과 일치하지 않습니다.UIKit의
popToRootViewController(animated:)컨벤션과 달리, 이 메서드는 실제로는 단일 pop 동작을 수행합니다 (self.popViewController(animated: animated)). 메서드명은 root까지 pop하는 동작을 암시하지만, 실제 구현은 스택에서 마지막 뷰 컨트롤러 하나만 제거하므로 혼동의 여지가 있습니다. 메서드명을popLastViewController또는 유사한 이름으로 변경하여 실제 동작을 명확히 해주세요.
| public func accessToken() -> String? { | ||
| tokenRepository.get(.accessToken) | ||
| // tokenRepository.get(.accessToken) | ||
| "eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiJiYmE3ODIwYS0wMDUzLTQxZDctODdhYi00Zjk2ZWM3ZDI1MTMiLCJpYXQiOjE3NzA3OTUwMjYsImV4cCI6MTc3MDg4MTQyNn0.oNCkotV0uA-3kCtTwGhTUwA9fUUhuO85p1k3952oTfRaULOw2Ix3RpXq_ta82ynmUK7F3i8F0Jb1d4_Rl-zcgA" | ||
| } |
There was a problem hiding this comment.
하드코딩된 JWT 토큰은 보안 위험입니다.
테스트 목적이라 하더라도, 커밋된 JWT는 Git 히스토리에 영구적으로 남게 됩니다. 해당 토큰을 즉시 무효화(revoke)하고, 실제 토큰 조회 로직을 복원해 주세요. 테스트 시에는 환경 변수나 별도 설정 파일(.gitignore 처리)을 사용하는 것이 좋습니다.
🔒 제안: 원래 로직 복원
public func accessToken() -> String? {
-// tokenRepository.get(.accessToken)
- "eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiJiYmE3ODIwYS0wMDUzLTQxZDctODdhYi00Zjk2ZWM3ZDI1MTMiLCJpYXQiOjE3NzA3OTUwMjYsImV4cCI6MTc3MDg4MTQyNn0.oNCkotV0uA-3kCtTwGhTUwA9fUUhuO85p1k3952oTfRaULOw2Ix3RpXq_ta82ynmUK7F3i8F0Jb1d4_Rl-zcgA"
+ tokenRepository.get(.accessToken)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public func accessToken() -> String? { | |
| tokenRepository.get(.accessToken) | |
| // tokenRepository.get(.accessToken) | |
| "eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiJiYmE3ODIwYS0wMDUzLTQxZDctODdhYi00Zjk2ZWM3ZDI1MTMiLCJpYXQiOjE3NzA3OTUwMjYsImV4cCI6MTc3MDg4MTQyNn0.oNCkotV0uA-3kCtTwGhTUwA9fUUhuO85p1k3952oTfRaULOw2Ix3RpXq_ta82ynmUK7F3i8F0Jb1d4_Rl-zcgA" | |
| } | |
| public func accessToken() -> String? { | |
| tokenRepository.get(.accessToken) | |
| } |
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 22-22: Uncovered a JSON Web Token, which may lead to unauthorized access to web applications and sensitive user data.
(jwt)
🤖 Prompt for AI Agents
In `@Projects/Data/Sources/Adapter/TokenProviderAdapter.swift` around lines 20 -
23, The accessToken() function currently returns a hardcoded JWT string; restore
the original token retrieval by calling the token repository (uncomment or
reimplement tokenRepository.get(.accessToken)) in
TokenProviderAdapter.accessToken(), remove the embedded token from source, and
ensure fallback/optional handling remains (return String?); for tests, switch to
reading the token from an environment variable or a gitignored config and update
any callers of accessToken() if needed.
There was a problem hiding this comment.
이부분은 코드에 있는게 아니라 임시로 했더라면 xcconfig에서 가져왔으면 더 좋을 것 같습니다.
There was a problem hiding this comment.
@KimNahun
PR description image: none provided.
Comment image: none provided.
의견 감사합니다. 말씀 주신 대로 코드는 즉시 원복하고, 로컬 임시 토큰이 필요하면 xcconfig를 통해 주입하는 흐름으로 처리하겠습니다.
- 즉시 조치 — 하드코딩 제거(파일: Projects/Data/Sources/Adapter/TokenProviderAdapter.swift)
- Line 근처: accessToken()
- 변경안:
public func accessToken() -> String? {
tokenRepository.get(.accessToken)
}- 선택 사항(권장) — DEBUG 전용 xcconfig fallback
- 운영(Release)과 테스트/개발(Debug)을 분리해, 저장소나 바이너리에 비밀값을 남기지 않도록 합니다.
- TokenRepository(또는 현 Adapter)에서 저장소에 토큰이 없을 때, DEBUG 빌드에 한해 Info.plist 값을 읽는 fallback을 둡니다.
예시(Adapter에 DEBUG 전용 fallback 추가):
public func accessToken() -> String? {
if let token = tokenRepository.get(.accessToken) {
return token
}
`#if` DEBUG
if let token = Bundle.main.object(forInfoDictionaryKey: "ACCESS_TOKEN") as? String, !token.isEmpty {
return token
}
`#endif`
return nil
}설정 방법
- .gitignore에 비밀 xcconfig를 무시:
- 예: Configurations/Debug.secrets.xcconfig, 혹은 **/secrets.xcconfig
- Configurations/Debug.secrets.xcconfig(커밋 금지, 로컬 전용):
- ACCESS_TOKEN = your_dev_token_here
- Configurations/Debug.xcconfig에 include:
#include? "Debug.secrets.xcconfig"
- Target의 Debug Info.plist에 키 추가:
- Key: ACCESS_TOKEN, Value: $(ACCESS_TOKEN) (Debug 설정에만 적용)
- Release는 위 키를 정의하지 않거나 빈 값으로 유지하여, 배포물에 비밀이 포함되지 않게 합니다.
- 보안 후속 조치
- 이미 커밋된 JWT는 즉시 서버 측에서 폐기/로테이션 부탁드립니다.
- (필요 시) PR 브랜치 히스토리에서 해당 토큰 문자열을 정리(BFG/git filter-repo 등)하는 것도 권장드립니다.
바로 원복(1번)만 이 PR에 반영하고, DEBUG fallback(2번)은 원하시면 이번 PR에 포함하거나 별도 PR로 진행하겠습니다. 어떤 방식으로 진행할지 알려주세요!
✏️ Learnings added
Learnt from: KimNahun
Repo: YAPP-Github/27th-App-Team-1-iOS PR: 24
File: Projects/Data/Sources/Adapter/TokenProviderAdapter.swift:20-23
Timestamp: 2026-02-13T11:24:52.093Z
Learning: 팀 선호: 임시/로컬 테스트용 비밀값은 코드에 하드코딩하지 않고, gitignore된 xcconfig를 통해 Info.plist로 주입한 뒤 DEBUG에서만 읽는다.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| extension TripResponse { | ||
| public func toDomain() -> [TripInfo] { | ||
| self.content.map { | ||
| .init( | ||
| id: $0.travelId, | ||
| title: $0.title, | ||
| thumbnailUrl: $0.thumbnail ?? "", | ||
| creator: $0.programName, | ||
| country: $0.country, | ||
| city: $0.city, | ||
| nights: $0.nights, | ||
| days: $0.days, | ||
| countryCode: $0.country | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the TripResponse model definition
find . -name "*.swift" -type f | xargs grep -l "struct TripResponse" | head -5Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 138
🏁 Script executed:
# Search for countryCode and country-related fields in Swift files
rg -n "countryCode|country_code|countryIso" --type=swift -C 3Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 1146
🏁 Script executed:
# Look at the TripResponse structure to understand the available fields
rg -n "struct TripResponse|class TripResponse" --type=swift -A 20 | head -100Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 1554
🏁 Script executed:
# Search for country code conversion or mapping logic
rg -n "countryCode|country.*code|CountryCode|Country.*Code" --type=swift -B2 -A2 | head -80Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 844
🏁 Script executed:
# Check if there are any utility functions for country code conversion
rg -n "func.*country|country.*func" --type=swift -i -B1 -A3 | head -60Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 544
🏁 Script executed:
# Look for any test files related to TripResponse transformation
find . -name "*TripResponse*Test*" -o -name "*Test*TripResponse*" | head -5Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 57
🏁 Script executed:
# Read the complete String+.swift file to see all country-related extensions
cat Projects/Core/Sources/Extensions/Foundation+/String+.swiftRepository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 912
🏁 Script executed:
# Search for any reverse conversion (name to code) or country code mapping
rg -n "country.*Code|countryCode.*=|country_code" --type=swift -B3 -A3Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 1146
🏁 Script executed:
# Search for test files, mocks, or sample data that shows what the country field contains
find . -type f \( -name "*Mock*" -o -name "*Test*" -o -name "*Stub*" \) -exec grep -l "country\|TripResponse\|TripContent" {} \; | head -10Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 151
🏁 Script executed:
# Look for JSON files or API response examples
find . -type f \( -name "*.json" -o -name "*.txt" \) -exec grep -l "country\|travelId\|programName" {} \; | head -10Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 57
🏁 Script executed:
# Check the mock file to see what data it uses
cat Projects/Features/FollowFeature/Sources/Mock/MockFollowDetailService.swift | head -100Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 3961
🏁 Script executed:
# Look for any API documentation, comments, or README that describes the response format
find . -type f \( -name "README*" -o -name "*.md" \) -exec grep -l "country\|API\|response" {} \; | head -10Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 57
🏁 Script executed:
# Search for any comments near the TripResponse or TripContentResponse definition
rg -n "TripContentResponse|TripResponse" --type=swift -B5 -A30 | grep -A30 "struct TripContentResponse"Repository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 2506
countryCode에 국가 이름이 직접 매핑되고 있습니다. ISO 코드로의 변환이 필요합니다.
Line 26에서 countryCode: $0.country로 매핑하고 있으며, 실제 데이터에는 country 필드에 국가명(예: "태국")이 저장됩니다. 하지만 String.toKoreanCountryName(), String.toFlag() 등의 확장 메서드들은 2자리 ISO 코드(예: "TH")를 기대하고 있습니다.
countryCode 필드에는 ISO 3166-1 alpha-2 코드가 매핑되어야 하므로, 국가명을 ISO 코드로 변환하는 로직을 추가하거나 DTO에 실제 코드 필드가 있는지 확인하세요.
🤖 Prompt for AI Agents
In `@Projects/Data/Sources/Transform/TripResponse`+.swift around lines 14 - 29, In
TripResponse.toDomain() the countryCode is incorrectly set to the country name
(countryCode: $0.country) which must be an ISO 3166-1 alpha-2 code; update the
mapping in TripResponse.toDomain() to convert the incoming country name to its
2-letter ISO code (or use the DTO's ISO field if available) by calling the
conversion helper (e.g., a new/existing countryNameToISO or similar extension)
before assigning to countryCode so downstream extensions like
String.toKoreanCountryName()/toFlag() receive the expected ISO code.
| var headerTitle: String { | ||
| switch self { | ||
| case .category: | ||
| "인기 여행 따라가기" | ||
| case .recommendedTrip: | ||
| "나혜주님께 추천하는\n따라가기 여행 콘텐츠에요!" | ||
| default: "" | ||
| } | ||
| } |
There was a problem hiding this comment.
하드코딩된 사용자 이름 "나혜주님"을 동적으로 변경해야 합니다.
recommendedTrip의 headerTitle에 사용자 이름이 하드코딩되어 있습니다. 실제 로그인한 사용자 이름을 반영하도록 변경하거나, 최소한 TODO 주석을 남겨 추후 수정이 필요함을 표시해 주세요.
예시 리팩터링
- var headerTitle: String {
+ func headerTitle(userName: String? = nil) -> String {
switch self {
case .category:
- "인기 여행 따라가기"
+ return "인기 여행 따라가기"
case .recommendedTrip:
- "나혜주님께 추천하는\n따라가기 여행 콘텐츠에요!"
- default: ""
+ return "\(userName ?? "회원")님께 추천하는\n따라가기 여행 콘텐츠에요!"
+ default:
+ return ""
}
}🤖 Prompt for AI Agents
In
`@Projects/Features/HomeFeature/Sources/Views/CollectionView/SectionKind/HomeSectionKind.swift`
around lines 17 - 25, The headerTitle getter in HomeSectionKind currently
hardcodes a user name for the recommendedTrip case; update it so the string uses
a dynamic user name (e.g., inject or fetch the current user's displayName)
instead of the literal "나혜주님" by changing the recommendedTrip branch in var
headerTitle to format with the runtime user name (or call a helper like
UserManager.current.name) and ensure localization/escaping as needed; if you
cannot wire the user model right now, replace the literal with a clear TODO
comment in HomeSectionKind::headerTitle indicating the value must be replaced
with the logged-in user's name and reference the recommendedTrip case.
| func tableView(_ tableView: UITableView, didSelectRowAt indexPath: IndexPath) { | ||
| tableView.deselectRow(at: indexPath, animated: true) | ||
| let menu = SettingSection.allCases[indexPath.section].items[indexPath.row] | ||
|
|
||
| switch menu.cellType { | ||
| case .toggle, .icon: | ||
| listener?.didTapMenu(item: menu) | ||
| default: | ||
| break | ||
| } | ||
| } |
There was a problem hiding this comment.
.toggle 셀 선택 처리가 실제로는 동작하지 않습니다.
SettingMenuCell에서 이미 지적한 바와 같이, UISwitch가 터치 이벤트를 가로채기 때문에 .toggle 타입 셀의 didSelectRowAt은 호출되지 않습니다. 여기서 .toggle 케이스를 처리하는 것은 dead code에 해당합니다. 실제 토글 이벤트는 UISwitch의 .valueChanged 핸들러를 통해 처리해야 합니다.
🤖 Prompt for AI Agents
In `@Projects/Features/SettingFeature/Sources/SettingViewController.swift` around
lines 127 - 137, The .toggle branch in tableView(_:didSelectRowAt:) is dead
because the UISwitch in SettingMenuCell intercepts taps; remove or stop relying
on the .toggle handling there and instead handle toggle events in the UISwitch
.valueChanged handler inside SettingMenuCell (or via a delegate/closure exposed
by SettingMenuCell) and call listener?.didTapMenu(item:) (or a new
listener?.didToggleMenu(item:isOn:)) from that handler; update SettingMenuCell
to expose a switchValueChanged callback (or delegate method) and wire it in
SettingViewController when configuring cells, keeping
tableView(_:didSelectRowAt:) only for selectable types like .icon.
| case .toggle(let isOn): | ||
| toggleSwitch.isHidden = false | ||
| toggleSwitch.isOn = isOn | ||
| self.selectionStyle = .none |
There was a problem hiding this comment.
UISwitch 터치 이벤트가 didSelectRowAt에 전달되지 않습니다.
UISwitch는 터치 이벤트를 직접 소비하므로, 사용자가 스위치를 탭하면 tableView(_:didSelectRowAt:)가 호출되지 않습니다. 따라서 listener?.didTapMenu(item:)가 트리거되지 않고, 스위치의 시각적 상태만 변경되며 실제 로직은 실행되지 않습니다.
UISwitch에 .valueChanged 액션 핸들러를 추가하여 토글 이벤트를 처리해야 합니다.
🤖 Prompt for AI Agents
In
`@Projects/Features/SettingFeature/Sources/UITableView/Cells/SettingMenuCell.swift`
around lines 56 - 59, The toggle switch currently only updates its visual state
in SettingMenuCell (case .toggle) so taps never trigger
tableView(_:didSelectRowAt:); add a UIControl.Event.valueChanged target-action
to toggleSwitch (e.g., in the cell initializer or in configureForMenuItem) that
calls a handler on the cell which in turn invokes listener?.didTapMenu(item:
currentItem) (or the appropriate method) to perform the toggle logic; ensure the
handler reads toggleSwitch.isOn and uses the cell's associated menu item (stored
property) to notify the listener, and keep selectionStyle = .none and
toggleSwitch.isHidden logic as-is.
| if (200...299).contains(response.statusCode) { | ||
| do { | ||
| let decodedData = try response.map(BaseResponse<T>.self) | ||
|
|
||
| if let data = decodedData.data { | ||
| continuation.resume(returning: data) | ||
| } else { | ||
| continuation.resume(throwing: NetworkError.noData) | ||
| } | ||
| } catch { | ||
| continuation.resume(throwing: NetworkError.decodingFailed) | ||
| } | ||
| } else { | ||
| do { | ||
| let errorResponse = try response.map(ErrorResponse.self) | ||
| continuation.resume( | ||
| throwing: NetworkError.unknown( | ||
| errorResponse.message ?? "알 수 없는 오류가 발생했습니다." | ||
| ) | ||
| ) | ||
| } catch { | ||
| continuation.resume(throwing: NetworkError.unknown("알 수 없는 오류가 발생했습니다.")) | ||
| } | ||
| } |
There was a problem hiding this comment.
기존 메서드와 성공 판단 로직이 불일치합니다.
기존 request 메서드는 baseResponse.code == "2000"으로 성공을 판별하지만, 이 메서드는 HTTP status code 200...299를 사용합니다. 서버가 HTTP 200을 반환하면서 비즈니스 에러 코드를 담을 경우, 이 메서드는 에러를 감지하지 못하고 noData로 잘못 처리할 수 있습니다.
의도적인 설계라면 주석으로 명시해 주시고, 그렇지 않다면 기존 패턴과 통일하는 것을 권장합니다.
🤖 Prompt for AI Agents
In `@Projects/Modules/Networks/Sources/Extensions/MoyaProvider`+Async.swift around
lines 117 - 140, The success check currently uses HTTP status (200...299) but
your other `request` method treats success by inspecting `baseResponse.code ==
"2000"`, causing inconsistent behavior; update this async handler (in
MoyaProvider+Async.swift) to first decode the response into BaseResponse<T>
(using response.map(BaseResponse<T>.self)), then check `baseResponse.code ==
"2000"`: if true, return `baseResponse.data` or throw `NetworkError.noData`; if
false, throw `NetworkError.unknown(baseResponse.message ?? "알 수 없는 오류가
발생했습니다.")`; keep the ErrorResponse fallback only if BaseResponse decoding fails
and use `response.map(ErrorResponse.self)` to extract message, and ensure all
continuation.resume calls are used consistently (resume(returning:) or
resume(throwing:)).
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@Projects/Data/Sources/Repository/Home/HomeRepository.swift`:
- Around line 21-23: fetchMyTripInfo currently returns hardcoded sample data
(MyTripSummary and Schedule) which must be removed; replace the temporary return
with a proper not-implemented/error path or real API call and add a TODO comment
and link to a tracking issue so this can't be shipped accidentally.
Specifically, update the fetchMyTripInfo() implementation to either call the
intended network/data source method (or throw a clear unimplemented error)
instead of returning MyTripSummary(...) and add a TODO + issue ID comment above
the function referencing the tracking ticket.
In `@Projects/Features/HomeFeature/Sources/HomeInteractor.swift`:
- Around line 159-169: In itemSelected(_:), avoid using Int(trip.id) ?? 0 which
can pass an invalid 0 ID; for the .popularTrip and .recommendedTrip cases, parse
trip.id with a guard-style check and early-return on failure (or otherwise
handle the error) before calling listener?.homeDidTapFollowDetail(with:), e.g.
convert trip.id to Int into a non-optional intId and only call
homeDidTapFollowDetail(with:) when conversion succeeds; alternatively consider
changing the domain model so trip.id is an Int to eliminate the conversion.
In `@Projects/Features/HomeFeature/Sources/HomeViewController.swift`:
- Around line 204-231: The pattern match for HomeItem.category in the
UICollectionViewDiffableDataSource closure binds only one associated value and
discards the isSelected flag; update the case from case .category(let category)
to explicitly include the second associated value (e.g., case .category(let
category, _)) so both associated values are handled consistently with
HomeInteractor.itemSelected and to make the intent explicit.
In `@Projects/Modules/Networks/Sources/Extensions/MoyaProvider`+Async.swift:
- Line 109: The generic function asyncThowsRequest<T: Decodable>(_ target:
Target) async throws -> T is missing the Sendable constraint; update its generic
bound to T: Decodable & Sendable to match the other methods (request,
requestPlain) because the implementation uses withCheckedThrowingContinuation
and crosses a concurrency boundary, which requires Sendable conformance for safe
usage. Ensure the function signature for asyncThowsRequest uses the combined
constraint T: Decodable & Sendable.
- Line 109: The method name has a typo: rename the function asyncThowsRequest<T:
Decodable>(_ target: Target) async throws -> T to asyncThrowsRequest<T:
Decodable>(_ target: Target) async throws -> T and update all call sites and
references accordingly (e.g., any usages of asyncThowsRequest in
MoyaProvider+Async.swift and other modules); keep the signature, generics,
throws/async behavior and visibility identical, and search for the exact symbol
asyncThowsRequest to replace it so public API is corrected consistently.
- Around line 126-135: The current decode block maps response to BaseResponse<T>
but only checks baseResponse.data; update it to also validate the business code
(BaseResponse<T>.code) matches the success code used elsewhere (e.g., "2000")
and if it does not, resume the continuation with an appropriate business/server
error from NetworkError (e.g., NetworkError.businessError or
NetworkError.serverError with the code/message) instead of treating it as
noData; locate the logic around BaseResponse<T> and continuation.resume in the
async extension (MoyaProvider+Async.swift) and add this code-path so
unsuccessful business codes produce the correct NetworkError.
🧹 Nitpick comments (9)
Projects/Features/SearchFeature/Sources/SearchViewController.swift (2)
20-20: SwiftLint 포맷팅 경고 수정 필요SwiftLint에서 두 가지 경고가 발생합니다: 쉼표 뒤 이중 공백, 그리고 여는 중괄호 앞 공백 누락.
🔧 포맷 수정 제안
-final class SearchViewController: UIViewController, SearchPresentable, SearchViewControllable{ +final class SearchViewController: UIViewController, SearchPresentable, SearchViewControllable {
100-127: 키보드 애니메이션 duration을 notification에서 읽는 것을 권장합니다.현재
0.3으로 하드코딩되어 있지만, 시스템이 제공하는 실제 애니메이션 duration과 curve를userInfo에서 읽으면 시스템 애니메이션과 정확히 동기화됩니다.keyboardWillHideNotification구독에서도 동일하게 적용 가능합니다.♻️ 리팩토링 제안 (keyboardWillShow 예시)
NotificationCenter.default.rx.notification(UIResponder.keyboardWillShowNotification) .subscribe(onNext: { [weak self] notification in guard let keyboardFrame = notification.userInfo?[UIResponder.keyboardFrameEndUserInfoKey] as? NSValue else { return } let keyboardHeight = keyboardFrame.cgRectValue.height + let duration = notification.userInfo?[UIResponder.keyboardAnimationDurationUserInfoKey] as? Double ?? 0.3 self?.containerView.snp.updateConstraints { $0.bottom.equalToSuperview().inset(keyboardHeight) } - UIView.animate(withDuration: 0.3) { + UIView.animate(withDuration: duration) { self?.view.layoutIfNeeded() } })Projects/Features/PopularTravelFeature/Sources/PopularTravelInteractor.swift (3)
71-95: 로딩 상태 관리가 이중으로 되어 있습니다.
setupStream()의popularTravelDataRelay.map { $0 == nil }구독(Line 89-94)과fetchData()내 수동setLoading호출(Line 100, 122, 124)이 동시에 로딩 상태를 제어하고 있습니다. 초기 로드에서는 양쪽 모두 동작하고, 리로드 시에는 수동 호출만 동작합니다.현재 기능적 문제는 없지만, 향후 유지보수 시 혼란을 줄 수 있으니 하나의 방식으로 통일하는 것을 고려해 주세요.
HomeInteractor도 동일한 패턴이므로 함께 정리하면 좋겠습니다.
76-82: SwiftLint: 클로저에서 implicit return 사용을 권장합니다.
return키워드를 제거하여 SwiftLintimplicit_return규칙을 충족할 수 있습니다.♻️ 제안
.map { model, selectedId -> [PopularTravelSectionModel] in - return [ + [ .init(section: .category, items: model.category.map { .category($0, isSelected: $0.id == selectedId) }), .init(section: .popularTrip, items: model.popularTrip.map { .popularTrip($0) }) ] }
140-147: 도메인 모델 계층에서 타입 정의 정규화 권장
trip.id가 String 타입으로 정의되어 있어 Interactor에서Int(trip.id) ?? 0변환이 필요합니다. 변환 실패 시 0이 자동으로 전달되면 의도하지 않은 데이터 요청이 발생할 수 있습니다.
TripResponse+.swift매핑 계층에서 API의 문자열 ID를 정수로 변환하거나,TripInfo도메인 모델의id를Int타입으로 정의하여 변환 시점을 앞당기면 Interactor 단계에서 불필요한 변환과 암묵적 오류 처리를 제거할 수 있습니다. 동일한 패턴이HomeInteractor에서도 두 번 사용되고 있습니다.Projects/Data/Sources/Repository/Home/HomeRepository.swift (1)
25-47: 반복되는 do/catch 에러 매핑 패턴을 헬퍼로 추출할 수 있습니다.
fetchCategoryList,fetchPopularTripList,fetchRecommendTripList세 메서드 모두 동일한do { ... } catch { throw error.toNDGLError() }패턴을 사용하고 있습니다. 헬퍼 함수로 추출하면 중복을 줄일 수 있습니다.♻️ 에러 매핑 헬퍼 제안
+ private func withNDGLErrorMapping<T>(_ operation: () async throws -> T) async throws -> T { + do { + return try await operation() + } catch { + throw error.toNDGLError() + } + } + public func fetchCategoryList() async throws -> [TripCategory] { - do { - return try await homeService.getCategoryList().map { $0.toDomain() } - } catch { - throw error.toNDGLError() - } + try await withNDGLErrorMapping { + try await homeService.getCategoryList().map { $0.toDomain() } + } }Projects/Features/HomeFeature/Sources/HomeInteractor.swift (2)
137-140: 에러가 조용히 무시되고 있습니다.
catch블록에서 에러를 로깅하거나 사용자에게 구체적인 에러 메시지를 전달하지 않고 있습니다. 디버깅이 어려워질 수 있으므로 최소한 로깅을 추가하는 것을 권장합니다.♻️ 에러 로깅 추가 제안
- } catch let error { + } catch { + print("❌ [HomeInteractor] fetchHomeData failed: \(error)") presenter.setLoading(false) presenter.showErrorView(true) }
99-104:homeDataRelay기반 로딩 스트림이fetchHomeData()의 직접 호출과 중복됩니다.
setupStream에서homeDataRelay.map { $0 == nil }로 로딩 상태를 관리하지만,fetchHomeData()에서도presenter.setLoading(true/false)를 직접 호출합니다.homeDataRelay는 데이터 로드 후 다시nil로 리셋되지 않으므로, 이 스트림은 초기 로드 시에만true를 한 번 emit하고 이후에는 의미가 없습니다. 로딩 상태 관리를 한 곳으로 통일하는 것이 좋습니다.Projects/Features/HomeFeature/Sources/HomeViewController.swift (1)
118-147:CellRegistration/SupplementaryRegistration을 사용하므로 수동 등록이 불필요합니다.
setDataSource()에서UICollectionView.CellRegistration과SupplementaryRegistrationAPI를 사용하고 있는데, 이 API들은 내부적으로 셀/보충 뷰 등록을 자동으로 처리합니다.setCollectionView()의 수동register호출은 중복이며 제거해도 됩니다.
| func itemSelected(item: HomeItem) { | ||
| switch item { | ||
| case .category(let category, _): | ||
| selectedCategoryRelay.accept(category.id) | ||
| case .popularTrip(let trip): | ||
| listener?.homeDidTapFollowDetail(with: Int(trip.id) ?? 0) | ||
| case .recommendedTrip(let trip): | ||
| listener?.homeDidTapFollowDetail(with: Int(trip.id) ?? 0) | ||
| default: break | ||
| } | ||
| } |
There was a problem hiding this comment.
Int(trip.id) ?? 0으로 인해 잘못된 ID가 전달될 수 있습니다.
trip.id가 String 타입인데 Int로 변환 실패 시 0이 기본값으로 전달됩니다. ID 0이 유효하지 않은 경우, 잘못된 상세 화면으로 이동하거나 API 에러가 발생할 수 있습니다. 변환 실패 시 early return하거나, 도메인 모델에서 id를 처음부터 Int로 정의하는 것이 더 안전합니다.
🛡️ guard let 사용 제안
case .popularTrip(let trip):
- listener?.homeDidTapFollowDetail(with: Int(trip.id) ?? 0)
+ guard let id = Int(trip.id) else { return }
+ listener?.homeDidTapFollowDetail(with: id)
case .recommendedTrip(let trip):
- listener?.homeDidTapFollowDetail(with: Int(trip.id) ?? 0)
+ guard let id = Int(trip.id) else { return }
+ listener?.homeDidTapFollowDetail(with: id)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func itemSelected(item: HomeItem) { | |
| switch item { | |
| case .category(let category, _): | |
| selectedCategoryRelay.accept(category.id) | |
| case .popularTrip(let trip): | |
| listener?.homeDidTapFollowDetail(with: Int(trip.id) ?? 0) | |
| case .recommendedTrip(let trip): | |
| listener?.homeDidTapFollowDetail(with: Int(trip.id) ?? 0) | |
| default: break | |
| } | |
| } | |
| func itemSelected(item: HomeItem) { | |
| switch item { | |
| case .category(let category, _): | |
| selectedCategoryRelay.accept(category.id) | |
| case .popularTrip(let trip): | |
| guard let id = Int(trip.id) else { return } | |
| listener?.homeDidTapFollowDetail(with: id) | |
| case .recommendedTrip(let trip): | |
| guard let id = Int(trip.id) else { return } | |
| listener?.homeDidTapFollowDetail(with: id) | |
| default: break | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@Projects/Features/HomeFeature/Sources/HomeInteractor.swift` around lines 159
- 169, In itemSelected(_:), avoid using Int(trip.id) ?? 0 which can pass an
invalid 0 ID; for the .popularTrip and .recommendedTrip cases, parse trip.id
with a guard-style check and early-return on failure (or otherwise handle the
error) before calling listener?.homeDidTapFollowDetail(with:), e.g. convert
trip.id to Int into a non-optional intId and only call
homeDidTapFollowDetail(with:) when conversion succeeds; alternatively consider
changing the domain model so trip.id is an Int to eliminate the conversion.
| } | ||
|
|
||
|
|
||
| func asyncThowsRequest<T: Decodable>(_ target: Target) async throws -> T { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
제네릭 타입 T에 Sendable 제약이 누락되었습니다.
같은 extension 내 다른 메서드들(request, requestPlain)은 T: Decodable & Sendable로 선언되어 있습니다. withCheckedThrowingContinuation을 통해 동시성 경계를 넘기므로 Sendable 준수가 필요합니다. Swift strict concurrency 모드에서 경고/오류가 발생할 수 있습니다.
♻️ 수정 제안
- func asyncThowsRequest<T: Decodable>(_ target: Target) async throws -> T {
+ func asyncThrowsRequest<T: Decodable & Sendable>(_ target: Target) async throws -> T {🤖 Prompt for AI Agents
In `@Projects/Modules/Networks/Sources/Extensions/MoyaProvider`+Async.swift at
line 109, The generic function asyncThowsRequest<T: Decodable>(_ target: Target)
async throws -> T is missing the Sendable constraint; update its generic bound
to T: Decodable & Sendable to match the other methods (request, requestPlain)
because the implementation uses withCheckedThrowingContinuation and crosses a
concurrency boundary, which requires Sendable conformance for safe usage. Ensure
the function signature for asyncThowsRequest uses the combined constraint T:
Decodable & Sendable.
메서드 이름에 오타가 있습니다: asyncThowsRequest → asyncThrowsRequest
r이 빠져 있습니다. 공개 API 이름이므로 수정하지 않으면 사용처에 전파됩니다.
🐛 수정 제안
- func asyncThowsRequest<T: Decodable>(_ target: Target) async throws -> T {
+ func asyncThrowsRequest<T: Decodable>(_ target: Target) async throws -> T {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func asyncThowsRequest<T: Decodable>(_ target: Target) async throws -> T { | |
| func asyncThrowsRequest<T: Decodable>(_ target: Target) async throws -> T { |
🤖 Prompt for AI Agents
In `@Projects/Modules/Networks/Sources/Extensions/MoyaProvider`+Async.swift at
line 109, The method name has a typo: rename the function asyncThowsRequest<T:
Decodable>(_ target: Target) async throws -> T to asyncThrowsRequest<T:
Decodable>(_ target: Target) async throws -> T and update all call sites and
references accordingly (e.g., any usages of asyncThowsRequest in
MoyaProvider+Async.swift and other modules); keep the signature, generics,
throws/async behavior and visibility identical, and search for the exact symbol
asyncThowsRequest to replace it so public API is corrected consistently.
| do { | ||
| let baseResponse = try response.map(BaseResponse<T>.self) | ||
| if let data = baseResponse.data { | ||
| continuation.resume(returning: data) | ||
| } else { | ||
| continuation.resume(throwing: NetworkError.noData) | ||
| } | ||
| } catch { | ||
| continuation.resume(throwing: NetworkError.decodingFailed) | ||
| } |
There was a problem hiding this comment.
HTTP 200 응답에서도 baseResponse.code 검증이 필요합니다.
HTTP status가 성공이더라도 비즈니스 코드가 "2000"이 아닌 경우를 처리하지 않으면, 서버 비즈니스 에러가 정상 데이터 없음(noData)으로 잘못 분류됩니다. 기존 패턴과 통일하는 것을 권장합니다.
♻️ 비즈니스 코드 검증 추가 제안
do {
let baseResponse = try response.map(BaseResponse<T>.self)
- if let data = baseResponse.data {
+ if baseResponse.code == Self.successCode, let data = baseResponse.data {
continuation.resume(returning: data)
+ } else if baseResponse.code != Self.successCode {
+ let errorResponse = try response.map(ErrorResponse.self)
+ continuation.resume(throwing: NetworkError.serverError(errorResponse))
} else {
continuation.resume(throwing: NetworkError.noData)
}🤖 Prompt for AI Agents
In `@Projects/Modules/Networks/Sources/Extensions/MoyaProvider`+Async.swift around
lines 126 - 135, The current decode block maps response to BaseResponse<T> but
only checks baseResponse.data; update it to also validate the business code
(BaseResponse<T>.code) matches the success code used elsewhere (e.g., "2000")
and if it does not, resume the continuation with an appropriate business/server
error from NetworkError (e.g., NetworkError.businessError or
NetworkError.serverError with the code/message) instead of treating it as
noData; locate the logic around BaseResponse<T> and continuation.resume in the
async extension (MoyaProvider+Async.swift) and add this code-path so
unsuccessful business codes produce the correct NetworkError.
| } | ||
|
|
||
| func toKoreanCountryName() -> String { | ||
| guard self.count == 2 else { return "알 수 없음"} |
| import UIKit | ||
|
|
||
| public extension UICollectionReusableView { | ||
| static var reusableViewIdentifier : String { | ||
| return String(describing: self) | ||
| } |
There was a problem hiding this comment.
매번 idenifier 추가로 만들어줬는데 이 부분 있는 것 좋습니다. (생각만 하고 있었는데 👍 )
| public func accessToken() -> String? { | ||
| tokenRepository.get(.accessToken) | ||
| // tokenRepository.get(.accessToken) | ||
| "eyJhbGciOiJIUzUxMiJ9.eyJzdWIiOiJiYmE3ODIwYS0wMDUzLTQxZDctODdhYi00Zjk2ZWM3ZDI1MTMiLCJpYXQiOjE3NzA3OTUwMjYsImV4cCI6MTc3MDg4MTQyNn0.oNCkotV0uA-3kCtTwGhTUwA9fUUhuO85p1k3952oTfRaULOw2Ix3RpXq_ta82ynmUK7F3i8F0Jb1d4_Rl-zcgA" | ||
| } |
There was a problem hiding this comment.
이부분은 코드에 있는게 아니라 임시로 했더라면 xcconfig에서 가져왔으면 더 좋을 것 같습니다.
|
|
||
| func configure(_ model: HomePresentationModel.Banner) { | ||
| [emptyView, upCommingView, onGoingView].forEach { $0.isHidden = true } | ||
|
|
||
| if model.tripSchedule.isEmpty { | ||
| self.type = .empty | ||
| emptyView.isHidden = false | ||
| return | ||
| } | ||
|
|
||
| let now = Date() | ||
| let calendar = Calendar.current | ||
|
|
||
| let startOfToday = calendar.startOfDay(for: now) | ||
| let startOfTravel = calendar.startOfDay(for: model.startDay) | ||
| let startOfEnd = calendar.startOfDay(for: model.endDay) | ||
| let dateRangeString = formatDateRange(start: model.startDay, end: model.endDay) |
There was a problem hiding this comment.
cell에 이런 로직적인 부분을 조금 더 제거하고, toDomain() 이나 인터렉터 같은 곳에서 해당 부분을 처리한 뒤 cell에서는 ui를 그리는 역할에만 더 집중하면 어떨까요
There was a problem hiding this comment.
해당 로직은 추후에 다시 리펙토링 할 예정입니다
| override func prepareForReuse() { | ||
| super.prepareForReuse() | ||
|
|
||
| disposeBag = DisposeBag() |
There was a problem hiding this comment.
재사용될 때 중복 구독을 방지하기 위해서 prepareForeReuse에서 초기화하도록 했습니다
| func setStyle() { | ||
| titleLabel.do { | ||
| $0.numberOfLines = 2 | ||
| } | ||
| } |
There was a problem hiding this comment.
더 읽어보니 전체적으로 setStyle() 함수 안에 실질적으로 선언해주시고, 초기화는 정말 생성만 해두신 것 같습니다.
처음에는 저 부분만 저렇게 된줄 알고 리뷰를 달았었는데 전체적으로 스타일은 통일이 된 것 같아서 좋습니다.
혹시 저렇게 하셨을 때의 장점이 있을까요? 아주 보편적이진 않은 것 같아서요~!
There was a problem hiding this comment.
저는 개인적으로 프로퍼티 생성 부분은 가볍게 가져가고 setStyle() 함수에서 추가적인 설정하는 게 보기 편하다고 생각해서 했습니다!
|
|
||
| final class RecommendInfoCell: UICollectionViewCell { | ||
| static let defaultWidth = 240.adjusted | ||
| static let defaultHeight = 253.adjustedH | ||
|
|
There was a problem hiding this comment.
셀 너비, 높이를 저는 UICollectionViewDelegateFlowLayout으로 지정해줬었는데, 이렇게 셀에 따로 static으로 선언하신 이유도 궁금합니다!
There was a problem hiding this comment.
let itemSize = NSCollectionLayoutSize(
widthDimension: .fractionalWidth(1.0),
heightDimension: .estimated(RecommendInfoCell.defaultHeight)
)
let item = NSCollectionLayoutItem(layoutSize: itemSize)
let groupSize = NSCollectionLayoutSize(
widthDimension: .absolute(RecommendInfoCell.defaultWidth),
heightDimension: .estimated(RecommendInfoCell.defaultHeight)
)위 코드와 같이 compositionalLayout에서 설정 편하게 하기 위해서 선언했습니다.
PopularInfoCell과 같이 다른 모듈에서도 같이 사용하는 경우 사용하는 모듈에서 구체적인 수치를 넣지 않아도
defatulHegith, defaultWidth로 접근하기 쉽게 하려고 했습니다.
|
|
||
| // MARK: - Configure | ||
| func configure(_ model: HomePresentationModel.RecommendedTrip) { | ||
| if let url = URL(string: model.thumbnailUrl) { | ||
| thumbnailView.kf.setImage(with: url, options: [.transition(.fade(0.3))]) | ||
| } |
There was a problem hiding this comment.
혹시 컬렉션뷰 셀에서 이미지 로드할 때 빠르게 스크롤하거나 하면 다른 이미지가 잘못 표시된다거나 할 수 있는 이슈에 대한 전략에 대해 고려해주신 사항이 있을까요?
There was a problem hiding this comment.
prepareForReuse를 통해 작업 취소하는 부분이 누락됐네요...
override public func prepareForReuse() {
super.prepareForReuse()
thumbnailView.kf.cancelDownloadTask()
thumbnailView.image = nil
}이 로직이 들어간다면 대응이 되지 않을까요?
|
|
||
| func prepareForReuse() { | ||
| imageView.kf.cancelDownloadTask() | ||
| titleLabel.text = nil | ||
| dateLabel.text = nil | ||
| dDayLabel.text = nil | ||
| imageView.image = nil | ||
| } | ||
| } |
There was a problem hiding this comment.
따로 prepareForReuse를 만들어주셨네요. uicollectionviewcell같은 경우는 재사용 될텐데, 이 따로 만드신 uiview도 재사용이 되고 있나요?
There was a problem hiding this comment.
해당 뷰들을 HomeBannerCell에서 사용하고 있어서 HomeBannerCell이 재사용될 때 하위로 가지고 있는 뷰들도
초기화 해주기 위해서 넣었습니다.
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@Projects/Data/Sources/Transform/UpcomingResponse`+.swift:
- Around line 19-20: The current use of startDate.toDate() ?? .now and
endDate.toDate() ?? .now silently masks parse failures and can miscompute
D-day/state; update the transformation so parse failures are propagated instead
of defaulting to .now: modify toDomain() (or the surrounding mapper) to return
an Optional or be throws, remove the .now fallbacks when assigning
startDay/endDay (use guard/try to exit/return nil or rethrow on parse failure),
and/or add a clear processLogger.error or similar when toDate() returns nil so
callers can handle invalid input; reference startDate, endDate, toDate(), and
toDomain() when making the change.
In `@Projects/Features/HomeFeature/Sources/HomeInteractor.swift`:
- Around line 126-135: The three concurrent `async let` bindings
(usecase.fetchCategoryList, fetchPopularTripList, fetchRecommendTripList) are
awaited together to build HomePresentationModel so a single failure bubbles up
and fails the entire home screen; change to await each fetch independently with
individual error handling/fallbacks (e.g., do/catch per fetch or use an explicit
Task/TaskGroup that maps failures to empty/default models) so that
HomePresentationModel(banner: myTripBanner, category: <safeCategoryResult>,
popularTrip: <safePopularResult>, recommendedTrip: <safeRecommendResult>) is
constructed even if one call fails; update the code around HomePresentationModel
construction to pass those safe results.
- Around line 79-104: The Rx subscriptions in setupStream() do not guarantee
delivery on the main thread, so UI calls like presenter.update(with:) and
presenter.setLoading(isLoading) can run off the main thread when
homeDataRelay.accept(model) is called from a background Task; fix by inserting
an observe(on: MainScheduler.instance) (RxSwift) before each subscribe in
setupStream() so the combineLatest chain (using homeDataRelay and
selectedCategoryRelay) and the homeDataRelay.map { $0 == nil } chain both
observe on the main scheduler; ensure you reference setupStream(),
homeDataRelay, selectedCategoryRelay, presenter.update(with:),
presenter.setLoading(isLoading), and disposeBag when making the change.
- Around line 107-151: The presenter calls inside fetchHomeData()
(presenter.setLoading and presenter.showErrorView) are executed from a
background Task and must be dispatched to the main actor; update fetchHomeData()
so all UI-related calls (presenter.setLoading(true/false),
presenter.showErrorView(true/false), and any homeDataRelay/selectedCategoryRelay
updates that drive UI) are executed on the main thread—either annotate the Task
body or those call sites with `@MainActor/await` MainActor.run { ... } to ensure
main-thread execution while keeping the existing async flow and error handling
in the Task.
In `@Projects/Features/HomeFeature/Sources/Models/HomePresentationModel.swift`:
- Line 40: There are two typos to fix: rename the property viedoType to
videoType in the HomePresentationModel (and any other files that consume it
across the domain API, e.g., TripCategory related models and view registration
code) and rename the method toPresention() to toPresentation() (ensure
HomeInteractor's call site is updated to call toPresentation()); update all
references/usage sites and any public API signatures so they remain consistent
across the six affected files.
In `@Projects/Features/HomeFeature/Sources/Views/Cells/HomeBannerCell.swift`:
- Around line 63-70: Replace the hardcoded transport icon and label in the
onGoing case by reading the model/schedule transport field instead of always
using DSKitAsset.Assets.icBus2.image and the literal "대중교통": update the
self.type = .onGoing(...) call to derive transportIcon and transport from
model.transport (or schedule.transport) via a small mapping (e.g., enum/switch)
so new transport types render correctly, and add a TODO comment in
HomeBannerCell noting this is a temporary server limitation if only one
transport type is currently returned so future developers know to remove the
workaround.
🧹 Nitpick comments (11)
Projects/Domain/Sources/Model/Home/MyTripSummary.swift (1)
29-52:Schedule이라는 이름이 너무 범용적입니다.
Schedule은 Domain 모듈의 public scope에서 다른 타입과 충돌할 가능성이 있습니다. 예를 들어TripSchedule등 도메인 맥락을 포함한 이름을 고려해 주세요. 임시 모델이므로 API 확정 시 함께 정리해도 괜찮습니다.Projects/Data/Sources/Transform/UpcomingResponse+.swift (1)
33-39:DateFormatter를 매 호출마다 생성하는 대신static인스턴스를 재사용하세요.
DateFormatter생성 비용이 크므로,static let으로 한 번만 생성하여 재사용하는 것이 일반적인 iOS 관례입니다.♻️ 수정 제안
extension String { + private static let yyyyMMddFormatter: DateFormatter = { + let formatter = DateFormatter() + formatter.locale = Locale(identifier: "en_US_POSIX") + formatter.dateFormat = "yyyy-MM-dd" + return formatter + }() + func toDate() -> Date? { - let formatter = DateFormatter() - formatter.locale = Locale(identifier: "en_US_POSIX") - formatter.dateFormat = "yyyy-MM-dd" - return formatter.date(from: self) + return Self.yyyyMMddFormatter.date(from: self) } }Projects/Modules/Networks/Sources/TargetType/HomeAPI.swift (1)
13-19:// MARK: - API 나오기 전 임시주석이 아직 남아 있습니다.실제 API 경로가 이미 정의되어 있으므로, 임시 표시 주석을 제거하거나 업데이트해 주세요.
MyTripSummary.swift와Schedule등 다른 파일에도 동일한 주석이 남아 있습니다.Projects/Features/HomeFeature/Sources/Models/HomePresentationModel.swift (2)
43-61:PopularTrip과RecommendedTrip이 거의 동일한 구조입니다.두 struct의 필드가 동일합니다(
id,title,thumbnailUrl,creator,schedule,country,city). 현재는 분리된 채로 유지해도 괜찮지만, 향후 중복이 관리 부담이 된다면 하나의 타입으로 통합하는 것을 고려해 주세요.
147-154:DateFormatter를 매 호출마다 생성하고 있습니다.
UpcomingResponse+.swift의String.toDate()와 동일한 패턴입니다.static let으로 재사용하는 것을 권장합니다.♻️ 수정 제안
extension Date { + private static let koreanMMddFormatter: DateFormatter = { + let formatter = DateFormatter() + formatter.locale = Locale(identifier: "ko_KR") + formatter.dateFormat = "M월 d일" + return formatter + }() + func toKoreanMMdd() -> String { - let formatter = DateFormatter() - formatter.locale = Locale(identifier: "ko_KR") - formatter.dateFormat = "M월 d일" - return formatter.string(from: self) + return Self.koreanMMddFormatter.string(from: self) } }Projects/Features/HomeFeature/Sources/Views/Cells/HomeBannerCell.swift (1)
50-54:model.id == 0으로 empty 배너를 구분하는 것은 취약합니다.매직 넘버 대신
Optional<Banner>또는 별도 enum을 사용하여 "다가오는 여행 없음" 상태를 명시적으로 표현하는 것이 더 안전합니다. 실제 여행 ID가 0인 경우 의도치 않게 empty로 처리될 수 있습니다.Projects/Features/HomeFeature/Sources/Views/Cells/RecommendInfoCell.swift (1)
59-61: 썸네일 로딩 실패 시 placeholder 이미지가 없습니다.URL이 유효하지 않거나 네트워크 오류 시
systemGray6배경만 보입니다. Kingfisher의placeholder파라미터를 활용하여 기본 이미지를 표시하면 UX가 개선됩니다.Projects/Modules/Networks/Sources/Service/HomeService.swift (1)
13-19: "API 나오기 전 임시" 주석이 아직 남아 있습니다.PR 설명에 따르면 API가 이미 연결된 것으로 보입니다 (
upcoming api 연결커밋 참고). 이 주석이 더 이상 유효하지 않다면 제거하는 것이 좋습니다. 아직 임시 상태라면 트래킹 이슈를 연결해 두세요.Projects/Domain/Sources/UseCase/HomeUsecase.swift (1)
11-17: HomeService와 동일한 "임시" 주석이 남아 있습니다.API가 연결된 상태라면 이 주석을 제거하거나, 아직 변경 예정인 부분이 있다면 구체적으로 무엇이 임시인지 명시해 주세요.
Projects/Features/HomeFeature/Sources/HomeInteractor.swift (2)
145-149:print(error)대신 구조화된 로깅을 사용하세요.프로덕션 코드에서
os_log,NetworkLogger등)를 사용하는 것이 좋습니다.
84-92: SwiftLint: implicit return 경고.Line 85의
return은 클로저에서 암시적 반환이 가능하므로 제거할 수 있습니다. (SwiftLintimplicit_return규칙)♻️ 수정 제안
.map { model, selectedId -> [HomeSectionModel] in - return [ + [ .init(section: .banner, items: [.banner(model.banner)]),
| startDay: self.startDate.toDate() ?? .now, | ||
| endDay: self.endDate.toDate() ?? .now, |
There was a problem hiding this comment.
날짜 파싱 실패 시 .now 폴백은 잘못된 데이터를 조용히 전파합니다.
startDate 또는 endDate 파싱이 실패하면 현재 날짜로 대체되어, HomeBannerCell에서 D-day 계산이 잘못되거나 여행 상태(upcoming/ongoing/empty)가 오판될 수 있습니다. 최소한 로그를 남기거나, toDomain()을 throws/Optional 반환으로 변경하여 호출부에서 적절히 처리하는 것을 권장합니다.
🤖 Prompt for AI Agents
In `@Projects/Data/Sources/Transform/UpcomingResponse`+.swift around lines 19 -
20, The current use of startDate.toDate() ?? .now and endDate.toDate() ?? .now
silently masks parse failures and can miscompute D-day/state; update the
transformation so parse failures are propagated instead of defaulting to .now:
modify toDomain() (or the surrounding mapper) to return an Optional or be
throws, remove the .now fallbacks when assigning startDay/endDay (use guard/try
to exit/return nil or rethrow on parse failure), and/or add a clear
processLogger.error or similar when toDate() returns nil so callers can handle
invalid input; reference startDate, endDate, toDate(), and toDomain() when
making the change.
| private func setupStream() { | ||
| Observable.combineLatest( | ||
| homeDataRelay.compactMap { $0 }, | ||
| selectedCategoryRelay | ||
| ) | ||
| .map { model, selectedId -> [HomeSectionModel] in | ||
| return [ | ||
| .init(section: .banner, items: [.banner(model.banner)]), | ||
| .init(section: .category, items: model.category.map { | ||
| .category($0, isSelected: $0.id == selectedId) | ||
| }), | ||
| .init(section: .popularTrip, items: model.popularTrip.map { .popularTrip($0) }), | ||
| .init(section: .recommendedTrip, items: model.recommendedTrip.map { .recommendedTrip($0) }) | ||
| ] | ||
| } | ||
| .subscribe(with: self) { owner, sections in | ||
| owner.presenter.update(with: sections) | ||
| } | ||
| .disposed(by: disposeBag) | ||
|
|
||
| homeDataRelay | ||
| .map { $0 == nil } | ||
| .subscribe(with: self) { owner, isLoading in | ||
| owner.presenter.setLoading(isLoading) | ||
| } | ||
|
|
||
| async let myTripsResult = homeService.fetchMyTrips() | ||
| async let tripsByCategoryResult = homeService.fetchAllPopularTrips() | ||
| async let recommendationsResult = homeService.fetchRecommendations() | ||
|
|
||
| let (myTripsData, tripsByCategoryData, recommendationsData) = await ( | ||
| (try? myTripsResult.get()) ?? [], | ||
| (try? tripsByCategoryResult.get()) ?? [:], | ||
| (try? recommendationsResult.get()) ?? [] | ||
| ) | ||
|
|
||
| await MainActor.run { | ||
| self.myTrips = myTripsData | ||
| self.tripsByCategory = tripsByCategoryData | ||
| self.recommendations = recommendationsData | ||
| presenter.hideLoading() | ||
| presenter.updateMyTrips(myTripsData) | ||
| presenter.updatePopularTrips(tripsByCategoryData, categories: categories) | ||
| presenter.updateRecommendations(recommendationsData) | ||
| .disposed(by: disposeBag) |
There was a problem hiding this comment.
Rx 구독에서 메인 스레드 보장이 없습니다.
homeDataRelay.accept(model)이 Task 내부(백그라운드 스레드)에서 호출되면, BehaviorRelay의 옵저버도 같은 스레드에서 실행됩니다. presenter.update(with:)는 UI 업데이트이므로 메인 스레드에서 실행되어야 합니다.
🔧 MainScheduler.instance로 observe 추가
.map { model, selectedId -> [HomeSectionModel] in
...
}
+ .observe(on: MainScheduler.instance)
.subscribe(with: self) { owner, sections in
owner.presenter.update(with: sections)
}setLoading 구독도 동일하게 적용하세요:
homeDataRelay
.map { $0 == nil }
+ .observe(on: MainScheduler.instance)
.subscribe(with: self) { owner, isLoading in
owner.presenter.setLoading(isLoading)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private func setupStream() { | |
| Observable.combineLatest( | |
| homeDataRelay.compactMap { $0 }, | |
| selectedCategoryRelay | |
| ) | |
| .map { model, selectedId -> [HomeSectionModel] in | |
| return [ | |
| .init(section: .banner, items: [.banner(model.banner)]), | |
| .init(section: .category, items: model.category.map { | |
| .category($0, isSelected: $0.id == selectedId) | |
| }), | |
| .init(section: .popularTrip, items: model.popularTrip.map { .popularTrip($0) }), | |
| .init(section: .recommendedTrip, items: model.recommendedTrip.map { .recommendedTrip($0) }) | |
| ] | |
| } | |
| .subscribe(with: self) { owner, sections in | |
| owner.presenter.update(with: sections) | |
| } | |
| .disposed(by: disposeBag) | |
| homeDataRelay | |
| .map { $0 == nil } | |
| .subscribe(with: self) { owner, isLoading in | |
| owner.presenter.setLoading(isLoading) | |
| } | |
| async let myTripsResult = homeService.fetchMyTrips() | |
| async let tripsByCategoryResult = homeService.fetchAllPopularTrips() | |
| async let recommendationsResult = homeService.fetchRecommendations() | |
| let (myTripsData, tripsByCategoryData, recommendationsData) = await ( | |
| (try? myTripsResult.get()) ?? [], | |
| (try? tripsByCategoryResult.get()) ?? [:], | |
| (try? recommendationsResult.get()) ?? [] | |
| ) | |
| await MainActor.run { | |
| self.myTrips = myTripsData | |
| self.tripsByCategory = tripsByCategoryData | |
| self.recommendations = recommendationsData | |
| presenter.hideLoading() | |
| presenter.updateMyTrips(myTripsData) | |
| presenter.updatePopularTrips(tripsByCategoryData, categories: categories) | |
| presenter.updateRecommendations(recommendationsData) | |
| .disposed(by: disposeBag) | |
| private func setupStream() { | |
| Observable.combineLatest( | |
| homeDataRelay.compactMap { $0 }, | |
| selectedCategoryRelay | |
| ) | |
| .map { model, selectedId -> [HomeSectionModel] in | |
| return [ | |
| .init(section: .banner, items: [.banner(model.banner)]), | |
| .init(section: .category, items: model.category.map { | |
| .category($0, isSelected: $0.id == selectedId) | |
| }), | |
| .init(section: .popularTrip, items: model.popularTrip.map { .popularTrip($0) }), | |
| .init(section: .recommendedTrip, items: model.recommendedTrip.map { .recommendedTrip($0) }) | |
| ] | |
| } | |
| .observe(on: MainScheduler.instance) | |
| .subscribe(with: self) { owner, sections in | |
| owner.presenter.update(with: sections) | |
| } | |
| .disposed(by: disposeBag) | |
| homeDataRelay | |
| .map { $0 == nil } | |
| .observe(on: MainScheduler.instance) | |
| .subscribe(with: self) { owner, isLoading in | |
| owner.presenter.setLoading(isLoading) | |
| } | |
| .disposed(by: disposeBag) |
🧰 Tools
🪛 SwiftLint (0.63.2)
[Warning] 85-85: Prefer implicit returns in closures, functions and getters
(implicit_return)
🤖 Prompt for AI Agents
In `@Projects/Features/HomeFeature/Sources/HomeInteractor.swift` around lines 79 -
104, The Rx subscriptions in setupStream() do not guarantee delivery on the main
thread, so UI calls like presenter.update(with:) and
presenter.setLoading(isLoading) can run off the main thread when
homeDataRelay.accept(model) is called from a background Task; fix by inserting
an observe(on: MainScheduler.instance) (RxSwift) before each subscribe in
setupStream() so the combineLatest chain (using homeDataRelay and
selectedCategoryRelay) and the homeDataRelay.map { $0 == nil } chain both
observe on the main scheduler; ensure you reference setupStream(),
homeDataRelay, selectedCategoryRelay, presenter.update(with:),
presenter.setLoading(isLoading), and disposeBag when making the change.
| private func fetchHomeData() { | ||
| fetchDataTask?.cancel() | ||
|
|
||
| presenter.setLoading(true) | ||
| presenter.showErrorView(false) | ||
|
|
||
| fetchDataTask = Task { [weak self] in | ||
| guard let self, !Task.isCancelled else { return } | ||
|
|
||
| do { | ||
| let myTripBanner: HomePresentationModel.Banner = await { | ||
| do { | ||
| return try await self.usecase.fetchMyTripInfo().toPresention() | ||
| } catch { | ||
|
|
||
| return .empty | ||
| } | ||
| }() | ||
|
|
||
| async let categories = self.usecase.fetchCategoryList().map { $0.toHomeModel() } | ||
| async let populars = self.usecase.fetchPopularTripList().map { $0.toPopularHomeModel() } | ||
| async let recommended = self.usecase.fetchRecommendTripList().map { $0.toRecommendHomeModel() } | ||
|
|
||
| let model = try await HomePresentationModel( | ||
| banner: myTripBanner, | ||
| category: categories, | ||
| popularTrip: populars, | ||
| recommendedTrip: recommended | ||
| ) | ||
|
|
||
| guard !Task.isCancelled else { return } | ||
|
|
||
| if self.selectedCategoryRelay.value == nil, let firstId = model.category.first?.id { | ||
| self.selectedCategoryRelay.accept(firstId) | ||
| } | ||
|
|
||
| homeDataRelay.accept(model) | ||
| presenter.setLoading(false) | ||
| } catch let error { | ||
| print(error) | ||
| presenter.setLoading(false) | ||
| presenter.showErrorView(true) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Task 내부에서 presenter 호출 시 메인 스레드 보장이 필요합니다.
Line 144, 147, 148의 presenter.setLoading()과 presenter.showErrorView()는 UI 업데이트이지만, Task { } 블록은 메인 스레드 실행을 보장하지 않습니다. @MainActor로 디스패치하거나 DispatchQueue.main.async를 사용하세요.
🔧 MainActor 적용 예시
guard !Task.isCancelled else { return }
if self.selectedCategoryRelay.value == nil, let firstId = model.category.first?.id {
self.selectedCategoryRelay.accept(firstId)
}
- homeDataRelay.accept(model)
- presenter.setLoading(false)
+ await MainActor.run {
+ self.homeDataRelay.accept(model)
+ self.presenter.setLoading(false)
+ }
} catch let error {
print(error)
- presenter.setLoading(false)
- presenter.showErrorView(true)
+ await MainActor.run {
+ self.presenter.setLoading(false)
+ self.presenter.showErrorView(true)
+ }
}🤖 Prompt for AI Agents
In `@Projects/Features/HomeFeature/Sources/HomeInteractor.swift` around lines 107
- 151, The presenter calls inside fetchHomeData() (presenter.setLoading and
presenter.showErrorView) are executed from a background Task and must be
dispatched to the main actor; update fetchHomeData() so all UI-related calls
(presenter.setLoading(true/false), presenter.showErrorView(true/false), and any
homeDataRelay/selectedCategoryRelay updates that drive UI) are executed on the
main thread—either annotate the Task body or those call sites with
`@MainActor/await` MainActor.run { ... } to ensure main-thread execution while
keeping the existing async flow and error handling in the Task.
| async let categories = self.usecase.fetchCategoryList().map { $0.toHomeModel() } | ||
| async let populars = self.usecase.fetchPopularTripList().map { $0.toPopularHomeModel() } | ||
| async let recommended = self.usecase.fetchRecommendTripList().map { $0.toRecommendHomeModel() } | ||
|
|
||
| let model = try await HomePresentationModel( | ||
| banner: myTripBanner, | ||
| category: categories, | ||
| popularTrip: populars, | ||
| recommendedTrip: recommended | ||
| ) |
There was a problem hiding this comment.
카테고리·인기·추천 중 하나라도 실패하면 전체 홈 화면이 에러 상태가 됩니다.
async let 바인딩 3개가 하나의 try await로 묶여 있어, 하나의 API 실패로 전체 데이터 로딩이 실패합니다. 배너처럼 개별 fallback을 적용하면 부분 실패 시에도 나머지 섹션을 표시할 수 있습니다.
🤖 Prompt for AI Agents
In `@Projects/Features/HomeFeature/Sources/HomeInteractor.swift` around lines 126
- 135, The three concurrent `async let` bindings (usecase.fetchCategoryList,
fetchPopularTripList, fetchRecommendTripList) are awaited together to build
HomePresentationModel so a single failure bubbles up and fails the entire home
screen; change to await each fetch independently with individual error
handling/fallbacks (e.g., do/catch per fetch or use an explicit Task/TaskGroup
that maps failures to empty/default models) so that
HomePresentationModel(banner: myTripBanner, category: <safeCategoryResult>,
popularTrip: <safePopularResult>, recommendedTrip: <safeRecommendResult>) is
constructed even if one call fails; update the code around HomePresentationModel
construction to pass those safe results.
| struct Category: Hashable { | ||
| let id: Int | ||
| let creator: String | ||
| let viedoType: VideoType |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# viedoType과 toPresention 사용처를 확인합니다.
echo "=== viedoType 사용처 ==="
rg -n 'viedoType' --type swift
echo ""
echo "=== toPresention 사용처 ==="
rg -n 'toPresention' --type swiftRepository: YAPP-Github/27th-App-Team-1-iOS
Length of output: 1639
오타: viedoType → videoType, toPresention → toPresentation
- Line 40:
viedoType은videoType의 오타입니다. 도메인 모델(TripCategory)의 public API에서 시작되어 6개 파일(홈, 인기 여행 프레젠테이션 모델 및 뷰 레지스트레이션 등)에 걸쳐 전파되어 있으므로, 함께 수정하는 것을 권장합니다. - Line 91:
toPresention()은toPresentation()의 오타입니다. HomeInteractor에서 호출되고 있습니다.
✏️ 수정 제안
struct Category: Hashable {
let id: Int
let creator: String
- let viedoType: VideoType
+ let videoType: VideoType
} extension MyTripSummary {
- func toPresention() -> HomePresentationModel.Banner {
+ func toPresentation() -> HomePresentationModel.Banner {- viedoType: self.viedoType
+ videoType: self.videoType🤖 Prompt for AI Agents
In `@Projects/Features/HomeFeature/Sources/Models/HomePresentationModel.swift` at
line 40, There are two typos to fix: rename the property viedoType to videoType
in the HomePresentationModel (and any other files that consume it across the
domain API, e.g., TripCategory related models and view registration code) and
rename the method toPresention() to toPresentation() (ensure HomeInteractor's
call site is updated to call toPresentation()); update all references/usage
sites and any public API signatures so they remain consistent across the six
affected files.
| self.type = .onGoing( | ||
| title: model.title, | ||
| date: model.duration, | ||
| transportIcon: DSKitAsset.Assets.icBus2.image, | ||
| duration: "\(schedule.estimatedDuration)분", | ||
| place: schedule.placeName, | ||
| imageUrl: schedule.thumbnailUrl | ||
| ) |
There was a problem hiding this comment.
교통수단 아이콘과 텍스트가 하드코딩되어 있습니다.
Line 66에서 transportIcon이 항상 icBus2로, Line 106에서 transport가 항상 "대중교통"으로 고정되어 있어, 모델의 transport 필드가 무시됩니다. 현재 서버가 한 가지 타입만 반환한다면 임시 조치로 이해하지만, 추후 다양한 교통수단 지원 시 누락될 수 있으므로 TODO 주석을 남겨두시길 권장합니다.
Also applies to: 101-110
🤖 Prompt for AI Agents
In `@Projects/Features/HomeFeature/Sources/Views/Cells/HomeBannerCell.swift`
around lines 63 - 70, Replace the hardcoded transport icon and label in the
onGoing case by reading the model/schedule transport field instead of always
using DSKitAsset.Assets.icBus2.image and the literal "대중교통": update the
self.type = .onGoing(...) call to derive transportIcon and transport from
model.transport (or schedule.transport) via a small mapping (e.g., enum/switch)
so new transport types render correctly, and add a TODO comment in
HomeBannerCell noting this is a temporary server limitation if only one
transport type is currently returned so future developers know to remove the
workaround.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@Projects/Features/HomeFeature/Sources/HomeInteractor.swift`:
- Around line 99-104: The stream mapping on homeDataRelay (.map { $0 == nil } ->
presenter.setLoading) conflicts with explicit presenter.setLoading(true) in
fetchHomeData()/reloadBtnTapped(); fix by making loading control consistent:
either reset the relay at the start of fetchHomeData() (call
homeDataRelay.accept(nil) before the network request) so the existing
subscription emits isLoading = true and then remove the explicit
presenter.setLoading(true) call, or remove the stream-based subscription (the
.map { $0 == nil } subscriber) and keep only the explicit
presenter.setLoading(true)/setLoading(false) calls; locate homeDataRelay,
fetchHomeData(), reloadBtnTapped(), and the subscriber block to apply the chosen
change.
🧹 Nitpick comments (1)
Projects/Features/HomeFeature/Sources/HomeInteractor.swift (1)
84-92: SwiftLint: 클로저 내 불필요한return키워드.Line 85의
return은 SwiftLintimplicit_return규칙에 의해 경고됩니다.♻️ implicit return 적용
.map { model, selectedId -> [HomeSectionModel] in - return [ + [ .init(section: .banner, items: [.banner(model.banner)]),
| homeDataRelay | ||
| .map { $0 == nil } | ||
| .subscribe(with: self) { owner, isLoading in | ||
| owner.presenter.setLoading(isLoading) | ||
| } | ||
|
|
||
| async let myTripsResult = homeService.fetchMyTrips() | ||
| async let tripsByCategoryResult = homeService.fetchAllPopularTrips() | ||
| async let recommendationsResult = homeService.fetchRecommendations() | ||
|
|
||
| let (myTripsData, tripsByCategoryData, recommendationsData) = await ( | ||
| (try? myTripsResult.get()) ?? [], | ||
| (try? tripsByCategoryResult.get()) ?? [:], | ||
| (try? recommendationsResult.get()) ?? [] | ||
| ) | ||
|
|
||
| await MainActor.run { | ||
| self.myTrips = myTripsData | ||
| self.tripsByCategory = tripsByCategoryData | ||
| self.recommendations = recommendationsData | ||
| presenter.hideLoading() | ||
| presenter.updateMyTrips(myTripsData) | ||
| presenter.updatePopularTrips(tripsByCategoryData, categories: categories) | ||
| presenter.updateRecommendations(recommendationsData) | ||
| .disposed(by: disposeBag) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
로딩 상태 관리가 스트림 기반과 명시적 호출 간에 불일치합니다.
Line 99-104의 스트림은 homeDataRelay가 nil인지 여부로 로딩 상태를 판단하지만, fetchHomeData()에서 재요청 시 relay를 nil로 리셋하지 않습니다. 따라서 reloadBtnTapped() 호출 시 이 스트림은 isLoading = true를 방출하지 않고, Line 110의 presenter.setLoading(true) 명시적 호출만 동작합니다.
두 메커니즘이 경쟁하면 로딩 상태가 꼬일 수 있습니다. 스트림 기반 로딩을 제거하고 명시적 호출로 통일하거나, fetchHomeData() 시작 시 homeDataRelay.accept(nil)로 리셋하는 방식 중 하나를 선택하세요.
♻️ 방법 1: fetchHomeData에서 relay 리셋
private func fetchHomeData() {
fetchDataTask?.cancel()
+ homeDataRelay.accept(nil)
presenter.setLoading(true)
presenter.showErrorView(false)이 경우 Line 99-104의 스트림이 자연스럽게 isLoading = true를 방출하므로, Line 110의 명시적 setLoading(true) 호출은 제거해도 됩니다.
♻️ 방법 2: 스트림 기반 로딩 제거 (명시적 호출로 통일)
- homeDataRelay
- .map { $0 == nil }
- .subscribe(with: self) { owner, isLoading in
- owner.presenter.setLoading(isLoading)
- }
- .disposed(by: disposeBag)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| homeDataRelay | |
| .map { $0 == nil } | |
| .subscribe(with: self) { owner, isLoading in | |
| owner.presenter.setLoading(isLoading) | |
| } | |
| async let myTripsResult = homeService.fetchMyTrips() | |
| async let tripsByCategoryResult = homeService.fetchAllPopularTrips() | |
| async let recommendationsResult = homeService.fetchRecommendations() | |
| let (myTripsData, tripsByCategoryData, recommendationsData) = await ( | |
| (try? myTripsResult.get()) ?? [], | |
| (try? tripsByCategoryResult.get()) ?? [:], | |
| (try? recommendationsResult.get()) ?? [] | |
| ) | |
| await MainActor.run { | |
| self.myTrips = myTripsData | |
| self.tripsByCategory = tripsByCategoryData | |
| self.recommendations = recommendationsData | |
| presenter.hideLoading() | |
| presenter.updateMyTrips(myTripsData) | |
| presenter.updatePopularTrips(tripsByCategoryData, categories: categories) | |
| presenter.updateRecommendations(recommendationsData) | |
| .disposed(by: disposeBag) | |
| // Stream-based loading removed; use explicit presenter.setLoading() calls instead |
🤖 Prompt for AI Agents
In `@Projects/Features/HomeFeature/Sources/HomeInteractor.swift` around lines 99 -
104, The stream mapping on homeDataRelay (.map { $0 == nil } ->
presenter.setLoading) conflicts with explicit presenter.setLoading(true) in
fetchHomeData()/reloadBtnTapped(); fix by making loading control consistent:
either reset the relay at the start of fetchHomeData() (call
homeDataRelay.accept(nil) before the network request) so the existing
subscription emits isLoading = true and then remove the explicit
presenter.setLoading(true) call, or remove the stream-based subscription (the
.map { $0 == nil } subscriber) and keep only the explicit
presenter.setLoading(true)/setLoading(false) calls; locate homeDataRelay,
fetchHomeData(), reloadBtnTapped(), and the subscriber block to apply the chosen
change.
2 participants
🔗 연결된 이슈
📄 작업 내용
💻 주요 코드 설명
1. Data 계층 Mapper 구현 및 의존성 격리 (
toDomain)ProgramResponse를 도메인 엔티티인TripCategory로 변환하는 로직을 Data 모듈 내에 위치시켰습니다.VideoType의 RawValue 매칭 등을 매퍼 내부에서 처리하여, 도메인 레이어는 순수한 비즈니스 모델만 다룰 수 있게 설계했습니다.2. MainFeature 중심의 내비게이션 구조 리팩토링
MainFeature가 앱의 주요 흐름을 제어하는 역할을 수행합니다.MainRouter로 이관하여, 탭 상단에 새로운 화면을 푸시하거나 교체하는 로직을 통합 관리합니다.MainFeature가 각 하위 Feature(Follow,PopularTravel,Search,Setting)를 자식 RIB으로 직접 관리함으로써, Feature 모듈 간의 직접적인 의존성 없이 독립적으로 동작할 수 있는 구조를 확립했습니다.Feature모듈은 내부 로직 수행을 위해Data나Networks모듈을 직접 참조하지 않도록 설계했습니다.Domain계층의 인터페이스(Protocol)에만 의존하게 함으로써, 기능 모듈의 재사용성과 테스트 용이성을 확보했습니다.Data,Networks)를 생성합니다.Component를 통해 각Feature로 주입(Dependency Injection)하는 구조를 채택하여 모듈 간 결합도를 낮추었습니다.🏗️ 개편된 모듈 구조도
3. 네비게이션 동기화 및 안정화 로직 (
MainRouter&ViewControllers)isMovingFromParent를 이용한 이탈 감지viewDidDisappear시점에isMovingFromParent플래그를 확인하여 사용자의 실제 화면 이탈 의도(스와이프 또는 백버튼 클릭)를 정확히 판별합니다.containsInStack)MainRouter에서pop을 수행하기 전,containsInStack인터페이스를 통해 해당 뷰가 네비게이션 스택에 실제로 남아있는지 검증합니다.pop이 진행 중인 경우, 중복 명령이 전달되는 것을 방지합니다.guard let을 활용한 조기 리턴과detachChild후 상태 변수를 즉시nil로 초기화하는 순서를 최적화하여 재귀적 호출을 원천 차단했습니다.// SettingViewController override func viewDidDisappear(_ animated: Bool) { super.viewDidDisappear(animated) if isMovingFromParent { listener?.detachSetting() } } // MainRouter func detachSetting() { guard let router = settingRouter else { return } if viewController.containsInStack(router.viewControllable) { viewController.popRootViewController(animated: true) } detachChild(router) self.settingRouter = nil }👀 기타 더 이야기해볼 점
1. 모듈 의존성 관련 일시적 조치 (Dependency Cleanup)
Feature모듈은Data와Networks를 직접 참조하지 않고Domain인터페이스에만 의존해야 합니다. 또한Networks는Domain을 참조하지 않는 단방향 구조를 지향합니다.MainFeature에서Networks를 참조하지 않을 경우 빌드가 깨지는 이슈가 발생했습니다.import와 의존 관계를 완전히 정리할 예정입니다.PR이 너무 커져서 죄송합니다...
Summary by CodeRabbit
새로운 기능
개선사항