프로젝트 매니저 [STEP 2] Zion, karen #317

karenyang835 · 2023-10-06T06:51:21Z

@delmaSong

안녕하세요. 델마~
Zion, karen🦦입니다.

최대한 요구사항에 맞추어 작성하기 위해 많이 늦어졌습니다.🥹
Step02 PR 보냅니다.

리뷰 잘 부탁드립니다.🙇🏻‍♀️

🤔 고민한 부분

1️⃣ CollectionView VS TableView

UI구현을 CollectionView로 할 것인지 TableView로 할것인지에 대해서 고민을 했었는데, 요구사항에 나오는 사항은 List형식이기 때문에 굳이 CollectionView까지 가야되는건가 고민을 했었습니다.
처음에는 TableView로 구현을 했었다가 각 일정을 생성시에 Cell간의 간격을 주는게 CollctionView에서 설정하기가 더 수월했고, UI를 그리고 표현함에 있어서 더 다양한 유연성을 제공하기에 추후에 UI가 변동되거나 리팩토링 될 경우에도 대응이 수월할 것 같다 판단되어서 CollectionView로 적용하였습니다.

2️⃣ DiffableDataSource Header 갱신

DiffableDataSource를 사용하면서 Header를 갱신하기위해 여러 방법들을 고민했습니다. 특히 기본적으로 제공하는 snapShot.reloadSection 및 Section을 매번 새로운 UUID로 주는 방법을 고민해봤습니다만 부자연스러운 애니메이션의 발생 및 불필요한 동작으로 인해 결국 ListViewController에서 HeaderView를 들고있을 수 있도록 구현했습니다. 이로인해 taskList가 갱신될 때 마다 ListViewController에서 HeaderView에 직접적으로 접근하여 taskCount을 갱신시켜줄 수 있었습니다.

🙇🏻‍♀️ 조언을 얻고 싶은 부분

1️⃣ `Edit`일 경우 표시되는 메뉴버튼

edit을 할 경우에 상단에 위치하는 버튼이 Cancel은 그대로 두고 edit와 done이 변동되어져야하는게 아닌가 싶은데 델마가 보시기에는 어떠신가요?
edit와 done 같은 기능을 하는데 두개가 나란히 같이 있는건 무의미해 보여서요.일단 edit으로 시작하면 어찌되었든 데이터를 전송하게 되는것 같아서 취소를 할 수가 없는 상황인 것 같습니다.

2️⃣ MVP, MVVM의 차이

MVC, MVP, MVVM과 같은 아키텍처들의 역할 및 목적에 대해 파악하고 비교하는 것에 가장 많은 시간을 들였습니다. 특히 MVP와 MVVM의 차이점에 대해 이해하고 데이터 바인딩을 할 수 있는 방법에 대해 생각하고 고민해봤습니다.

제가 생각하는 View Model의 가장 큰 특징은 데이터 바인딩입니다. 하지만 데이터 바인딩을 할 수 있는 여러가지 방법들을 생각해본다면 delegate Pattern도 데이터 바인딩을 할 수 있는 하나의 방법이라고 생각합니다. MVP에서는 Presenter가 delegate를 통해 View에 UI를 갱신해야한다는 이벤트를 전달하는 아키텍처로 알고 있습니다. 그랬을 때 MVP와 MVVM의 분명한 차이가 발생하지 않는다고 생각합니다.

MVP와 MVVM을 구분할 수 있는 다른 특징을 알고 싶습니다.

delmaSong

수고많으셨어요~!
뷰모델의 특징이 데이터 바인딩도 있지만, 역할 분리 차원에서 생각해보면 어떨까요?
지금 UseCase가 하는일을 좀 더 확장해본다면 어떨까요?
혹은 cell 내에서 dateFormatter를 가지고 계산하는 일들을 뷰모델이 할 수도 있지 않을까요?
모든 뷰컨이 가지고있는 taskList를 한군데에서 관리되도록 하는건요?
역할분리 관점에서 다시 바라보시면 좋을 것 같습니다. 수고많으셨어요!

아 그리고 아래 이미지 확인해주세요~!

delmaSong · 2023-10-07T09:12:34Z

ProjectManager/ProjectManager/Controller/ListViewController.swift

+enum ListKind: String {
+    case todo = "TODO"
+    case doing = "DOING"


ListKind! 이 이름을 봤을 땐 뷰와 관련이 높은 객체라고만 생각이 들어서,
JobStatus나 TaskStatus라는 이름은 어떨까요?

TaskStatus로 변경했습니다!

delmaSong · 2023-10-07T09:18:12Z

ProjectManager/ProjectManager/Controller/ListViewController.swift

+                let deleteAction = UIContextualAction(style: .destructive, title: "Delete") { [weak self] action, view, handler in
+                    guard let self = self else { return }
+
+                    self.didSwipedDeleteTask(deleteTask: self.taskList[indexPath.row])


deleteTask가 중복되어서 wildcard pattern을 이용해도 좋을 것 같네용

중복되는 부분은 wildcard Pattern을 사용했습니다!

delmaSong · 2023-10-07T09:19:06Z

ProjectManager/ProjectManager/Controller/ListViewController.swift

+protocol ListViewControllerDelegate: AnyObject {
+    func didTappedRightDoneButtonForUpdate(updateTask: Task)
+    func didSwipedDeleteTask(deleteTask: Task)
+    func moveCell(moveToListKind: ListKind, task: Task)


from, to 라고 argument label을 이름지어도 좋을 것 같아요 ㅎㅎ

delmaSong · 2023-10-07T09:48:37Z

ProjectManager/ProjectManager/Controller/ListViewController.swift

+    func reloadTaskList(taskList: [Task]) {
+        setUpDiffableDataSourceSanpShot(taskList: taskList)
+        headerView?.setUpContents(title: listKind.rawValue, taskCount: "\(taskList.count)")
+    }
+
+    private func setUpDiffableDataSourceSanpShot(taskList: [Task] = []) {
+        var snapShot = NSDiffableDataSourceSnapshot<Section, Task>()
+
+        self.taskList = taskList
+        snapShot.appendSections([.main])
+        snapShot.appendItems(taskList)
+        diffableDataSource?.apply(snapShot)
+    }


현재 구조면 taskList에 변경이 있을 때마다 NSDiffableDataSourceSnapshot 인스턴스를 생성하게 되네요,
매번 새롭게 생성하는 이유가 있나요?

MainVC에서처럼 ListVC도 taskList에 프로퍼티 옵저버를 다는 건 어떨까요?

apply 함수는 현재의 UI State와 변경될 UI State에서의 차이점을 계산하여 적용되는 것으로 알고 있습니다.
현재의 SnapShot을 불러와서 삽입 및 삭제의 로직을 나누는 것 보다 apply 로직을 하나로 두는 것이 사용하기 쉽고 가독성에도 좋다고 생각했습니다.

현재 taskList에 대한 write와 관련된 작업은 전부 MainVC에서 발생하고 있습니다!.
따라서 MainVC에서 프로퍼티 옵저버를 다는 것이 한군데서 옵저빙하는 방법이 되기 때문에 좋은 방법이 될 수 있지 않을까 합니다.

delmaSong · 2023-10-07T09:52:56Z

ProjectManager/ProjectManager/Controller/MainViewController.swift

+extension MainViewController {
+    private func didTappedRightAddButton() {


Right~ 라는 단어는 빼도 좋을 것 같아요.
addButton이 꼭 오른쪽에 있으리란 법도 없고, 디자인 상 왼쪽이나 가운데로 수정이 된다면 불필요하게 변수도 같이 수정되어야 하겠어요.

7cf5bf1

right와 left가 들어가 있는 부분을 전부 삭제해주었습니다.

delmaSong · 2023-10-07T10:03:12Z

ProjectManager/ProjectManager/Controller/MainViewController.swift

+extension MainViewController: ListViewControllerDelegate {
+    func moveCell(moveToListKind: ListKind, task: Task) {
+        taskList = useCase.convertUpdatedTaskList(taskList: taskList,
+                                                  updateTask: task,
+                                                  moveTolistKind: moveToListKind)
+    }


이 작업만 UseCase에서 하는 이유가 있나요?
카렌과 시온이 생각하는 UseCase란 뭔가요?

뷰를 그리는 로직 및 사용자의 이벤트를 처리하는 로직 이외에는 전부 비즈니스 로직이라고 생각하고, UseCase에서는 이러한 비즈니스로직을 담당해야한다고 생각합니다. moveCell 메서드와 같이 단순히 TaskList를 Converting해주는 로직은 UseCase에서 담당해야한다고 생각합니다.

delmaSong · 2023-10-07T10:03:41Z

ProjectManager/ProjectManager/Controller/TaskViewController.swift

+        case update
+    }
+
+    weak var  delegate: TaskViewControllerDelegate?


var와 delegate 사이에 띄어쓰기가 두번 된거같아요

7cf5bf1

수정해 주었습니다.

delmaSong · 2023-10-07T10:07:38Z

ProjectManager/ProjectManager/Controller/TaskViewController.swift

+    private let placeHolderLabel: UILabel = {
+        let label = UILabel()
+
+        label.text = "여기는 할일 내용 입력하는 곳이지롱\n입력 가능한 글자수는 1000자로 제한합니다"


실제로 1000자로 제한하는 부분은 어디에 있나요?

7240a33

저희가 누락한 것 같습니다. 코드 추가하여 적용해주었습니다.

delmaSong · 2023-10-07T10:11:09Z

ProjectManager/ProjectManager/View/ListCollectionViewCell.swift

+    private let dateFormatter: DateFormatter = {
+        let dateFormatter = DateFormatter()
+
+        dateFormatter.dateFormat = "yyyy. MM. dd."
+        dateFormatter.locale = Locale.current
+        dateFormatter.timeZone = TimeZone.current
+        return dateFormatter
+    }()


모든 셀이 꼭 dateFormatter를 가지고 있어야하나요?
여기서는 전달받은 날짜값만 보여줘도 적절할 것 같은데, 어떻게 생각하세요?

이부분은 ListViewControllerUseCase를 부여해서 해결했습니다.
이부분에 대해서는 위에 주신 ViewModel의 코멘트와 관련해서 구두로 대화를 나눠보고 싶습니다 델마 따로 DM으로 요청드릴게요!

delmaSong · 2023-10-07T10:12:10Z

ProjectManager/ProjectManager/View/ListCollectionViewCell.swift

+    private func isPassDeadline(deadline: Double) -> Bool {
+        let currentDateString = dateFormatter.string(from: Date())
+        let deadlineDateString = dateFormatter.string(from: Date(timeIntervalSince1970: deadline))
+        let currentDate = dateFormatter.date(from: currentDateString) ?? Date()
+        let deadlineDate = dateFormatter.date(from: deadlineDateString) ?? Date()
+
+        return currentDate.timeIntervalSince1970 > deadlineDate.timeIntervalSince1970
+    }


이것도 모든 셀에서 계산할 일인가 싶긴 하네요!
셀이 계산하지 않고 전달받은 값만 보여주고, 값을 계산하거나 관리하는 건 더 상위의 역할에서 하는 게 어떨까요

이부분 또한 상위 타입에서의 UseCase로 옮겨 해결했습니다!

LeeZion94 and others added 18 commits October 3, 2023 14:21

feat: Cell, TaskViewController 구현 및 프로젝트 기초 구성 구현

d8669fd

feat: ListViewController 레이아웃 구현

9c67f84

feat: ListViewController DiffableDataSource 적용

f803ddd

feat: MainViewController 레이아웃 구현

eb6c675

feat: Task 생성시 MainViewController에서 ListViewController를 갱신하도록 변경

39b8121

fix: 불필요 타입 삭제

549bf84

feat: taskCountLabel 라운딩 적용

f91a534

feat: shadow적용

c37229d

feat: Task Update로직 추가

34238a3

feat: deadline에 따른 텍스트 색상변경 적용

386a36e

feat: isPassDeadline 로직 수정

5acaed5

fix: descriptionLabel 줄 수 제한 적용

eb4c7ae

feat: swipeDelete 로직추가

1118c27

feat: trailingAction 적용

353c110

feat: Cell Move 로직추가

2d97e3d

fix: Section 갱신로직 수정

7f1c36f

feat: showToast 로직 추가

c3d1160

feat: MainViewController UseCase 생성

4a0ff41

delmaSong self-requested a review October 7, 2023 08:35

delmaSong reviewed Oct 7, 2023

View reviewed changes

karen and others added 10 commits October 8, 2023 15:14

refactor: 불필요한 컨벤션 삭제, 네이밍 수정

7cf5bf1

feat: descriptionTextView글자수 제한 적용

7240a33

refactor: listKind -> TaskStatus로 수정

e090224

refactor: deleteTask 중복 노출 수정

28957b6

feat: ListViewControllerUseCase 생성

2933395

fix: 제목만 들어간 경우 titleLoad 불가 수정

01c3cf6

fix: TextView 영역을 벗어나서 노출되는 텍스트 안보이도록 수정

0787d98

refactor: argument label to로 수정

9adceec

refactor: convertTitle부분 UseCase로 옮김

076b3e0

Update README.md

587b1f6

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

프로젝트 매니저 [STEP 2] Zion, karen #317

프로젝트 매니저 [STEP 2] Zion, karen #317

karenyang835 commented Oct 6, 2023 •

edited

Loading

delmaSong left a comment •

edited

Loading

delmaSong Oct 7, 2023

LeeZion94 Oct 8, 2023

delmaSong Oct 7, 2023

LeeZion94 Oct 8, 2023

delmaSong Oct 7, 2023

delmaSong Oct 7, 2023

delmaSong Oct 7, 2023

LeeZion94 Oct 8, 2023

delmaSong Oct 7, 2023

karenyang835 Oct 8, 2023

delmaSong Oct 7, 2023

LeeZion94 Oct 8, 2023

delmaSong Oct 7, 2023

karenyang835 Oct 8, 2023 •

edited

Loading

delmaSong Oct 7, 2023

karenyang835 Oct 8, 2023

delmaSong Oct 7, 2023

LeeZion94 Oct 8, 2023

delmaSong Oct 7, 2023

LeeZion94 Oct 8, 2023

		extension MainViewController {
		private func didTappedRightAddButton() {

프로젝트 매니저 [STEP 2] Zion, karen #317

Are you sure you want to change the base?

프로젝트 매니저 [STEP 2] Zion, karen #317

Conversation

karenyang835 commented Oct 6, 2023 • edited Loading

🤔 고민한 부분

1️⃣ CollectionView VS TableView

2️⃣ DiffableDataSource Header 갱신

🙇🏻‍♀️ 조언을 얻고 싶은 부분

1️⃣ Edit일 경우 표시되는 메뉴버튼

2️⃣ MVP, MVVM의 차이

delmaSong left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karenyang835 Oct 8, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

karenyang835 commented Oct 6, 2023 •

edited

Loading

1️⃣ `Edit`일 경우 표시되는 메뉴버튼

delmaSong left a comment •

edited

Loading

karenyang835 Oct 8, 2023 •

edited

Loading