프로젝트 매니저 [STEP 2-2] Ari #104
Conversation
- Storage의 delete 메소드의 파라미터 타입을 UUID에서 Project로 재수정 - Storage 수정에 따른 Repository, UseCase 전반적인 수정 - ProjectListViewModel 수정에 따른 이전 테스트코드 제거 - ProjectListViewModel를 Input, Output, transform으로 나누어 재설계
- HeaderView에 초기 설정하는 configure 메소드 추가 - Cell에 초기설정하는 configure 메소드 추가 - UITableViewDelegate에 viewForHeaderInSection 메소드 구현 - formattedString 프로퍼티를 static 메소드로 수정 - ProjectListUserCase에 구현하지 않은 정의부분 제거 - ProjectState에 rawValue를 추가
- ProjectListViewModel 내부 Input 타입을 RxCocoa에 맞게 리팩토링 - Output 타입을 외부에서 값을 주입하지 못하도록 Observable로 수정 - Input과 Output 수정에 따른 transform 메소드 내부를 대폭 수정 - MainViewController에 UILongPressGestureRecognizer를 추가 - ViewModel과 View를 바인딩 처리하는 setUpBindings 메소드 추가 - tableView가 deselect 될 수 있도록 delegate 메소드 구현
- MainCoordinator 타입 생성 - info.plist에서 Main Storyboard를 제거 - SceneDelegate에서 Coordinator를 활용하여 초기설정을 하도록 구현 - Cell을 터치했을 때 read 모드의 DetailViewController를 present하도록 구현
- DetailCoordinator 타입 생성 - MainCoordinator에서 DetailCoordinator를 생성하여 화면전환을 구성하도록 리팩토링 - DetailViewModel에 coordinator 프로퍼티를 추가하고 init을 수정 - Detail 스토리보드 내부 TextView의 스크롤을 비활성화 시키고, 오토레이아웃을 조정하여 스크롤 되도록 수정
- Storage에 1개의 요소만 가져올 수 있는 fetch 메소드 오버로드 - Input에 타입을 RxCocoa에 맞춰 옵셔널로 리팩토링 - DetailViewController의 기존 configure 메소드를 제거하고 binding 형태로 대폭 수정 - ProjectCell의 ChangedDateColor 메소드 오타를 수정
- input 등록 시 변경되었을 때의 값만 구독하도록 changed로 수정 - transform 내부에 current 변수들을 옵셔널하게 수정
- 길게 터치시 뷰모델에게 터치한 Cell과 Project를 전달하도록 구성 - 전달받은 Cell과 Project를 활용하여 Coordinator에게 액션시트를 띄워달라고 요청 - 액션시트에서 버튼을 터치했을 때 useCase에게 상태를 업데이트해달라고 요청 - ProjectListUseCase에 changedState 메소드를 추가 - ProjectState 타입에 excluded 프로퍼티를 추가
…록 리팩토링 #3 - DetailViewModel 내부 mode에 따라 update, create를 하는 부분 개선
- excluded를 튜플에서 배열로 수정 - MainCoordinator의 showActionSheet 메소드를 Observable로 리팩토링
- 글자수가 일정숫자 넘어가게 되면 Done 버튼을 비활성화 - DetailViewModel 내부 Output에 isDescriptionTextValid 프로퍼티 추가 - 뷰모델 내부 축약가능한 부분 리팩토링
- forEach를 for-in문으로 수정 - if로 옵셔널 바인딩 처리하는 부분 flatMap으로 대체 - 접근제한 - 매직넘버를 .zero로 수정
- self를 활용하는 곳에 withUnretained 메소드를 활용하여 리팩토링 - DetailViewController가 닫힐 때마다 ViewModel, Coordinator, Controller 모두 메모리에서 없어지도록 개선 - Coordinator 프로토콜 구현
- 줄바꿈 컨벤션 수정 - MainCoordinator에 테스트용으로 적었던 didSet을 제거
- 테스트 타겟에 RxBlocking, RxTest 라이브러리 추가
There was a problem hiding this comment.
아리, 이번에도 고생 많으셨습니다! 갈수록 반응형 프로그래밍 프레임워크를 다루는 감각이 늘어가고 있다는게 느껴지네요!
앱 디자인이 굉장히 깔끔했고, withUnretained(_:)와 같이 새로 도입된 연산자를 사용해보시며 메모리 관리를 하셨다는 부분이 굉장히 인상적이었습니다. 전제적으로 오탈자가 다소 발생하고 있는데, 그때그때 체크하기 어려우시다면 아래 그림과 같은 기능을 이용해서 살펴볼 수도 있습니다.
질의응답
- Coordinator를 적절히 사용했는지??
네, 코디네이터 패턴이 잘 적용되어 있네요. 저도 어떤 로직에 따라 화면 전환 이벤트가 발생할테니 View 보다는 ViewModel에 위치하는게 적절하다고 생각해요. CoordinatorType은 지금은 필요한지 잘 모르겠네요. 화면 전환 이벤트를 열거형의 케이스로 관리하고 싶으신걸까요?
코디네이터 패턴은 구현해보셨듯이 매 프로젝트마다 보일러플레이트가 다소 발생하는데, 이 때문에 라이브러리가 꽤 있어요. 패턴에 익숙해지시면 이 라이브러리도 참고해보세요. RxFlow
- ViewModel에 UIKit이 import 되어있는데 괜찮을까요?
ViewModel의 태생적 목적상 View와 무관한 상태로 유지하는 것이 가장 이상적이긴 해요. 극단적으로 View의 어떤 요소에도 의존하고 있지 않다면, UIKit을 사용하는 모바일이든, AppKit을 사용하는 데스크탑/랩탑이든 플랫폼에 관계없이 이 로직을 재사용할 수 있으니까요. 하지만 ViewModel이 View를 모름으로써 생기는 장점 (1:N 관계 형성 가능 등)을 다소 포기하더라도 필요할 때는 아리와 같은 결정을 할 수 있어야 한다고 생각합니다. 절충은 가끔 필요하죠.
- Memory leak 방지를 위한 동작이 올바른지?
누가 memory leak을 일으키고 있나요? 그것이 명확하면 적용하신 방법을 사용하지 않아도 아리가 알고 있는 방법으로 해결할 수 있습니다! 잘 모르겠다면 회의실에서 보시죠 😊
- ViewModel의 transform 메소드 길이
바인딩을 등록하는 메서드의 경우 저는 한 메서드가 여러가지 일을 한다라고 생각하지 않습니다. 해당 메서드는 여러가지 일을 하는 것이 아니라 단지 어떤 일이 일어나면 무엇을 할 것인지라는 바인딩에 충실한 것이니까요. 그래서 메서드 길이가 문제가 되지는 않는다고 생각합니다. 하지만 하나하나의 바인딩 로직이 같은 동작을 해도 어렵고 길게 만들 수도 있고, 반대의 경우도 가능하기 때문에 최대한 간결해야한다고 생각합니다 (특정 이벤트가 발생했을 때 실행되는 로직이므로 오히려 이것이 하나의 메서드처럼 관리되어야 한다고 생각).
- ViewModel에 비즈니스 로직을 넣지 않으려고 했는데요...
현재는 두 타입 모두 테스트 가능하니 큰 문제 없어보이네요. ViewModel은 단지 View에 전달할 내용으로 가공하는 타입이다라고 생각하신다면 UseCase에 일부 옮겨보는 것도 고려해볼 수 있을 것 같습니다.
- UITableViewDelegate 부분 HeaderFooterView 설정
tableView.rx.tableHeaderView에 next 이벤트를 전달해서 뷰를 전달해줄 수는 있겠지만 현재 delegate 메서드처럼 변화가 있을 때마다 호출되어 헤더를 갱신시켜주지는 않을거에요. 이 방식을 채택하려면 추가적인 바인딩이 필요할겁니다. ((todo, doing, done) 갯수 변동 이벤트 발행 -> 필요한 뷰로 매핑하여 tableView.rx.tableHeaderView에 바인딩)
추가
상태값을 할당한 후에 반복적으로 onNext(_:)나 accept(_:)를 호출하고 있다면, 프로퍼티 옵저버를 활용해본 것도 좋을 것 같네요.
계속해서 화이팅입니다!! 💪 궁금한점, 필요한 것 있으시면 언제든지 말씀해주세요!
| <document type="com.apple.InterfaceBuilder3.CocoaTouch.Storyboard.XIB" version="3.0" toolsVersion="19529" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" useAutolayout="YES" useTraitCollections="YES" useSafeAreas="YES" colorMatched="YES" initialViewController="BYZ-38-t0r"> | ||
| <document type="com.apple.InterfaceBuilder3.CocoaTouch.Storyboard.XIB" version="3.0" toolsVersion="19529" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" useAutolayout="YES" useTraitCollections="YES" useSafeAreas="YES" colorMatched="YES" initialViewController="qCv-vy-mOU"> |
There was a problem hiding this comment.
이번 PR에서는 그런 사례가 발생하지 않았지만, 스토리보드는 만지기만 해도 변경사항이 생길 때가 있죠. 이런 점 때문에 협업할 때 사용하기 번거롭다는 의견도 있는데, 왜 그런 현상이 일어나는지, 대략 어떤 내용이 바뀌는지 관심 가져보시는 것도 좋을 것 같네요. 추가로 Interface builder에서 보는 내용이 XML 형식으로는 어떻게 작성되어 있는지도 잡지식 정도로 알아두는건 어떨까요~?
There was a problem hiding this comment.
오.. 조언 감사합니다.
이번 프로젝트의 경우 혼자서 하는 개인프로젝트이기도 해서 과감하게 스토리보드를 선택했었는데요.
협업할 때에는 스토리보드 사용을 좀 더 고려해봐야겠네요.
There was a problem hiding this comment.
스토리보드는 어떤 iOS 개발자가 합류해도 일정한 생산성을 보장할 수 있다는 측면에서 매력적이에요. 스토리보드도 훌륭한 도구입니다!
| @@ -54,6 +55,19 @@ extension DefaultProjectStorage: ProjectStorage { | |||
| func fetch() -> BehaviorSubject<[Project]> { | |||
| return projectStore | |||
| } | |||
|
|
|||
| func fetch(id: UUID) -> Single<Project> { | |||
| guard let project = projects.filter({ $0.id == id }).first else { | |||
| func fetch(id: UUID) -> Single<Project> { | ||
| guard let project = projects.filter({ $0.id == id }).first else { | ||
| return Single.create { observer in | ||
| observer(.failure(StorageError.notFound)) | ||
| return Disposables.create() | ||
| } | ||
| } | ||
| return Single.create { observer in | ||
| observer(.success(project)) | ||
| return Disposables.create() | ||
| } | ||
| } |
There was a problem hiding this comment.
이런건 어떠신가요~?
func fetch(id: UUID) -> Single<Project> {
return Single.create { observer in
guard let project = projects.first(where: { $0.id == id }) else {
observer(.failure(StorageError.notFound))
return Disposables.create()
}
observer(.success(project))
return Disposables.create()
}
}프로젝트 수가 많아진다면 데이터 수에 관계 없이 키 값을 통해 O(1)로 접근 가능한 해시테이블(딕셔너리)로 변경해보는 것도 재미있겠네요. 데이터 수가 많지 않을 것이라고 예상되는 지금 프로젝트에서는 큰 의미는 없을 것 같기는 합니다.
There was a problem hiding this comment.
오...? 이렇게도 줄일 수 있군요. 감사합니다!!
딕셔너리는 고려해봤었는데, 어짜피 로컬을 구현하게 된다면
이 구현부는 없애버릴 생각으로 간단하게 배열로 구현해보았습니다 ㅎㅎ
| enum ProjectState: String, CaseIterable { | ||
| case todo = "TODO" | ||
| case doing = "DOING" | ||
| case done = "DONE" | ||
|
|
||
| var index: Int { | ||
| switch self { | ||
| case .todo: | ||
| return 0 | ||
| case .doing: | ||
| return 1 | ||
| case .done: | ||
| return 2 | ||
| } | ||
| } |
There was a problem hiding this comment.
현재는 ProjectState 인스턴스가 있으면 index로 매핑은 되지만 반대 상황에서 매핑을 지원하지는 않네요. 향후에 그런 소요가 발생한다면 rawValue를 Int로, 현재 사용하는 rawValue를 uppercased()와 같은 메서드를 지원해봐도 좋을 것 같아요. 테이블뷰를 세팅하는 메서드에서 테이블뷰의 index를 알고 있는 상태에서 어떤 ProjectState와 매핑되는지 알아내고 싶을 때 사용할 수 있을 것 같아요.
There was a problem hiding this comment.
엇.. 그렇네요. 생각치 못한 부분인데, 의견 감사합니다.
적극 반영해보겠습니다!!! 😤
| var excluded: [String] { | ||
| switch self { | ||
| case .todo: | ||
| return [ProjectState.doing.rawValue, ProjectState.done.rawValue] | ||
| case .doing: | ||
| return [ProjectState.todo.rawValue, ProjectState.done.rawValue] | ||
| case .done: | ||
| return [ProjectState.todo.rawValue, ProjectState.doing.rawValue] | ||
| } | ||
| } |
There was a problem hiding this comment.
프로퍼티 이름을 통해 자신이 제외되어 있다는 것은 파악이 가능한데, 반환값에 대한 예상이 쉽지 않네요. 프로퍼티 이름에 모든 것을 넣기 어렵다면 문서화 주석을 이용해봐도 좋겠어요.
There was a problem hiding this comment.
오.. 문서화 주석... 감사합니다!
안그래도 프로퍼티 이름에 모든걸 넣기가 애매해서... 저렇게 네이밍 처리를 했었는데... 좋은 방법이 있었네요 🥺
| if date.isPastDeadline { | ||
| dateLabel.textColor = .systemRed | ||
| } else { | ||
| dateLabel.textColor = .black | ||
| } |
There was a problem hiding this comment.
오잉...? 여긴 왜안썼져....
간결하게 표현될 수 있는 조건문일 경우에는 활용해주는 편입니다!
이부분은 리팩토링 해보도록 할게요~!
| @@ -76,7 +76,7 @@ class MemoryUseCaseTests: XCTestCase { | |||
| sut?.create(project) | |||
|
|
|||
| // when | |||
| let result = try? sut?.delete(project2.id).toBlocking().first() | |||
| let result = try? sut?.delete(project2).toBlocking().first() | |||
There was a problem hiding this comment.
유닛 테스트에서는 테스트 함수를 throwing function으로 정의하면 (func someFunction() throws) 에러 처리를 하지 않고도 try 키워드를 사용할 수 있어요. 그렇다면 만약 이렇게 해두고 에러가 발생한다면 어떻게 될까요?
There was a problem hiding this comment.
오... throwing function을 활용해볼 수도 있었군요. 몰랐네요.
찾아보니 실패할경우 테스트가 실패했다고 표시된다고 하네요! 감사합니다 😊
| @@ -93,10 +93,85 @@ class MemoryUseCaseTests: XCTestCase { | |||
| let project3 = Project(title: "실패", description: "실패 내용", date: Date()) | |||
|
|
|||
| // when | |||
| let result = try? sut?.delete(project3.id).toBlocking().first() | |||
| let result = try? sut?.delete(project3).toBlocking().first() | |||
|
|
|||
| // then | |||
| XCTAssertNil(result) | |||
| XCTAssertTrue(try! sut?.fetch().toBlocking(timeout: 1).first()?.count == 2) | |||
There was a problem hiding this comment.
엇.. 그렇네요. 리팩토링 진행해봐야겠어요. 감사합니다 😁
| let text = "1234567889" | ||
|
|
||
| // when | ||
| let result = sut!.isNotValidate(text) |
There was a problem hiding this comment.
이렇게 사용할 계획이라면 sut의 타입을 암시적 옵셔널 해제 타입으로 설정해두는 것도 고려해볼 수 있겠네요.
| enum CoordinatorType { | ||
| case main | ||
| case datil | ||
| } |
There was a problem hiding this comment.
각 코디네이터가 이 타입의 인스턴스를 가지고 있을뿐 이것으로 하는 행동은 없어보이는데, 향후에 어떤 용도로 사용할 계획이신가요?
There was a problem hiding this comment.
해당 타입으로 MainCoordinator 내부에서 childCoordinator를 제거해줄 때 필터링 용도로 활용하고 있습니다!
- 가독성을 위해 보기 어려운 부분을 변수로 따로 할당해주어 리팩토링 - sut을 암시적 옵셔널 타입으로 수정
- fetch(id:) 메소드 내부 간결하게 수정 - 줄바꿈 컨벤션 수정
- 인덱스로 케이스에 접근할 수 있도록 rawValue를 문자열이 아닌 정수타입으로 수정 - 변수, 메소드에 문서화 주석을 활용하여 퀵헬프를 추가
- enum 타입의 Storyboard를 구현 - StoryboardCreatable 프로토콜 생성 및 UIStoryboard extension 추가 - 각 ViewController에 StoryboardCreatable 프로토콜 준수 및 storyboard 프로퍼티 구현
- SceneDelegate에서 생성후 주입시켜주기 때문에 혼동을 줄 수 있어 제거
|
안녕하세요. 라이언!! 수정사항
이외에 몇가지 답변이 잘 이해가 가지 않는 부분들이 있어서 같이 봐주셨으면 합니다! |
|
아리, 이번 스텝도 훌륭하게 해내셨네요! 👏 질문해주신 부분들에 대해서는 회의실에서 이야기도 나누었고, 이번 리뷰의 코멘트로도 달아두었어요. 구조상 크게 달라져야하는 부분은 없으니 이번 스텝은 현 상태에서 머지하도록 하겠습니다. 고생 많으셨고 다음 스텝에서 만나요~🚀 화이팅!💪 |
안녕하세요. 라이언(@ryan-son) 🦁 아리(@leeari95)입니다~
이번 PR은 ViewModel을 개선하고, View 구성 후 바인딩까지 모두 마쳤습니다.
Rx를 익히는데 어려움이 있어서 늦어졌는데, 최대한 할 수 있는 부분 모두 Rx로 구현마치고 PR 보내드리게 되었습니다.
아직도 클린 아키텍처 부분은 능숙치가 않아서.. 부족한 부분이 있을 것 같은데, 이부분 체크해주시면 감사하겠습니다!
고민했던 점
1. ViewModel을 Input과 Output으로 구분지어 설계
2. Coordinator 패턴 적용
3. Memory leak을 방지
withUnretained()operator를 활용했습니다.궁금했던 것 / 조언을 얻고 싶은 점
1. Coordinator를 적절히 사용했는지??
2. ViewModel에 UIKit이 import 되어있는데 괜찮을까요?
longPressGesture이벤트를 받았을 때UITableViewcell을 ViewModel에게 넘겨주고 있습니다.3. Memory leak 방지를 위한 동작이 올바른지?
DetailViewController내부viewDidDisappear시점에 viewModel과 Coordinator를nil처리 해주고있는데... 괜찮은 방법인지 여쭤보고 싶습니다.4. ViewModel의 transform 메소드 길이
5. ViewModel에 비즈니스 로직을 넣지 않으려고 했는데요...
6. UITableViewDelegate 부분 HeaderFooterView 설정