Skip to content

Conversation

@Guryss
Copy link
Member

@Guryss Guryss commented Feb 25, 2024

⭐️Issue

#125

🌟Motivation

RecordView MVVM 패턴 적용했습니다.

🌟Key Changes

  • ImageLiterals 파일 및 해당 파일에 사용된 UIImage Extension 내 load 함수 삭제 했습니다.
  • Images 폴더 삭제 했습니다. (ImageLiterals 만 들어있었음)
  • Input, Output 구분하여 비즈니스 로직 분리해봤습니다.
    ViewModel, ViewController 위주로 봐주심 될 것 같네요!
    처음엔 어려웠는데 ...... 하다보니 이해가 되는거같음 !! 진짜 조타

🌟Simulation

없다능

🌟To Reviewer

  • 현재 bindViewModel 함수에 있는 모든 output 관련 UI 함수들이 viewDidLoad될때 모두 실행되고 있어여 ..
    내가 이해하기론 Input에서 true를 던져주면 Output에서 그걸 받아가지고 UI 바꿔주는걸로 아는데 ..
    왜 자꾸 ViewDidLoad 즉 뷰 최초 로드 될때 bindViewModel이 실행되는건지 모르겠서요 🥲🥲🥲🥲🥲🥲

ㄴ 해결 완료헀습니다 !!!
처음에 Output에 들어가는 객체들을 모두 BehaviorRelay로 잡아주었더니 생겼던 문제였습니다.
BehaviorSubject는 구독 이전에도 이벤트를 방출하기 때문에 viewDidLoad되자마자 함수 내의 output 코드들이 실행되었던 것입니다.
따라서 구독 이전엔 이벤트를 방출하지 않는 PublishRelay로 잡아주었더니 해당 문제가 해결되었습니다 :)
(낙낙 .... 고맙습니다 ^3^)

🌟Reference

https://nacho.tistory.com/21

@Guryss Guryss added the guryss label Feb 25, 2024
@Guryss Guryss self-assigned this Feb 25, 2024
@Guryss Guryss changed the title [Refactor] #125 - RecordView MVVM pattern [Refactor] #125 - RecordView MVVM 패턴 적용 Feb 25, 2024
@Guryss Guryss changed the title [Refactor] #125 - RecordView MVVM 패턴 적용 [Refactor] #125 - Record MVVM 패턴 적용 Feb 29, 2024
Copy link
Contributor

@Naknakk Naknakk left a comment

Choose a reason for hiding this comment

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

좋아요 ! 깔끔하게 잘 된 것 같습니당 ㅎㅎㅎ 고생많았습니다 !!

+++
추가로 BehaviorRelay는 구독 이전에 이벤트를 방출한다기 보다는 구독하는 순간에 이벤트를 방출해 현재값을 받아와 스트림을 한번 실행하도록 하고, 이후로 이벤트가 방출될 때마다 스트림을 실행하도록 하는 친구이고 /
PublishRelay는 구독하는 순간에는 별다른 작업이 없고, 일반적인 rx Subject처럼 이벤트 방출할 때마다 스트림을 실행하도록 하는 친구로 알고 있습니당 ㅎㅎ
잘 알고 있겠지만 적어준 내용이 약간 느낌이 달라서 다시 적어봤어요 !!

import UIKit

extension UIImage {
static func load (named imageName: String) -> UIImage {
Copy link
Contributor

Choose a reason for hiding this comment

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

W3
안쓰게 된 메서드 삭제해주는 디테일 좋아요 !

Copy link
Member Author

Choose a reason for hiding this comment

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

성의있는 리뷰 정말 정말 고맙습니다 :)

Copy link
Member

@ena-isme ena-isme left a comment

Choose a reason for hiding this comment

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

네이밍 수정까지 야무지네용
수고하셨습니다 💖

$0.setTitle(StringLiterals.Alignment.newest, for: .normal)
$0.setTitle(StringLiterals.Alignment.newest.title, for: .normal)
$0.setTitleColor(.wssGray300, for: .normal)
$00.titleLabel?.font = .Label1
Copy link
Member

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋ 귀엽네

configuration.background.cornerRadius = 12
configuration.contentInsets = NSDirectionalEdgeInsets(top: 18, leading: 42, bottom: 18, trailing: 42)
$0.configuration = configuration
}
Copy link
Member

Choose a reason for hiding this comment

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

W3
우리 플젝은 iOS 버전이 가장 최근것으로 되어있는 것 같은데
버전을 조금 내리는 건 어떻게 생각하세용 ?

let newestButtonTapped: ControlEvent<Void>
let oldestButtonTapped: ControlEvent<Void>
let recordCellSelected: ControlEvent<IndexPath>
let emptyButtonTapped: ControlEvent<Void>
Copy link
Member

Choose a reason for hiding this comment

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

W3
controlEvent ,,, 알아갑니다

.subscribe(with: self, onNext: { owner, _ in
output.showAlignmentView.accept(true)
})
.disposed(by: disposeBag)
Copy link
Member

Choose a reason for hiding this comment

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

W3
Controlevent 써서 이렇게 바로 true 값으로 보내줄 수 있구나 ,,,

Copy link
Collaborator

@hyowon612 hyowon612 left a comment

Choose a reason for hiding this comment

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

넘 깔끔합니당 수고해썽요 !!

var selectedItemImage: UIImage {
switch self {
case .home:
case .home:
Copy link
Collaborator

Choose a reason for hiding this comment

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

W3
ㅋㅋㅋㅋ 꼼꼼이 !

viewWillAppearEvent: viewWillAppearEvent.asObservable(),
sortTypeButtonTapped: rootView.headerView.headerAlignmentButton.rx.tap,
newestButtonTapped: rootView.alignmentView.libraryNewestButton.rx.tap,
oldestButtonTapped: rootView.alignmentView.libraryOldesttButton.rx.tap,
Copy link
Collaborator

Choose a reason for hiding this comment

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

W1
libraryOldesttButton 오타 !!

Copy link
Member Author

Choose a reason for hiding this comment

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

폭룡적인 오타구만. 수정할게요 ><

@Guryss Guryss merged commit d395725 into main Mar 3, 2024
@Guryss Guryss deleted the Refactor/#125-Record branch March 3, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

5 participants