[Refactor] #625 - FeedViewModel로 FeedVC 리팩토링 #630

yangsubinn · 2022-05-21T16:05:52Z

🔥Pull requests

⛳️ 작업한 브랜치

refactor/[Refactor] FeedVC 리펙토링 #625

👷 작업한 내용

데이터를 담는 리스트와 리스트를 수정하는 부분은 최대한 다 FeedViewModel 로 옮겨서 그 안에서만 수정하도록 했습니다...

🚨참고 사항

생각보다 많이 엉망이었던 것 같아요..
일단 목표는 리스트를 수정하는 부분은 전부 viewModel에서만 처리하도록 하는 것이었고 아마.. 전부 수정한 것 같습니다..
근데 이게 맞는지도 확실히 모르겠고 , 보다보니 너어무 리스트가 많은 것 같아서 이것도 바꾸긴 해야할 것 같은데 ..
일단 더 좋은 방법이 있다면 .. 듣고자 먼저 풀리퀘 올려봅니다 .. 질문.. 태클.. 조언 .. 전부 다 받습니다 ..!

📟 관련 이슈

Resolved: [Refactor] FeedVC 리펙토링 #625

…e 함수 구성

…odel 연결

yangsubinn · 2022-05-23T15:00:33Z

이슈발생.. 했응께 좀만 다시 수정하고 다시 태그해드리겠습니다.. 그때 봐주세요..😇 @hyun99999 @L-j-h-c

yangsubinn · 2022-05-24T08:47:18Z

수정했습니다 확인부탁드립니다 🤓 @hyun99999 @L-j-h-c
저거보다 더 좋은 방법이 있을 것 같긴한데 일단.. 생각이 안나

L-j-h-c

워후 확실히 전보다 훨씬 보기 좋아졌네요!! 수고 많으셨습니다~~! 한 가지 궁금한 점은 뷰모델에 타입 프로퍼티를 두고 접근하신 부분이에요!! 보통은 뷰컨트롤러 안에 뷰모델을 두고 쓰는데, 그렇게 하면 뷰컨트롤러가 메모리에서 해제될 때 뷰모델도 해제될 수 있기 때문에... 그렇게 쓰지 않나 싶은데 이렇게 shared로 접근하는 건 처음봐서요!!

L-j-h-c · 2022-05-26T10:50:39Z

Spark-iOS/Spark-iOS/Source/ViewModel/FeedViewModel.swift

+    public lazy var firstList: [Record] = []
+    public lazy var secondList: [Record] = []
+    public lazy var thirdList: [Record] = []
+    public lazy var fourthList: [Record] = []
+    public lazy var fifthList: [Record] = []
+    public lazy var sixthList: [Record] = []
+    public lazy var seventhList: [Record] = []
+    public lazy var eighthList: [Record] = []


배열 안에 배열을 넣는 방법은 어떨까요? 그러면 removeAllData나 setDataList 같은 메서드들도 길이가 많이 줄어들 것 같은데.. 가독성을 위해서 뭐가 더 좋은지는 잘 모르겟서여...

왐맘마 해보겠슴니다

L-j-h-c · 2022-05-26T10:50:59Z

Spark-iOS/Spark-iOS/Source/ViewModel/FeedViewModel.swift

+
+        switch indexPath.section {
+        case 0:
+            print("✈️ firstList - \(firstList[indexPath.item])")


난 여행을 떠나.

L-j-h-c · 2022-05-26T10:52:53Z

Spark-iOS/Spark-iOS/Source/ViewModel/FeedViewModel.swift

+        switch indexPath.section {
+        case 0:
+            targetList = firstList
+        case 1:
+            targetList = secondList
+        case 2:
+            targetList = thirdList
+        case 3:
+            targetList = fourthList
+        case 4:
+            targetList = fifthList
+        case 5:
+            targetList = sixthList
+        case 6:
+            targetList = seventhList
+        default:
+            targetList = eighthList
+        }


배열 안에 배열을 넣어주면 targetList = list[indexPath.section]처럼 할 수 있을 것 같아서여...

yangsubinn · 2022-06-03T04:12:47Z

워후 확실히 전보다 훨씬 보기 좋아졌네요!! 수고 많으셨습니다~~! 한 가지 궁금한 점은 뷰모델에 타입 프로퍼티를 두고 접근하신 부분이에요!! 보통은 뷰컨트롤러 안에 뷰모델을 두고 쓰는데, 그렇게 하면 뷰컨트롤러가 메모리에서 해제될 때 뷰모델도 해제될 수 있기 때문에... 그렇게 쓰지 않나 싶은데 이렇게 shared로 접근하는 건 처음봐서요!!

일단 shared로 접근한건 제가 저 친구를 싱글톤 패턴의 Manager 파일을 참조해서 만들다가 그런 것 같아여..!
같은 데이터를 필요로 하는 여러 뷰컨에서 하나의 manager에 모든 데이터를 보관하고 그 데이터를 여러 곳에서 수정하기 때문에
여기서도 FeedViewModel 안에 타입 프로퍼티로 FeedViewModel 인스턴스를 만들어서
여러 뷰컨에서 접근하더라도 하나의 인스턴스를 사용하겠다는 의미였슴니다

여기서 갖고 있는 데이터가 다른 뷰컨에서 쓰일 일이 지금은 없긴 하지만,, 만일 나중에라도 다른 곳에서 수정될 일이 생긴다면 메모리 관리하는 면에서도 최초 한번 하나의 인스턴스로 만들어두고 같은 데이터를 수정한다는 점에서 두는 것도 나뿌지 않을랑가 싶어요 ..

yangsubinn · 2022-06-07T17:18:07Z

@L-j-h-c @hyun99999
록시선배 코드리뷰 반영해서 배열 안의 배열 형태로 수정했슴니다 ! 코드가 훠어어어얼씬 깔끔해졌어요 !
그리고 방금 배열 안의 배열로 수정하면서 조그만하게 느낀 부분이 ,,
FeedViewModel로 배열 관련 로직을 전부 분리해뒀더니 정말 딱 요 파일만 수정하게 돼서 먼가 잘 분리되었다는 .. 느낌이 들었습니다.. 아님 말고 ..
로직은 FeedViewModel에서 처리하고, 처리한 결과의 최종 배열만 VC로 전달하고 .. 그러려고 했습니다

깔꼼하고 뿌듯한 기분은 제가 오랜만에 개발을 해서일까여 .. 🥹

L-j-h-c · 2022-06-12T08:46:27Z

@L-j-h-c @hyun99999 록시선배 코드리뷰 반영해서 배열 안의 배열 형태로 수정했슴니다 ! 코드가 훠어어어얼씬 깔끔해졌어요 ! 그리고 방금 배열 안의 배열로 수정하면서 조그만하게 느낀 부분이 ,, FeedViewModel로 배열 관련 로직을 전부 분리해뒀더니 정말 딱 요 파일만 수정하게 돼서 먼가 잘 분리되었다는 .. 느낌이 들었습니다.. 아님 말고 .. 로직은 FeedViewModel에서 처리하고, 처리한 결과의 최종 배열만 VC로 전달하고 .. 그러려고 했습니다

깔꼼하고 뿌듯한 기분은 제가 오랜만에 개발을 해서일까여 .. 🥹

오오오 코드가 확실히 줄었네요...이걸 이렇게 뚝딱 해버리시다니 멋져요!! 앱을 점점 Develop 시키면서 규모도 커지고 하면 유지보수를 해야할 일이 많아질 것 같은데 큰맘먹고 ViewModel로 리팩하신 부분...멋있습니다..난 언제 보관함 리팩하냐고 아 ㅋㅋ
아니 글고 왜 개발 오랜만이냐고 ㅋㅋ

hyun99999 · 2022-06-19T09:32:48Z

view model 에 대해서 싱글톤 패턴 사용
수빈선배가 말한대로 싱글톤 패턴을 사용하면 혹여나 다른 뷰에서 사용할 때 메모리를 아낄 수 있을거 같아요!
또한, view model 과 view 는 1:N 의 가능하기 때문에 뷰 모델을 수빈선배가 말한대로 공유할 여지는 있다고 생각해요! 이때의 단점은 뷰모델이 커질 수도 있다가 있을거 같아요.
반면에 싱글톤 패턴이기 때문에 준호선배 말한대로 뷰 컨트롤러가 메모리에서 해제될 때 뷰 모델이 해제되지 않기 때문에 불필요한 메모리를 잡고 있을 수도 있다고 생각해용. 또한, 싱글톤 패턴의 단점은 멀티스레드에서 같은 인스턴스에 대해서 동시에 접근하여 동기화 문제와 자원에 대한 병목 현상도 있다고 존재해용.
그래서 굳이 뷰에 대해서 뷰 모델이 유효한 것이지 다른 경우에도 필요한 것이 아닌데 싱글톤을 사용하는 것은 장점을 잘 살리는 방법은 아니기 때문에 단점이 먼저 보일 수도 있는 것 같아요!
그냥 단순하게 다른 뷰에서도 사용하게 된다면? 그냥 뷰 모델을 공유하는 것이지요

요 글도 참고해 보면 좋을거 같아요!

https://velog.io/@cherish307/Why-should-ViewModel-not-be-Singleton-Object

hyun99999

view model 로 분리해서 확실히 비지니스 로직을 확인하기 너무 좋고 유지보수도 좋을 것 같습니다!
멋진 코드 보구 가네용
클린코드 책... 조심스럽게 추천드리고 아니면 swift 에 적용해서 요약해서 블로그에 정리해둔 글도 많으니 보셔도 좋을 듯합니다! 이게 좀 더 좋지 않을까? 라는 애매했던 생각에 근거를 가질 수 있었어요!

hyun99999 · 2022-06-19T09:59:12Z

Spark-iOS/Spark-iOS/Source/ViewModel/FeedViewModel.swift

+    func removeAllData() {
+        dateList.removeAll()
+        dayList.removeAll()
+        for i in 0...7 {
+            contentList[i].removeAll()
+        }
+    }


7이라는 값도 상수 이기 때문에 유지보수에 힘들다구 생각해요!
contentList 의 갯수를 활용하면 좋을 것 같습니당!

오우 앗차차 생각 못했던 부분을 정확하게 찝어주시네욜 ..

hyun99999 · 2022-06-19T10:32:55Z

Spark-iOS/Spark-iOS/Source/ViewModel/FeedViewModel.swift

+    func setFeedsList(isScroll: Bool) {
+        if isScroll {
+            self.feeds.append(contentsOf: self.newFeeds)
+        } else {
+            self.feeds = self.newFeeds
+        }
+
+        if feeds.count >= feedCountSize {
+            isFirstScroll = false
+        }
+    }


요즘 클린코드 책을 읽구 있는데 이렇게 bool 값을 넘기는 flag 역할을 하는 인수는 좋지 못하다구 하더라구요! 함수는 무조건 하나의 역할만을 해야하는데 결국 참일경우에는 a경우를 거짓일 경우는 b일 경우를 하는 거니까 그렇지 못하다는 것이죵!
그래서 isScroll 파라미터가 참과 거짓인 경우를 나누는 함수가 더 좋다는 것으로 적용이 가능할 것 같아요!
bool 자료형을 가진 파라미터에 대해서는 위의 내용을 고려하셔 보는 것도 좋을 것 같습니당
위의 언급한대로 작성하게 되면 장점은 이 함수가 true 일때 무엇을 하는지 false 일때 무엇을 하는지 함수명에서 알 수 있다는 것이 될거 같습니당

오 .. 그럼 bool 자료형의 파라미터를 받기보다는
애초에 true인 경우의 함수와 false인 경우의 함수를 분리해서, 함수 내에서는 조건에 따라 나누지 않고 딱 한가지 경우일때의 코드를 실행하게 되는 ,,
이런거 말씀하신게 맞을까여?

넹 정확해요! 뷰에서 isScroll 이 참,거짓인 경우에 활용하게 되면 참,거짓인 경우에 어떤 일을 하는지 알 수 있다는 것을 말하고 싶었어요
그렇게 되면 어떻게 함수명을 지어야하는 지에 머리가 아파오는데 저라면 더해주는 함수와 대입해주는 함수명으로 설정해줄 것 같네용

hyun99999 · 2022-06-19T10:34:52Z

Spark-iOS/Spark-iOS/Source/ViewModel/FeedViewModel.swift

+    func getDataList(indexPath: IndexPath) -> Record {
+        var dataList: Record
+        dataList = contentList[indexPath.section][indexPath.item]
+        return dataList
+    }


return contentList[indexPath.section][indexPath.item] 도 좋은 코드가 될 것 같은데 혹시 어떤 점을 우려해서 작성하셨을까요?

아고 .. 지금 보니까 불필요해보이는데,, 리팩하면서 코드를 옮기는 과정에서 놓쳤나 .. 싶습니다..
바로 리턴하도록 수정하겠습니다!

Merge branch 'develop' of https://github.com/TeamSparker/Spark-iOS into refactor/TeamSparker#625

yangsubinn · 2022-06-20T06:24:02Z

@L-j-h-c @hyun99999 코드리뷰 반영해서 전부 수정했슴니다 머지할게여 !

yangsubinn added 7 commits May 21, 2022 00:52

[Add] TeamSparker#625 - FeedViewModel 추가, setDataList, changeLikeStat…

d5fdbda

…e 함수 구성

[Refactor] TeamSparker#625 - cellForItemAt, likeButtonTapped 함수 viewM…

f680c6c

…odel 연결

[Chore] TeamSparker#625 - 주석 수정

ac465e2

[refactor] TeamSparker#625 - day, date 리스트 세팅 함수 viewModel 연결

75f2f98

[refactor] TeamSparker#625 - setDataList 함수 viewModel 연결 및 네이밍 수정

8652345

[refactor] TeamSparker#625 - 서버 통신 후 데이터 세팅 함수 viewModel 연결

eaeaa28

[Fix] TeamSparker#625 - FeedViewModel 프레임워크 변경 및 주석 삭제

4cfe908

yangsubinn added 👷Refactor👷‍♂️ 동작변화없이 내부구조 변경 🦹t없e맑은水빈 우리 선배 이젠 타이니하지 않아 🖐help_wanted🖐 도움이 필요해요~! labels May 21, 2022

yangsubinn requested review from hyun99999 and L-j-h-c May 21, 2022 16:05

yangsubinn self-assigned this May 21, 2022

[Fix] TeamSparker#625 - targetList의 값만 바뀌고 로컬 리스트에는 적용되지 않던 문제 해결

d8750e8

L-j-h-c approved these changes May 26, 2022

View reviewed changes

[Fix] TeamSparker#625 - FeedViewModel 배열 안의 배열로 수정

64326f5

hyun99999 approved these changes Jun 19, 2022

View reviewed changes

yangsubinn added 2 commits June 20, 2022 15:20

[Chore] TeamSparker#630 - 코드리뷰 반영 수정

476f72b

[Merge] develop pull

1a0cb63

Merge branch 'develop' of https://github.com/TeamSparker/Spark-iOS into refactor/TeamSparker#625

yangsubinn merged commit f03001d into TeamSparker:develop Jun 20, 2022

hyun99999 mentioned this pull request Aug 7, 2022

[Chore] #633 - 버전 및 빌드 업데이트(1.0.2) #634

Merged

yangsubinn deleted the refactor/#625 branch October 11, 2022 02:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Refactor] #625 - FeedViewModel로 FeedVC 리팩토링 #630

[Refactor] #625 - FeedViewModel로 FeedVC 리팩토링 #630

yangsubinn commented May 21, 2022

yangsubinn commented May 23, 2022

yangsubinn commented May 24, 2022

L-j-h-c left a comment

L-j-h-c May 26, 2022

yangsubinn Jun 3, 2022

L-j-h-c May 26, 2022

L-j-h-c May 26, 2022

yangsubinn commented Jun 3, 2022

yangsubinn commented Jun 7, 2022

L-j-h-c commented Jun 12, 2022

hyun99999 commented Jun 19, 2022

hyun99999 left a comment

hyun99999 Jun 19, 2022

yangsubinn Jun 19, 2022

hyun99999 Jun 19, 2022

yangsubinn Jun 19, 2022

hyun99999 Jun 20, 2022

hyun99999 Jun 19, 2022

yangsubinn Jun 19, 2022

yangsubinn commented Jun 20, 2022

[Refactor] #625 - FeedViewModel로 FeedVC 리팩토링 #630

[Refactor] #625 - FeedViewModel로 FeedVC 리팩토링 #630

Conversation

yangsubinn commented May 21, 2022

🔥Pull requests

🚨참고 사항

📟 관련 이슈

yangsubinn commented May 23, 2022

yangsubinn commented May 24, 2022

L-j-h-c left a 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

yangsubinn commented Jun 3, 2022

yangsubinn commented Jun 7, 2022

L-j-h-c commented Jun 12, 2022

hyun99999 commented Jun 19, 2022

hyun99999 left a 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

yangsubinn commented Jun 20, 2022