-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Refactor/#138 ReactorKit 적용 - 북마크 #141
base: develop
Are you sure you want to change the base?
Conversation
|
||
import Then | ||
import SnapKit | ||
|
||
import Kingfisher | ||
|
||
final class BookmarkVC: BaseNavigationViewController { | ||
final class BookmarkVC: BaseNavigationViewController, View { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
View 채택 같은 부분은 SwiftUI인가요? 어느 부분에서 코드 구현이 되어있는지 잘 몰라서ㅜㅜ 질문 드리고자합니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ReactorKit의 View 입니다!
Reactor
와 연결해주기 위해 View를 채택해주어야 합니다.
링크 참고해주세요!
@@ -52,7 +52,6 @@ final class AuthInterceptor: RequestInterceptor { | |||
completion(.doNotRetryWithError(error)) | |||
return | |||
} | |||
tabBarVC.showToast(message: "LogoutSuccess".localized, bottomViewHeight: 50.0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
앗 여기서 커밋 메시지로 작성한 스레드 문제가 뭐였나요?? 어떤 점이 문제였는지 기억이 나질 않네요😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
기존 AuthInterceptor
에서 해당 코드를 호출할 때
메인 스레드가 아닌 다른 스레드에서 UI 작업을 시도하기 때문에 문제가 발생했습니다!
이번에 발견한 문제였어요,, 😅
@@ -47,6 +48,7 @@ final class BookmarkListVC: BaseNavigationViewController { | |||
|
|||
private let viewModel: BookmarkCardListVM | |||
private let bookmarkType: BookmarkSearchType | |||
var disposeBag: DisposeBag = .init() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
현재 이 disposeBag이 하는 역할이 있나요?? .init()으로 표현한 이유가 있을까요! (Reactor로 변화를 주다보니 기존에 사용하던 MVVM 패턴에서의 개념과 어느정도 조화를 이루는지 아직 감을 잘 못 잡고 있는 것 같습니다..ㅎ)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
마찬가지로 ReactorKit의 View
를 채택하게 되면 Reactor
와의 bind를 위해 disposeBag을 가져야 합니다!
.init()
은 타입 어노테이션을 해줬기 때문에 편의상 사용했어용
@@ -47,13 +47,30 @@ struct BookmarkInfo: Decodable { | |||
} | |||
} | |||
|
|||
extension BookmarkInfo { | |||
static let empty: Self = .init(id: 0, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
북마크 정보에 emtpy 모델을 추가신 이유가 있을까요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
State
에서 초기값으로 사용하기 위해 추가해주었습니다!
.disposed(by: disposeBag) | ||
|
||
reactor.state.map { $0.successModifiedInfo } | ||
.skip(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 skip은 어떤 상황에서 역할을 하게 되는건가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
State
의 초기값(empty 모델)을 제외하고 그 다음 데이터부터 받아오기 위해 사용했습니다~!
🧑🏻💻 PR 작업 내역 요약
📋 변경 사항
ReactorKit 의존성 추가
ViewModel -> Reactor 리팩토링
🔍 Code Review
📸 ScreenShot
�N/A
연결된 이슈 Close
close #138