컬렉션 API 연동 #270
컬렉션 API 연동 #270
Conversation
Summary of ChangesHello @pinocchio22, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 컬렉션 관리 기능을 백엔드 API와 연동하여 사용자가 컬렉션 목록을 조회하고 새로운 컬렉션을 생성할 수 있도록 합니다. 기존의 더미 데이터를 제거하고 실제 서버 데이터를 활용하도록 시스템을 확장하며, UI 컴포넌트가 이미지 URL을 기반으로 콘텐츠를 동적으로 로드할 수 있도록 개선합니다. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
컬렉션 조회 및 추가를 위한 API 연동 작업을 검토했습니다. 전반적으로 Clean Architecture에 따라 잘 구현되었지만, 몇 가지 개선점과 수정이 필요한 부분이 있습니다. 특히 CollectionListReactor에서 API 연동 후 발생한 버그는 앱 크래시를 유발할 수 있어 수정이 시급합니다. 또한, 코드에 남아있는 디버깅용 print 구문들을 제거하고, 도메in 모델의 날짜 타입을 표준 Date로 변경하여 코드의 안정성과 유지보수성을 높이는 것을 제안합니다.
| case .viewWillAppear: | ||
| return collectionListUseCase.execute().map { .setListData($0) } | ||
| case .itemTapped(let index): | ||
| return .just(.navigateTo(.detail(currentState.collections[index]))) |
There was a problem hiding this comment.
itemTapped 액션이 currentState.collections를 사용하여 상세 페이지로 이동하고 있습니다. 하지만 API 연동 후 실제 데이터는 collectionListData에 저장되므로, collections는 더미 데이터를 담고 있거나 collectionListData와 동기화되지 않은 상태일 수 있습니다. 이로 인해 잘못된 데이터로 상세 페이지가 열리거나, 인덱스가 맞지 않아 앱이 크래시될 수 있습니다. collectionListData의 데이터를 사용하도록 수정해야 합니다.
case .itemTapped(let index):
// FIXME: `collections`는 더미 데이터입니다. `collectionListData`를 사용해야 합니다.
// `collectionListData[index]`를 `BookmarkCollection`으로 변환하는 로직이 필요합니다.
return .just(.navigateTo(.detail(currentState.collections[index])))|
|
||
| public struct State { | ||
| @Pulse var route: Route | ||
| var collections: [BookmarkCollection] |
There was a problem hiding this comment.
State의 collections 속성은 API 연동으로 인해 더 이상 사용되지 않는 것으로 보입니다. itemTapped 액션에서 잘못 사용되고 있어 버그를 유발하고 있습니다. collectionListData를 사용하도록 수정한 뒤, 이 속성을 제거하여 혼동을 줄이고 코드를 정리하는 것이 좋습니다.
| self.initialState = State(route: .none, collections: [ | ||
| BookmarkCollection(id: 1, title: "1번", items: [ | ||
| BookmarkCollection(id: 1, title: "1000번", items: [ | ||
| DictionaryItem(id: 1, type: .item, mainText: "1번 아이템", subText: "1번 설명", image: .add, isBookmarked: false), | ||
| DictionaryItem(id: 2, type: .item, mainText: "2번 아이템", subText: "2번 설명", image: .add, isBookmarked: false) | ||
| ]), | ||
| BookmarkCollection(id: 2, title: "2번", items: [ | ||
| BookmarkCollection(id: 2, title: "2000번", items: [ | ||
| DictionaryItem(id: 3, type: .item, mainText: "3번 아이템", subText: "3번 설명", image: .add, isBookmarked: false), | ||
| DictionaryItem(id: 4, type: .item, mainText: "4번 아이템", subText: "4번 설명", image: .add, isBookmarked: false) | ||
| ]) | ||
| ]) | ||
| ], collectionListData: []) |
There was a problem hiding this comment.
init에서 collections 속성에 더미 데이터를 할당하는 코드는 더 이상 필요하지 않습니다. collections 속성이 제거되면 이 코드도 함께 제거하여 initialState를 간소화할 수 있습니다.
self.initialState = State(route: .none, collectionListData: [])| public let recentBookmarks: [BookmarkDTO] | ||
|
|
||
| public func toDomain() -> CollectionListResponse { | ||
| return CollectionListResponse(collectionId: collectionId, name: name, createdAt: createdAt, recentBookmarks: recentBookmarks.toDomain()) |
There was a problem hiding this comment.
createdAt이 [Int] 타입으로 도메인 모델에 그대로 전달되고 있습니다. 도메인 모델에서는 Date와 같은 표준 타입을 사용하는 것이 좋습니다. 이 toDomain() 메서드 내에서 [Int]를 Date로 변환하는 로직을 추가하고, CollectionListResponse의 createdAt 타입도 Date로 변경하는 것을 권장합니다. 이렇게 하면 데이터 변환의 책임을 Data 레이어에 유지하고 도메인 모델을 더 견고하게 만들 수 있습니다.
| public func createCollectionList(name: String) -> Completable { | ||
| let endPoint = CollectionEndPoint.createCollectionList(body: CreateCollectionRequestDTO(name: name)) | ||
| return provider.requestData(endPoint: endPoint, interceptor: tokenInterceptor) | ||
|
|
There was a problem hiding this comment.
불필요한 공백 라인이 있습니다. 코드 가독성을 위해 제거하는 것이 좋습니다.
| loadImages(from: input.images) { [weak self] images in | ||
| print("이미지:\(images)") | ||
| self?.cellView.setImages(images: images) | ||
| } |
There was a problem hiding this comment.
디버깅 목적으로 사용된 print 구문이 코드에 남아있습니다. 프로덕션 코드에 포함되지 않도록 제거해주세요.
loadImages(from: input.images) { [weak self] images in
self?.cellView.setImages(images: images)
}| private func loadImages(from urls: [String?], completion: @escaping ([UIImage?]) -> Void) { | ||
|
|
||
| var results = [UIImage?](repeating: nil, count: urls.count) | ||
| let dispatchGroup = DispatchGroup() | ||
|
|
||
| for (index, urlString) in urls.enumerated() { | ||
| dispatchGroup.enter() | ||
|
|
||
| ImageLoader.shared.loadImage(stringURL: urlString) { image in | ||
| results[index] = image | ||
| dispatchGroup.leave() | ||
| } | ||
| } | ||
|
|
||
| dispatchGroup.notify(queue: .main) { | ||
| completion(results) | ||
| } | ||
| } |
There was a problem hiding this comment.
loadImages 함수가 파일 최상단에 private으로 선언되어 있습니다. 이 함수는 CollectionListCell에서만 사용되므로, CollectionListCell의 private 메서드로 옮겨서 클래스와의 연관성을 명확히 하고 코드의 응집도를 높이는 것이 좋습니다.
| let imageView = view.subviews.compactMap { $0 as? UIImageView }.first | ||
| print("이미지 뷰 설정") | ||
| imageView?.image = index < images.count ? images[index] : nil | ||
| } | ||
| } |
There was a problem hiding this comment.
디버깅 목적으로 사용된 print 구문이 코드에 남아있습니다. 프로덕션 코드에 포함되지 않도록 제거해주세요.
for (index, view) in imageViews.enumerated() {
let imageView = view.subviews.compactMap { $0 as? UIImageView }.first
imageView?.image = index < images.count ? images[index] : nil
}| case .sort(let type): | ||
| print("sortsort실행~~~~~~~~~~~~~~~~~~~~~~~") | ||
| let viewController = owner.sortedFactory.make( |
There was a problem hiding this comment.
디버깅 목적으로 사용된 print 구문이 코드에 남아있습니다. 프로덕션 코드에 포함되지 않도록 제거해주세요.
case .sort(let type):
let viewController = owner.sortedFactory.make(
📌 이슈
✅ 작업 사항