[Refactor] #111 - RegisterNormalView Refactor #116

Naknakk · 2024-02-03T12:44:44Z

⭐️Issue

#111

🌟Motivation

RegisterNormalView를 Refactor 했습니당.

🌟Key Changes

멘토님 피드백을 바탕으로 수정했으며, 중복된 코드 제거, UIView에 들어있던 비즈니스 로직 ViewController로 이동 등의 변화가 있습니다.

멘토님 피드백 기반
- 등록/수정 완료 버튼에 throttle 추가로 연타 방지
- Service에서 사용한 강제 try! 문을 do-catch로 에러처리 해주었습니다.
- 매직넘버 줄이기(완전히 다 없애진 못했습니당.. 너무 자잘하고 당연한 것들로 보여서 굳이 없애지 않아도 되나..? 하는 것들이 남아있어서 고민입니당.)
- 접근 제어자 수정
- URL 처리시 직접 문자열을 작성하는 부분을 없애고, 메서드를 통해 받아오도록 하였습니다.
- 고차함수를 활용하여, for + append 에서 map으로 변경하였습니다.
함께 회의했던 것들 기반
- MARK 주석 양식 변경에 따른 메서드 분리 및 위치 정리를 했습니다.
- 특히 RxSwift 부분에서 Input / Output / Navigation 세가지로 분류하기 위해 노력했습니다. (곰튀김님이 세가지로 분류하셨길래 참고했습니당.)
- UIViewController+ 파일에 VC간 이동 함수를 추가하여, 화면 이동 함수를 깔끔하게 작성할 수 있었습니다.
개인적으로 아쉬웠던 부분들
- View Depth가 너무 깊어 직관적이지 못하다고 판단해, 다시 몇몇 View를 합쳐 Depth를 줄였습니다.
- DataPickerView에 들어있던 비즈니스 로직을 ViewController로 최대한 옮겼습니다.
- platformList 데이터가 View에 있던 것을 ViewController로 옮겼습니다.
- ViewController에서 급하게 작성하느라 중복된 코드를 최소화 하였습니다.
- RxSwift 순환 참조 방지를 위해 모든 구독 함수 (subscribe, bind, drive)를 with: self , onNext: 로 바꾸었습니다.
- UI관련된 부분은 모두 Relay + Driver를 통해 메인스레드 작동&에러 무시 처리를 하였습니다.

🌟Simulation

동작이 달라진 부분은 없습니다.

🌟To Reviewer

접근 제어자를 멘토님이 필요에 따라 private(set)으로 바꾸는게 어떠냐 하셨는데, private(set)의 경우 var로만 선언할 수 있어, 해당 클래스 내에서 변수로 선언해야만 합니다. 근데 UI Components의 경우 내부든 외부든 바꿀일이 없어 상수로 선언하고 싶었기 때문에.. 그냥 internal(기본) let으로 선언하였습니다.
코드를 전체적으로 옮기고 하는 일이 잦아서 변경사항이 굉장히 많아졌기 때문에.... 추가 기능 구현 이전에 리팩토링 한 것만 PR 우선 올립니다.
팩토리 패턴은 아래와 같이 적용할 수 있을 것 같은데, ModuleFactory 클래스를 어느 폴더에 작성해야할지 고민입니다. 어디에 만드는게 좋을까요?!? 일단 제 생각으로는 Resource 폴더 내에 Factory 폴더를 만들어도 좋을 것 같습니다..!

🌟To do

밀어서 뒤로가기 기능은 지원이 PR이 머지된 이후에 추가하겠습니당.

🌟Reference

https://ios-development.tistory.com/176 (throttle, debounce)
https://stackoverflow.com/questions/31710210/difference-between-var-privateset-and-let (private(set) vs. let)
https://gyuios.tistory.com/289 (withUnretained에서 with: self, onNext: ~ 로 변경된 이유)
https://so-kyte.tistory.com/206 (MVVM Input, Output)
https://codingmon.tistory.com/41 (Factory Pattern, 멘토님이 첨부하셨던 링크)

Guryss

수고하셨서용!!!! 굿뜨

VC보니까 Input, Output 구분 되게 잘해놓으신거 같더라구요??
제가 지금 그 부분에서 어려움을 겪고 있어서 ....,,, 제 코드 보고 리뷰 남겨주십쇼 참고로 Input으로 어떻게 빼야할지 모르겠음 ㅠㅠ

Guryss · 2024-02-05T07:16:22Z

WSSiOS/WSSiOS/Network/Novel/NovelService.swift

+    func getNovelInfo(novelId: Int) -> Single<NovelResult> {
+        do {
+            let request = try makeHTTPRequest(method: .get,
+                                               path: URLs.Novel.getNovelInfo(novelId: novelId),
+                                               headers: APIConstants.testTokenHeader,
+                                               body: nil)
+
+            NetworkLogger.log(request: request)
+
+            return urlSession.rx.data(request: request)
+                .map {NovelResult(
+                    newNovelResult: try? JSONDecoder().decode(NewNovelResult.self, from: $0),
+                    editNovelResult: try? JSONDecoder().decode(EditNovelResult.self, from: $0))
+                }
+                .asSingle()
+        } catch {
+            return Single.error(error)
+        }


W3;
do-catch 사용 굿굿!!! 나두 이렇게 처리했다능 !!!

Guryss · 2024-02-05T07:16:45Z

WSSiOS/WSSiOS/Resource/Constants/URLs/URLs.swift

+        static func postUserNovel(novelId: Int) -> String {
+            return "/user-novels/\(novelId)"
+        }
+        static func patchUserNovel(userNovelId: Int) -> String {
+            return "/user-novels/\(userNovelId)"
+        }


Guryss · 2024-02-05T07:21:02Z

...erNormal/RegisterNormalView/RegisterNormalAssistantView/RegisterNormalNovelSummaryView.swift


    private func setHieararchy() {


W1;
setHierarchy 같은데!!! 오타난듯하오 수정해주셍요~

대박이다 이걸 어떻게 발견했지 고마워용!!!

ena-isme

좋아요 깔꼼하네용

ena-isme · 2024-02-05T09:46:12Z

WSSiOS/WSSiOS/Network/Novel/NovelService.swift

+    func getNovelInfo(novelId: Int) -> Single<NovelResult> {
+        do {
+            let request = try makeHTTPRequest(method: .get,
+                                               path: URLs.Novel.getNovelInfo(novelId: novelId),
+                                               headers: APIConstants.testTokenHeader,
+                                               body: nil)
+
+            NetworkLogger.log(request: request)
+
+            return urlSession.rx.data(request: request)
+                .map {NovelResult(
+                    newNovelResult: try? JSONDecoder().decode(NewNovelResult.self, from: $0),
+                    editNovelResult: try? JSONDecoder().decode(EditNovelResult.self, from: $0))
+                }
+                .asSingle()
+        } catch {
+            return Single.error(error)
+        }


hyowon612

대박이네예 완젼 짱짱 수고했삼👍
쇽쇽샥샥.. 열시미 해볼게요 ㅋ 👼🏻

hyowon612 · 2024-02-08T14:33:10Z

WSSiOS/WSSiOS/Network/Novel/NovelService.swift

+    func getNovelInfo(novelId: Int) -> Single<NovelResult> {
+        do {
+            let request = try makeHTTPRequest(method: .get,
+                                               path: URLs.Novel.getNovelInfo(novelId: novelId),
+                                               headers: APIConstants.testTokenHeader,
+                                               body: nil)
+
+            NetworkLogger.log(request: request)
+
+            return urlSession.rx.data(request: request)
+                .map {NovelResult(
+                    newNovelResult: try? JSONDecoder().decode(NewNovelResult.self, from: $0),
+                    editNovelResult: try? JSONDecoder().decode(EditNovelResult.self, from: $0))
+                }
+                .asSingle()
+        } catch {
+            return Single.error(error)
+        }


hyowon612 · 2024-02-08T14:34:45Z

WSSiOS/WSSiOS/Resource/Extensions/UIViewController+.swift

+                                                       animated: true)
+    }
+
+    func moveToNovelDetailVC(userNovelId: Int) {


W3
VC로 통일하자구 했던가용? 컨벤션에는 약어 사용하지 말라구 했던거로 기억해서!!

아고 넵 약어 사용 빼겠습니다~~

Naknakk added 19 commits January 31, 2024 00:41

[Fix] #111 - 기존 토큰 만료로 인해 새로운 토큰으로 임시 변경

7210c0d

[Refactor] #111 - MARK 주석 양식 변경에 따른 메서드 분리 및 위치 정리

2f47a5c

[Refactor] #111 - RxSwift 순환 참조 방지 with: self 추가.

6c6ac09

[Refactor] #111 - 접근 제어자 적절하게 수정

7d55c47

[Refactor] #111 - 등록 버튼 연타 불가하도록 throttle 추가

71debc9

[Refactor] #111 - View Depth 줄이기. 너무 분리된 View들을 다시 합쳐주었음.

719f4f0

[Refactor] #111 - 비즈니스 로직 CustomDatePicker View에서 VC로 이동

f772ff5

[Refactor] #111 - 중복된 코드 정리

6cc257f

[Refactor] #111 - API error do-catch로 처리

b5c3f4e

[Refactor] #111 - navigation pop 메서드 UIViewController+ 에 추가

8a8b482

[Chore] #111 - class 네이밍 변경

ba6f803

[Chore] #111 - MARK 주석 규칙 반영

6413e2c

[Refactor] #111 - View에 남아있던 collectionView 데이터를 ViewController로 이동

c2cc16b

[Refactor] #111 - 매직넘버 줄이기, 함수 네이밍 변경

8a8b2ac

[Refactor] #111 - 매직String 줄이기

7351ef9

[Refactor] #111 - Service에서 직접 작성하던 String을 메서드로 변경

5ec427a

[Fix] #111 - 내 담당이 아닌 부분의 코드 건드린 것 복구

cc7ccc5

[Refactor] #111 - 별점 최대 최소 점수 매직넘버 줄이기

d19be44

[Fix] #111 - 내 담당이 아닌 부분의 코드 건드린 것 복구

fc4a0d0

Naknakk changed the title ~~Fix/#111 register view~~ [Refactor] #111 - RegisterNormalView Refactor Feb 3, 2024

Naknakk self-assigned this Feb 3, 2024

Naknakk added 2 commits February 3, 2024 21:58

[Refactor] #111 - Input Output 분리를 위한 코드 이동

dc1f9b0

[Refactor] #111 - Input, Output, Navigation 세가지로 분리를 위한 코드 이동

258b5b2

Naknakk requested review from Guryss, ena-isme and hyowon612 February 3, 2024 13:19

[Network] #111 - Test Token 코드 추가

03611ef

Naknakk marked this pull request as ready for review February 3, 2024 13:47

Naknakk added nagi reviewPlz labels Feb 3, 2024

[Merge] #111 - RegisterNormalView Refactor Merge

78a2823

Guryss approved these changes Feb 5, 2024

View reviewed changes

Naknakk added 4 commits February 5, 2024 20:26

[Merge] branch 'main' into Fix/#111-RegisterView

f250fe1

[Chore] #111 - 변경된 Image 에셋 방식 적용

1ec9f95

[Chore] #111 - 변경된 Color 에셋 양식 적용

edea486

[Chore] #111 - setHierarchy 오타 수정

ff13a39

ena-isme approved these changes Feb 6, 2024

View reviewed changes

hyowon612 approved these changes Feb 8, 2024

View reviewed changes

Naknakk added 3 commits February 9, 2024 21:21

[Chore] #111 - VC 약어로 작성한 것 ViewController로 수정

0352d63

[Merge] 메인 브랜치 최신화 및 현재 브랜치에 Merge

0a7de29

[Design] #111 - 밀어서 뒤로가기 기능 추가

bf15a46

Naknakk merged commit f042791 into main Feb 9, 2024

Naknakk added veryGoodDeveloper and removed reviewPlz labels Feb 9, 2024

Naknakk deleted the Fix/#111-RegisterView branch February 12, 2024 15:24

[Refactor] #111 - RegisterNormalView Refactor #116

[Refactor] #111 - RegisterNormalView Refactor #116

Uh oh!

Conversation

Naknakk commented Feb 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⭐️Issue

🌟Motivation

🌟Key Changes

🌟Simulation

🌟To Reviewer

🌟To do

🌟Reference

Uh oh!

Guryss left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ena-isme left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hyowon612 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Naknakk commented Feb 3, 2024 •

edited

Loading

Guryss left a comment •

edited

Loading