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

프로젝트 매니저 [STEP 2-1] maxhyunm #305

Open
wants to merge 20 commits into
base: ic_9_maxhyunm
Choose a base branch
from

Conversation

maxhyunm
Copy link

안녕하세요 하비(@havilog)!
프로젝트 매니저 STEP 2의 첫번째 PR을 올려드립니다...!
2-1번째인 이번 스텝은 테이블뷰에 데이터 뿌리기까지만 먼저 진행해보라는 하비의 말씀을 참고하여,
첫번째 뷰컨트롤러 화면에서 진행할 수 있는 내용까지 쪼개어 진행하였습니다.
(데이터 바인딩하여 화면에 뿌리기 & 스와이프로 삭제)
자세한 내용은 아래 정리하겠습니다!

고민했던 점

🔹 CoreDataManager

이번 프로젝트에서 로컬DB로 CoreData를 사용하기로 정했기 때문에, 이번 스텝에서는 CoreData 엔티티를 생성하고 관련 CRUD 로직을 처리할 수 있는 CoreDataManager 타입을 만들었습니다. (CoreDataManager는 다양한 엔티티와 함께 활용 가능하도록 제네릭 타입으로 구현하였습니다.)
그런데 STEP 2 요구사항 중에 디스크 저장 없이 메모리에만 저장하는 형식으로 구현하라는 부분이 있어, saveContext에서 실제 save()를 호출하는 부분을 주석처리해 두었습니다.

🔹 DB 엔티티 구성

TODO / DOING / DONE 을 각각의 엔티티로 나눌지, 아니면 status attribute를 주어서 NSPredicate를 통해 필터링해 구분할지를 고민하였습니다. 각 할일의 상태가 변할 때마다 데이터를 지우고 새로 쓰는 것보다는 상태값만을 수정하는 편이 데이터 유실 등의 위험이 적다고 판단되어 후자를 선택하였습니다.

🔹 MVVM 구현 방식

UIKitMVVM 관련 자료나 레포를 찾아보니 Clean Architencture를 적용하거나 Combine과 함께 사용한 케이스가 많았습니다. 하지만 이번 단계에서는 Combine이나 RxSwift까지 생각하지 않고 진행해보라는 조언을 주셨었기에 일부러 해당 방식은 배제를 하고 진행해보려 했습니다. 결과적으로 Observable 타입을 만들어 바인딩해주는 방식으로 진행하였습니다.

class Observable<T> {
    private var listener: ((T) -> Void)?
    
    var value: T {
        didSet {
            listener?(value)
        }
    }
    
    init(_ value: T) {
        self.value = value
    }
    
    func bind(_ closure: @escaping (T) -> Void) {
        closure(value)
        listener = closure
    }
}

Observable 타입은 위와 같이 생성하였습니다. 이처럼 구현하고 bind 메서드를 통해 listnerescaping 메서드를 지정해 주면, 해당 Observable 타입으로 감싼 value 값이 변경이 될 때마다 listner 메서드를 실행하게 됩니다. 이를 통해 구현부에서 값이 변할 때마다 특정 행위를 할 수 있도록 지정할 수 있습니다.

ViewModel의 경우 아래와 같이 프로토콜을 생성하여 구현하였습니다.

protocol ViewModelProtocol {
    var dataList: Observable<[ToDoStatus: [ToDo]]> { get set }
    var errorMessage: Observable<String?> { get set }
    var error: Observable<Bool> { get set }
    func fetchData()
    func createData(title: String?, body: String?, dueDate: Date?)
    func updateData(_ entity: ToDo, title: String?, body: String?, dueDate: Date?)
    func deleteData(_ entity: ToDo)
    func setError(_ message: String)
}

fetch/create/update/delete는 데이터를 알맞게 변형하여 CoreDataManager에 지정된 CRUD 메서드를 호출합니다.
데이터를 성공적으로 불러왔을 경우는 dataList의 값을 바꿔주고(이때 dataList에 바인딩된 listner를 호출하여 UI를 그림), 중간에 에러가 발생한 경우는 errorMessage를 변경합니다(이때 errorMessage에 바인딩된 listner를 호출하여 alert 생성).

🔹 AlertBuilder

Alert 생성시 반복적인 작업을 조금이나마 줄이고 정리하고자 AlertBuilder를 생성하였습니다. 자주 사용되는 actionAlertActionType으로 정리하였습니다.

🔹 TableView와 헤더를 담은 UIView 생성

이번 프로젝트에서는 하나의 View Controller 안에 세 개의 Table View가 들어갑니다. 각 Table View는 헤더 타이틀과 롱프레스 액션 선택지를 제외하면 동일한 모양을 하고 있습니다. 때문에 저는 Table View와 헤더를 하나의 View로 분리하여 반복적으로 활용하면 좋을 것 같다는 생각을 하였습니다. 그래서 ToDoListView를 생성하였고, 해당 뷰에서 반복적인 테이블뷰 로직들을 처리하고 있습니다.(날짜가 지난 할일은 빨간색으로 바꾸는 로직, 왼쪽으로 스와이퍼하여 삭제하는 로직 포함)

🔸 (아직 고민중인 점) View와 ViewModel 연결

각 테이블뷰마다 ViewModel을 가져야 할지, 아니면 ViewController에만 ViewModel을 연결하고 각각의 테이블뷰에는 해당 정보만을 뿌려줄지에 대해 고민을 하였습니다.
하지만 롱프레스 액션을 구현할 경우 ToDo에서 Doing으로, 또는 Done으로 데이터가 이동해야 하는데, 이때 각 뷰모델의 dataList를 연동할 방법을 찾기가 어려웠습니다. 때문에 View Controller만 ViewModel과 엮어주고, 해당 데이터에 변동이 생기면 세 가지 테이블뷰를 모두 reload하도록 구현해 두었습니다.
이 부분은 아직도 고민이 많이 되는 부분이라서 자신이 없습니다😭 조언 주시면 감사하겠습니다!!

@maxhyunm maxhyunm changed the title 프로젝트 매니저 [STEP 2] maxhyunm 프로젝트 매니저 [STEP 2-1] maxhyunm Sep 25, 2023
Copy link

@havilog havilog left a comment

Choose a reason for hiding this comment

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

수고하셨어요 맥스

저는 뷰는 최대한 쪼개는게 좋아서,
화면 하나라고 뷰컨이 하나일 필요는 없을거 같아요

화면 하나에도 10개의 뷰컨과 뷰모델을 가지고, 그 아이들을 조합한 부모 뷰컨이 있는게 좋다고 생각합니다

ProjectManager/ProjectManager/Model/ToDoStatus.swift Outdated Show resolved Hide resolved
case doing = 1
case done = 2

var name: String {
Copy link

Choose a reason for hiding this comment

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

요런 아이들은 customStringConvertible같은 프로토콜을 써도 괜찮을거 같아요

Copy link
Author

@maxhyunm maxhyunm Sep 25, 2023

Choose a reason for hiding this comment

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

CustomStringConfertible을 전에 활용해 보았을 때, String 타입이 들어가야 하는 경우에는 결국 .description으로 접근해야 했던 기억이 있어서 연산 프로퍼티로 진행했었습니다!(이 부분은 제가 잘못 알고 있는 것이라면 말씀 부탁드립니다 😭)
그런데 string 값으로 관련 status 값을 가져와야 하는 경우가 발생해서, name 대신 rawValue를 사용하는 쪽으로 수정하였습니다.

Copy link

Choose a reason for hiding this comment

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

네넵 사실 rawValue나 description이나 취향차이고 거기서 거기긴 합니다 ㅎㅎ;

ProjectManager/ProjectManager/Model/Observable.swift Outdated Show resolved Hide resolved

import CoreData

class CoreDataManager {
Copy link

Choose a reason for hiding this comment

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

class로 선언한 이유가 있을까요?

Copy link
Author

@maxhyunm maxhyunm Sep 25, 2023

Choose a reason for hiding this comment

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

하나의 매니저를 만들면 동일 인스턴스를 여기저기서 공유하게 되기 때문에 class로 생성했습니다! 다만 해당 타입 내부에 있는 프로퍼티가 let 뿐이라서 struct로 생성해도 문제가 없었을 것 같네요... struct로 수정하겠습니다!

Copy link

Choose a reason for hiding this comment

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

넵 그리고 class는 무조건 final! 습관 들여주세요

ProjectManager/ProjectManager/List/ToDoListView.swift Outdated Show resolved Hide resolved
ProjectManager/ProjectManager/List/ToDoListView.swift Outdated Show resolved Hide resolved
Comment on lines 98 to 100
self.toDoView.reloadTableView()
self.doingView.reloadTableView()
self.doneView.reloadTableView()
Copy link

Choose a reason for hiding this comment

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

모든 데이터를 reload하지 않고 사용할 수 있는 방법에 대해 고민하면 좋을 듯 합니다.

Copy link
Author

@maxhyunm maxhyunm Sep 25, 2023

Choose a reason for hiding this comment

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

list를 status 값에 따라 세 개로 나누어 각각의 리스트에 바인딩해주는 방식으로 수정하겠습니다...!

Copy link

Choose a reason for hiding this comment

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

각각 바인딩 하는 방식도 의도한 거긴 한데요,
reload를 부르는 방법이 아닌, performBatchUpdate를 하거나
delete update 같은 방식도 고민해보시면 좋을 듯 합니다.

@maxhyunm
Copy link
Author

maxhyunm commented Oct 3, 2023

안녕하세요 하비(@havilog)!
코멘트 주셨던 부분 기반으로 수정 진행하여 push 완료하였습니다!
자잘한 부분들은 위에 코멘트에 답변한 대로 수정을 마쳤고, 아래 부분은 추가로 설명이 필요할 듯하여 따로 정리해 드립니다!

🔹 뷰컨트롤러와 뷰모델 나누기

뷰컨트롤러와 뷰모델을 최대한 쪼개는 것이 좋을 것 같다는 말씀을 듣고 고민하다가,
status마다 뷰컨트롤러와 뷰모델을 따로 생성하여 base 뷰컨트롤러에 child로 추가하는 방식으로 수정하였습니다.
부모-자식 뷰모델 간에는 delegate를 적용하여 자식 뷰모델에서도 부모 뷰모델을 통해 데이터를 생성/수정/삭제할 수 있도록 하였습니다.

🔹 배치 업데이트

테이블뷰에 배치 업데이트를 적용하려면 데이터의 생성/수정/삭제에 따라 각기 다른 작업을 진행해야 하는 것을 확인하였습니다. (예: delete를 진행할 경우 해당 row를 골라 지워주어야 함) 이 부분에 있어 많은 고민을 하였는데... 결과적으로는 create, update, delete의 상태값을 가질 수 있는 Action이라는 타입을 생성하여 자식 뷰모델에서 Observable로 action 프로퍼티를 갖고 있도록 하였습니다.
기존에는 엔티티 배열 자체를 Observable 처리하여 배열이 바뀔 때마다 reload하도록 바인딩했다면, 지금은 이 action 값이 바뀔 때마다 상태값에 따라 다른 행위를 할 수 있도록 바인딩 처리를 하였습니다.

🔹 기타

데이터 추가시 제대로 화면이 바뀌는지 테스트하기 위해 BaseViewController의 + 버튼을 누르면 더미데이터가 생성되도록 임시 코드를 추가해 두었습니다.(#if DEBUG 처리하였고, STEP 2-2 작업시 삭제 예정입니다!)

Copy link

@havilog havilog left a comment

Choose a reason for hiding this comment

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

사실 의도는 하나의 todo를 만들고
여러개의 todo로 확장하면서
어떻게 확장성 있게 처음부터 설계할 수 있을지도 중요했을거 같은데
지금 상태에서 뭔가 요구사항을 더 수행하기보다는
viewmodel을 어떻게 develop할 수 있을지 고민해보시면 좋을 듯해요

Comment on lines 20 to 39
func setControllerTitle(title: String) {
self.controllerTitle = title
}

func setControllerMessage(message: String) {
self.controllerMessage = message
}

func addAction(_ actionType: AlertActionType, action: ((UIAlertAction) -> Void)? = nil) {
let action = UIAlertAction(title: actionType.title, style: actionType.style, handler: action)
alertActions.append(action)
}

func makeAlertController() -> UIAlertController {
alertController.title = controllerTitle
alertController.message = controllerMessage
alertActions.forEach { alertController.addAction($0) }

return alertController
}
Copy link

Choose a reason for hiding this comment

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

alertConfiguration 같은 struct 를 받아서,
파라미터로 해당 config(액션을 포함하거나 분리해서)를 받은 뒤,
받은 액션과 config들로 알럿을 띄운다면
AlertBuilder가 struct가 되고, 인터페이스도 하나로 통일될 거 같은데
어떻게 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

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

Nested Type으로 Configuration 타입을 생성하여 builder 타입변환해주었습니다! 또 각 설정 메서드에 self를 반환하도록 하여 .으로 메서드가 연결될 수 있도록 구현해 보았습니다.

// Created by Max on 2023/09/26.
//

final class KeywordArgument {
Copy link

Choose a reason for hiding this comment

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

요 친구도 struct여도 되지 않나요?

Copy link
Author

Choose a reason for hiding this comment

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

맞네요😂!! 변경하였습니다...!

Comment on lines 10 to 17
protocol ViewModelType {
associatedtype ViewModelError

var error: Observable<ViewModelError?> { get set }

func handle(error: Error)
func setError(_ error: ViewModelError)
}
Copy link

Choose a reason for hiding this comment

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

뷰 모델의 input과 output을 나눠서 정의해보는건 어떨까요?
뷰모델의 설계는 지금 워낙 제각각이긴 한데,

저는 Redux패턴이 가장 좋은거 같아요
대표적으로 TCA, ReactorKit 등이 있구요

input/output까지만 명시되는 뷰모델,
reduce까지해서 상태기반으로 정의되는 뷰모델
은 다르니까 고런 부분도 공부해보시면 좋을 듯 싶습니다~

Copy link
Author

Choose a reason for hiding this comment

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

input/output 구분에서 정말 오랜 시간 고민을 하게 되었습니다 😭
다양한 패턴을 들여다보다 보니 실질적인 구현에 있어서 헷갈리는 부분이 많아
결과적으로 kickstarter의 Protocol 활용한 input/output 구분 방식을 채택하였는데 잘 정리가 되었는지 모르겠네요...
앞으로 말씀주신 부분도 참고하여 열심히 공부해보도록 하겠습니다!!

}

override func viewWillAppear(_ animated: Bool) {
viewModel.fetchAllData()
Copy link

Choose a reason for hiding this comment

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

뷰는 뷰모델에서 어떤 역할을 하는지까지는 몰라도 될 것 같아요
뷰는 어떤 액션이 뷰에서 일어났다 까지만 정의해주고
그 액션이 일어났을때 -> 어떤 일을 할거다 를 뷰모델에서 정의해주세요

Copy link
Author

Choose a reason for hiding this comment

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

이 부분 제가 완전히 잘못 이해하고 있었던 것 같아서, 하비 말씀 듣고 나서야 조금 방향이 잡힌 것 같습니다😭
뷰모델이 뷰의 행위에 맞춰서 호흡을 같이할 수 있도록 메서드 형태를 전체적으로 수정하였습니다.
이 과정에서 UseCase라는 이름으로 ViewModel 내부 로직 일부가 분리되었는데(ToDo 데이터에 대한 공통 전처리로 뷰모델마다 반복되는 부분) 사실상 이게 UseCase가 맞는가...라는 고민은 들지만 달리 이름을 붙이기가 애매하여😭 일단 그렇게 네이밍을 하였습니다.

Comment on lines 104 to 109
let alertBuilder = AlertBuilder(prefferedStyle: .alert)
alertBuilder.setControllerTitle(title: error.alertTitle)
alertBuilder.setControllerMessage(message: error.alertMessage)
alertBuilder.addAction(.confirm)
let alertController = alertBuilder.makeAlertController()
present(alertController, animated: true)
Copy link

Choose a reason for hiding this comment

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

위에서 코멘트 단 대로
let alertBuilder = AlertBuilder.build(title, message, action...) 하나로도 가능해보여요

Comment on lines 61 to 65
override func prepareForReuse() {
titleLabel.textColor = .black
bodyLabel.textColor = .black
dateLabel.textColor = .black
}
Copy link

Choose a reason for hiding this comment

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

text도 초기화해야하지 않을까요?

Copy link
Author

Choose a reason for hiding this comment

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

앗 이 부분을 놓쳤네요! 추가했습니다!

@maxhyunm
Copy link
Author

maxhyunm commented Oct 6, 2023

안녕하세요 하비(@havilog)!
추가로 주신 코멘트 기반으로 많은 고민 끝에 수정을 올려드립니다.
아래와 같은 내용들이 주로 변경되었습니다.

🔹 AlertBuilder 타입 변경

코멘트주신 내용을 토대로 Configuration 타입을 나누어 Builder는 struct가 될 수 있도록 하였습니다. 또한 값을 세팅하는 메서드마다 return self를 추가하여 .으로 모든 메서드가 연결될 수 있도록 변경하였습니다.

🔹 ViewModel을 Input/Output으로 구분

POP를 활용하여 각 뷰모델의 Input에서 진행할 내용과 Output에서 전달받을 데이터를 구분하였습니다. 이를 통해 View에서는 Input/Output으로 지정된 내용으로만 접근할 수 있도록 제한하였습니다.

🔹 View의 상태/행위에 맞춰 ViewModel을 구성

View에서는 ViewModel에서 어떤 것을 하는지 몰라도 된다는 말씀을 듣고 고민한 끝에, ViewModel의 구성을 View의 상태나 행위에 맞춰 진행할 수 있도록 변경하였습니다. 이에 따라 viewDidLoad에서 호출되는 내용, addChildren에서 호출되는 내용 등으로 메서드를 구분하였습니다.

🔹 ViewModel의 데이터 전처리 로직 분리

ViewModel들이 모두 ToDo 엔티티를 활용하다보니 동일한 전처리가 들어가야 하는 부분들이 계속 반복되었습니다. 이 부분을 매번 delegate로 처리하는 것도 애매하다고 생각되어, delegate에서는 반드시 연결되어야 하는 부분(예: status가 바뀌었을 때 예전 테이블뷰와 새로운 테이블뷰가 모두 변경사항을 적용할 수 있도록 하기)에만 사용하고 나머지 로직은 UseCase로 분리하였습니다.
이에 따라 ViewModel에서는 CoreDataManager 대신 ToDoUseCase만 가지고 있도록 변경하였습니다.

🔹 status 변경용 PopOverView 추가

프로젝트 진행은 멈추라고 하셔서 이 부분은 사실 작업하지 않으려 하였는데... status 변경시 두 가지 테이블뷰에 적용이 되는지 여부를 확인하고 싶어서 추가 진행하였습니다.

아직도 많이 부족하지만, 하비 덕분에 많은 공부를 할 수 있었습니다!
확장성 있는 설계와 ViewModel을 나누는 다양한 패턴에 관해서는 더 많은 공부가 필요할 것 같습니다.

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

Successfully merging this pull request may close these issues.

2 participants