Skip to content
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

[Feat] #275 - Home 뷰 클린 아키텍쳐로 리팩토링 #281

Open
wants to merge 33 commits into
base: refactoring/#272-clean-architecture
Choose a base branch
from

Conversation

jeongdung-eo
Copy link
Contributor

@jeongdung-eo jeongdung-eo commented Jul 22, 2024

🌁 Background

  • home 뷰 MVVM + 클린 아키텍쳐 도입

##📱 Screenshot

👩‍💻 Contents

  1. MVC -> MVVM + Combine + clean Architecture
  2. collection publisher 구현
  3. collectionview Section Header와 nickname은 필터쪽 완성 후 다시 작업하겠습니다.

✅ Testing

📝 Review Note

  1. Collectionview 로직 변경
    [1차 개선]
    기존 MVC 아키텍처를 MVVM으로 전환하면서, 기존 diffableDataSource를 기본 dataSource로 리팩토링함.
    diffableDataSource는 UI 업데이트 시 애니메이션이 자연스럽게 적용되는 장점이 있지만, 홈뷰의 경우 데이터 업데이트나 애니메이션이 필요하지 않다고 판단해 기본 dataSource로 다시 구현함.

고민했던 부분
홈 화면은 여러 섹션으로 구성되어 있고, 데이터가 각 섹션별로 들어오게 됨.

  1. ViewModel에서 모든 섹션의 데이터를 받아 한 번에 ViewController에 바인딩하여 업데이트
    이 방법은 어느 한 섹션이라도 데이터가 늦게 도착하면 초기 화면에 아무것도 나타나지 않아 사용자 경험이 저하될 수 있음.

  2. 각각의 섹션을 별도로 ViewController에 바인딩하여 데이터가 들어오는 순서대로 업데이트
    이 방법을 사용할 때 reloadSection을 이용해 데이터를 업데이트했는데, 이로 인해 화면이 깜빡이거나 데이터 밀림 현상이 발생함.

section 밀림

이를 해결하기 위해 아래 코드를 사용해 애니메이션을 없앴음

UIView.performWithoutAnimation {
    self.collectionView.reloadSections(IndexSet(integer: 0))
}

하지만 이 방법은 reloadSection이 섹션 데이터를 다시 지우고 그리기 때문에 데이터 양이 증가할수록 성능에 영향을 미침.
특히 홈뷰의 복잡한 화면 전환(검색뷰, 필터뷰, 디테일뷰 등)에서 성능 저하가 우려됨.

따라서 !!! 단순히 애니메이션을 제거하려 했으나, 복잡한 화면 전환과 성능 문제를 고려해 diffableDataSource로 다시 리팩토링하기로 결정함. 데이터 변화가 생기더라도 snapshot을 사용해 변경된 데이터만 업데이트하도록 함.

[2차 개선]

diffableDataSource로 다시 리팩토링하면서 MVVM 패턴과 Combine을 활용해 기존 고민을 개선함.

  1. 섹션별 데이터 바인딩을 통해 사용자 경험을 향상시킴.
  2. ViewModel에서는 기존 Future 대신 CurrentValueSubject PassthroughSubject와 Task를 사용해 비동기 로직을 개선함.
  • Future는 한 번 값을 방출하고 종료되기 때문에, 지속적인 상태 업데이트나 유지가 어려움.
  • PassthroughSubject + Task를 사용하면, 비동기 작업 후 데이터를 끊김 없이 전달하고, 최신 상태를 유지할 수 있음.

But!!
각각의 데이터를 snapshot에 append하는 과정에서, 마지막에 도착한 데이터만 화면에 나타나는 문제가 발생함.

reloadBestList(bakery: [BestBakery] = [], review: [BestReview] = []) {
    guard snapShot { return }
}

[원인]
데이터가 들어올 때마다 새로운 snapshot을 만들어 데이터를 추가했기 때문에, 마지막 데이터만 표시됨.

[해결 방법]
현재 스냅샷을 가져와서, 기존 아이템이 비어있을 때만 업데이트하도록 수정했더니 제대로 동작함.

guard var snapShot = self.dataSource?.snapshot() else { return }

결론!! 홈뷰를 다시 diffableDataSource를 활용해 리팩토링했음.

  1. Collectionview Publisher 구현

RxCocoa에서는 rx.itemSelected를 통해 cell 이벤트를 쉽게 전달할 수 있지만, Combine에는 이와 같은 기능이 없어 직접 구현이 필요함.
홈 화면에서 cell을 눌렀을 때 해당 cell의 id와 name을 detail뷰로 넘겨주어야 했음. 처음에는 cell 클릭 액션이니까 당연히 ViewModel에서 처리해야 한다고 생각했음.

[문제점]

cell event를 ViewModel에 전달.
ViewModel에서 indexPath를 통해 id와 name을 다시 ViewController에 전달.
ViewController에서 화면 전환.
이런 플로우가 되었고, 이 과정에서 id와 name을 얻기 위해 ViewModel에 불필요한 데이터 저장 로직을 추가해야 했음. 또한, 유저 트래킹을 위한 애널리틱스 로직을 ViewModel과 ViewController에서 모두 처리해야 해서, MVVM의 경계가 단순 로직 분리로만 구현됨.

성민쿤과의 고민
IndexPath는 주로 UITableView, UICollectionView 등 리스트 형식 UI 컴포넌트에서 셀 위치를 식별하는 데 사용됨.
IndexPath 자체는 UI 레이아웃 및 구조와 밀접한 관련이 있기 때문에, ViewModel의 관심사가 아니라고 생각함.

이 문제를 해결하기 위해 RxCocoa처럼 CollectionView의 Publisher를 직접 구현하기로 결정함.

[과정]

  1. CollectionView에서 발생할 수 있는 두 가지 이벤트인 didSelect와 didDeselect를 열거형으로 구현.
  2. SubscriberType이 구독할 수 있는 Subscription을 구현.
  3. CollectionViewDelegateProxy를 구현하여 CollectionViewDelegate의 프록시 역할을 하고, 선택된 이벤트를 감지하여 이를 처리.
  4. CollectionView의 특정 이벤트를 구독할 수 있는 Publisher를 구현.

이렇게 Publisher를 구현함으로써 cell 이벤트에 대한 로직을 처리할 수 있었고, 이를 통해 불필요한 데이터 저장 로직을 제거하고, MVVM 구조를 좀 더 명확하게 유지할 수 있었음!!!

📣 Related Issue

📬 Reference

collectionview cell touch event publisher 생성시 참고
https://www.avanderlee.com/swift/custom-combine-publisher/

Comment on lines 19 to 23
let bestRepository: BestRepository

init(bestRepository: BestRepository) {
self.bestRepository = bestRepository
}
Copy link
Contributor

Choose a reason for hiding this comment

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

정리 필요 !!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +123 to 134
func configureStackView(with certifications: Certifications) {

// self.markStackView = GBStackView(type: .big, data: data)
//
// if let markStackView = markStackView {
// bakeryImage.addSubview(markStackView)
// markStackView.snp.makeConstraints {
// $0.top.leading.equalToSuperview().offset(10)
// $0.size.equalTo(CGSize(width: heightConsideringNotch(68), height: heightConsideringNotch(28)))
// }
// }
}
Copy link
Contributor

Choose a reason for hiding this comment

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

안쓰는거라면 삭제 !!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

요거는 디자인시스템 머지하면 !! 수정할게욤


// MARK: - Property

private lazy var safeArea = self.view.safeAreaLayoutGuide
Copy link
Contributor

Choose a reason for hiding this comment

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

이거 lazy 로 한 이유가 있남 ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

클래스의 인스턴스가 완전히 초기화되기 전에 self가 존재하지 않기 때문에, 초기화 단계에서는 self를 참조할 수 없슴당. lazy var를 사용하면 프로퍼티가 처음 접근될 때 초기화되므로, 그 시점에는 이미 self가 초기화되어 있어 안전하게 접근할 수 있어 lazy var로 선언했습니당

Comment on lines 107 to 111
.sink { err in
print("error:\(err)")
} receiveValue: { [weak self] bakery in
self?.updateBakery(bakery: bakery)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

아마 여기서 사용하는 sink 의 첫 closure 안에는 error 를 전달하는게 아니라 completion 을 전달할겁니다..!!
err 대신 completion 을 사용하는게 어떤가 ~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jeongdung-eo jeongdung-eo changed the title Feat/#275 clean home [Feat] #275 - Home 뷰 클린 아키텍쳐로 리팩토링 Aug 13, 2024
Comment on lines 335 to 338
final class HomeCompositionalLayout: UICollectionViewCompositionalLayout {


}
Copy link
Contributor

Choose a reason for hiding this comment

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

???

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ㅋㅋ 지송함다 ㅋㅋㅋ 따로 구현하려다가 ,,, 지우겠습니다 ..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines 28 to 29
private var bakerySubject = CurrentValueSubject<[BestBakery], Never>([])
private var reviewSubject = CurrentValueSubject<[BestReview], Never>([])
Copy link
Contributor

Choose a reason for hiding this comment

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

이걸 꼭 CurrentValueSubject로 해야했을까연 ??
이거 그냥 Passthrough로 하고 바인딩 한 다음에 값 전달하는건 어떨까 싶은 ..??
왜냐하면 처음에 값 들고 있지 않으니까 CurrentPassthrough 중이라면 후자가 더 적합할 것 같다고 생각했슴다

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오 !! 고민하던 부분인데 ~~ 반영했습니당
eeb5ffa

Copy link
Contributor

@seongmin221 seongmin221 left a comment

Choose a reason for hiding this comment

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

좋습니다 ~~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants