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

Refactor: [refactor/Restructure] 프로젝트 전체 구조 변경 및 뷰에서 기능 분리 #86

Open
wants to merge 11 commits into
base: dev
Choose a base branch
from

Conversation

l1004ga
Copy link
Collaborator

@l1004ga l1004ga commented Jan 5, 2025

프로젝트 전체 구조 변경

  • 해당 PR은 구조가 변경되었기 때문에, 직접 브랜치 받아서 확인해보시면 좋을 것 같아요.
  • 기존에 뷰 파일만 나열되어 있어서 보기 어려운 부분이 있어서, 최대한 뷰 파일을 기능별로 정리하고, 뷰 파일 내부에 기록된 기능을 따로 분리하고자 하였습니다.

뷰 : 메인파트(온보딩, 메인탭뷰, 주간알람), 투두리스트, 폴더리스트, 투두디테일, 대시보드로 그룹화

  • TodoCompositeGridView 삭제 후 TodoGridView로 진입하도록 진입점 통일
  • 전체적으로 뷰마다 사용되는 변수 정리 및 변수명 수정

기능

  • TodoGridView와 MakeTodoView의 경우 진입점 뷰와 데이터가 많이 섞여있어서 고려할 부분이 많을 것 같아서 우선 주말까지 작업을 위해서 이 부분은 코드 분리하지 않고 두었습니다. 이 부분은 추후 작업을 더 진행해야 할 것 같아요!

기타

  • 작업을 하던 중 수정이 필요한 사항들이 보여서 이슈에 작성을 해두었습니다.(기존에 논의된 것들도 있어요!)

@l1004ga l1004ga added the enhancement New feature or request label Jan 5, 2025
@l1004ga l1004ga requested a review from Hyungeol94 January 5, 2025 22:39
@l1004ga l1004ga self-assigned this Jan 5, 2025
@l1004ga l1004ga linked an issue Jan 5, 2025 that may be closed by this pull request
4 tasks
Copy link
Collaborator

@Hyungeol94 Hyungeol94 left a comment

Choose a reason for hiding this comment

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

리팩토링 노력해주신 부분 고맙습니다, 룰루 :)
다만, 현재 코드로는 정상적인 CRUD가 이루어지지 않아서 수정이 좀 필요해 보여요.

제가 QA 진행하면서 발견한 사항을 공유해 드릴게요.

  1. 먼저, 폴더 리스트에서 폴더 추가 및 삭제가 제대로되지 않아요. 아래 동영상 확인해 주세요.
    https://github.com/user-attachments/assets/621d6a18-27cd-4346-8db3-5fbfac4435d3

그리고 기본 폴더가 삭제되어버렸어요.. 🥲

  1. 폴더 삭제 후 앱 재진입 시 이런 에러도 발생했습니다.
Pasted Graphic

현재 MV 패턴으로 코드가 구성되어 있는데, MVVM 패턴스러운 로직들을 시도하시면서 꼬인 부분들이 있는 것 같습니다. 뷰 내부가 아니라, 뷰모델을 생성해서 그 부분에서 삭제처리 등등을 하는 것이 적합한지 한번 확인해보는 것도 좋을것 같아요.

@l1004ga
Copy link
Collaborator Author

l1004ga commented Jan 12, 2025

리팩토링 노력해주신 부분 고맙습니다, 룰루 :) 다만, 현재 코드로는 정상적인 CRUD가 진행되지 않아서 수정이 좀 필요해 보여요.

제가 QA 진행하면서 발견한 사항을 공유해 드릴게요.

  1. 먼저, 폴더 리스트에서 폴더 추가 및 삭제가 제대로되지 않아요. 아래 동영상 확인해 주세요.
    https://github.com/user-attachments/assets/621d6a18-27cd-4346-8db3-5fbfac4435d3

그리고 기본 폴더가 삭제되어버렸어요.. 🥲

  1. 폴더 삭제 후 앱 재진입 시 이런 에러도 발생했습니다.
Pasted Graphic 현재 MV 패턴으로 코드가 구성되어 있는데, MVVM 패턴스러운 로직들을 시도하시면서 꼬인 부분들이 있는 것 같습니다. 뷰 내부가 아니라, 뷰모델을 생성해서 그 부분에서 삭제처리 등등을 하는 것이 적합한지 한번 확인해보는 것도 좋을것 같아요.

저도 테스트를 하면서 폴더가 정상적으로 삭제되지 않는 것을 확인하여, 이슈에 해당 내용 따로 기록해두었습니다. #83 <- 이슈 참고
제가 확인하기로는 제가 리팩토링 하기 전 Head에 있는 코드 자체에서부터 삭제 이슈가 있었던 걸로 나와요! 기존 main, dev 브랜치로 실행해도 같은 문제가 발생하기 때문에 리팩토링에서 발생한 문제는 아닌 것 같습니다! 확인부탁드려요~

Copy link
Collaborator

@Hyungeol94 Hyungeol94 left a comment

Choose a reason for hiding this comment

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

아, 그렇네요. 그 부분은 확인했습니다!

Copy link
Collaborator

@Hyungeol94 Hyungeol94 left a comment

Choose a reason for hiding this comment

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

코드를 보면서 전반적으로 아키텍처를 MV -> MVVM으로 옮겨가고자 시도하시고 계시다는 생각이 들었습니다. MV -> MVVM으로 아키텍처의 전환을 시도하시는 이유가 궁금합니다. 예를 들어 코드 분리가 목적이라면, Extension을 사용하는 방법도 있는데 그 대신 아키텍처의 전환을 선택한 다른 이유가 있으신지 설명을 해주실 수 있으실지요 :)

class NotificationManager {
static let instance = NotificationManager() // Singleton
static let shared = NotificationManager()
Copy link
Collaborator

Choose a reason for hiding this comment

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

이름 바꾼 것 좋습니다 :)

@@ -8,13 +8,19 @@ import SwiftUI
import SwiftData

struct FolderEditView: View {

// 환경변수
Copy link
Collaborator

Choose a reason for hiding this comment

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

순서 정리한 것 좋습니다!

@Environment(\.modelContext) private var modelContext
@AppStorage("sortOption") private var sortOption: SortOption = .byDateIncreasing
Copy link
Collaborator

Choose a reason for hiding this comment

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

순서 정리한 부분 좋아요 :)

@l1004ga
Copy link
Collaborator Author

l1004ga commented Jan 12, 2025

코드를 보면서 전반적으로 아키텍처를 MV -> MVVM으로 옮겨가고자 시도하시고 계시다는 생각이 들었습니다. MV -> MVVM으로 아키텍처의 전환을 시도하시는 이유가 궁금합니다. 예를 들어 코드 분리가 목적이라면, Extension을 사용하는 방법도 있는데 그 대신 아키텍처의 전환을 선택한 다른 이유가 있으신지 설명을 해주실 수 있으실지요 :)

  1. MVVM 전환 시도 이유
  • 사실 MVVM이 아니여도 상관 없었고, 조금 더 기능 중심으로 쪼개서 아키텍처를 가져가볼까도 고민했는데, 저희 앱의 규모가 크지 않은 점과, 앱 내부적으로 다른 API, framework 등을 크게 사용하지 않기 때문에 기능 분리 중심의 MVVM 정도로 뷰에서 기능을 따로 빼는 작업으로도 충분할 것 같다는 생각을 했습니다. 추후 레이어 등의 분리가 필요하다고 판단되어도 MVVM으로 기능 분리를 해놓으면 충분히 도움이 될 것이라고 생각했어요.
  1. 왜 Extension을 사용하지 않았는가?
  • 저도 이 부분이 가장 고민이 많이 되었던 부분입니다. View를 extension을 하면 각 뷰에 종속적인 부분이 큰데, 우선 기능을 따로 분리하자의 목적이 컸던 점과, 예시로 AlarmSetting의 경우 홈화면에서 보여지는 알람과, MakeTodoView 내부에서 사용되는 알람 2개로 나뉘어 지는데 알람을 세팅하는 UserNotification을 사용하는 기능적인 부분은 같기 때문에 extension으로 사용하기보다 기능을 묶어서 따로 뺀 후, 각 기능이 사용되는 뷰에서 사용하는게 더 관리하기에 편리하지 않을까? 생각했습니다.

혹시 생각하신 구조와 다르거나 다른 의견이 있으시다면~ 전달주세용!! 저도 확신을 가지고 했다기 보다, 우선 이런 방향성으로 가보면 어떨까 해서 말로하기보다 코드로 전달드리는게 논의하기 편리할 것 같아서 한번 전체 구조만 대략적으로 잡아봤어요!!

@Hyungeol94
Copy link
Collaborator

Hyungeol94 commented Jan 12, 2025

코드를 보면서 전반적으로 아키텍처를 MV -> MVVM으로 옮겨가고자 시도하시고 계시다는 생각이 들었습니다. MV -> MVVM으로 아키텍처의 전환을 시도하시는 이유가 궁금합니다. 예를 들어 코드 분리가 목적이라면, Extension을 사용하는 방법도 있는데 그 대신 아키텍처의 전환을 선택한 다른 이유가 있으신지 설명을 해주실 수 있으실지요 :)

  1. MVVM 전환 시도 이유
  • 사실 MVVM이 아니여도 상관 없었고, 조금 더 기능 중심으로 쪼개서 아키텍처를 가져가볼까도 고민했는데, 저희 앱의 규모가 크지 않은 점과, 앱 내부적으로 다른 API, framework 등을 크게 사용하지 않기 때문에 기능 분리 중심의 MVVM 정도로 뷰에서 기능을 따로 빼는 작업으로도 충분할 것 같다는 생각을 했습니다. 추후 레이어 등의 분리가 필요하다고 판단되어도 MVVM으로 기능 분리를 해놓으면 충분히 도움이 될 것이라고 생각했어요.
  1. 왜 Extension을 사용하지 않았는가?
  • 저도 이 부분이 가장 고민이 많이 되었던 부분입니다. View를 extension을 하면 각 뷰에 종속적인 부분이 큰데, 우선 기능을 따로 분리하자의 목적이 컸던 점과, 예시로 AlarmSetting의 경우 홈화면에서 보여지는 알람과, MakeTodoView 내부에서 사용되는 알람 2개로 나뉘어 지는데 알람을 세팅하는 UserNotification을 사용하는 기능적인 부분은 같기 때문에 extension으로 사용하기보다 기능을 묶어서 따로 뺀 후, 각 기능이 사용되는 뷰에서 사용하는게 더 관리하기에 편리하지 않을까? 생각했습니다.

혹시 생각하신 구조와 다르거나 다른 의견이 있으시다면~ 전달주세용!! 저도 확신을 가지고 했다기 보다, 우선 이런 방향성으로 가보면 어떨까 해서 말로하기보다 코드로 전달드리는게 논의하기 편리할 것 같아서 한번 전체 구조만 대략적으로 잡아봤어요!!

답변 감사합니다 ~

  1. 로직 분리가 목적이었군요. 그렇다면 Extension을 사용하는 것도 충분히 좋은 대안이라고 생각합니다. 말씀해주신 바대로 우리 앱이 API를 사용하지 않기 때문에 로직이 복잡하지 않은데, MVVM으로 전환하면 이런 장점을 잃고 조금 더 복잡해지는 것처럼 보여요. 반면 Extensions으로 코드 분리를 한다면 더 복잡해지지 않고 심플함을 유지할 수 있을 것 같아요.

  2. 말씀해주신 알림 설정과 관련해서도 코드를 살펴보았습니다. NotificationManager에 알림 설정 로직은 이미 구현되어 있어서 공통적으로 사용 가능해보입니다. 또한 현재 뷰모델에서 구현해두신 로직 또한 AlarmSettingView의 UI에 의존적이라 VM + Extension으로 한다면 조금 더 코드가 간결해질 것 같습니다.

  3. 지금까지 구현된 바로는 모든 로직이 뷰(UI)에 의존적이라서 .. 뷰에 종속되지 않은 로직이 필요한가 싶습니다. 백그라운드 업데이트나 로딩 등의 작업이 필요할 때에는 뷰에 종속되지 않은 로직이 꼭 필요한데, 현재는 그런 부분이 딱히 없어서요.

결론 : 기존 아키텍처는 유지하되, Extension으로 코드 분리 및 관리는 어떠신지 제안드리고 싶습니다~

@l1004ga
Copy link
Collaborator Author

l1004ga commented Jan 13, 2025

코드를 보면서 전반적으로 아키텍처를 MV -> MVVM으로 옮겨가고자 시도하시고 계시다는 생각이 들었습니다. MV -> MVVM으로 아키텍처의 전환을 시도하시는 이유가 궁금합니다. 예를 들어 코드 분리가 목적이라면, Extension을 사용하는 방법도 있는데 그 대신 아키텍처의 전환을 선택한 다른 이유가 있으신지 설명을 해주실 수 있으실지요 :)

  1. MVVM 전환 시도 이유
  • 사실 MVVM이 아니여도 상관 없었고, 조금 더 기능 중심으로 쪼개서 아키텍처를 가져가볼까도 고민했는데, 저희 앱의 규모가 크지 않은 점과, 앱 내부적으로 다른 API, framework 등을 크게 사용하지 않기 때문에 기능 분리 중심의 MVVM 정도로 뷰에서 기능을 따로 빼는 작업으로도 충분할 것 같다는 생각을 했습니다. 추후 레이어 등의 분리가 필요하다고 판단되어도 MVVM으로 기능 분리를 해놓으면 충분히 도움이 될 것이라고 생각했어요.
  1. 왜 Extension을 사용하지 않았는가?
  • 저도 이 부분이 가장 고민이 많이 되었던 부분입니다. View를 extension을 하면 각 뷰에 종속적인 부분이 큰데, 우선 기능을 따로 분리하자의 목적이 컸던 점과, 예시로 AlarmSetting의 경우 홈화면에서 보여지는 알람과, MakeTodoView 내부에서 사용되는 알람 2개로 나뉘어 지는데 알람을 세팅하는 UserNotification을 사용하는 기능적인 부분은 같기 때문에 extension으로 사용하기보다 기능을 묶어서 따로 뺀 후, 각 기능이 사용되는 뷰에서 사용하는게 더 관리하기에 편리하지 않을까? 생각했습니다.

혹시 생각하신 구조와 다르거나 다른 의견이 있으시다면~ 전달주세용!! 저도 확신을 가지고 했다기 보다, 우선 이런 방향성으로 가보면 어떨까 해서 말로하기보다 코드로 전달드리는게 논의하기 편리할 것 같아서 한번 전체 구조만 대략적으로 잡아봤어요!!

답변 감사합니다 ~

  1. 로직 분리가 목적이었군요. 그렇다면 Extension을 사용하는 것도 충분히 좋은 대안이라고 생각합니다. 말씀해주신 바대로 우리 앱이 API를 사용하지 않기 때문에 로직이 복잡하지 않은데, MVVM으로 전환하면 이런 장점을 잃고 조금 더 복잡해지는 것처럼 보여요. 반면 Extensions으로 코드 분리를 한다면 더 복잡해지지 않고 심플함을 유지할 수 있을 것 같아요.
  2. 말씀해주신 알림 설정과 관련해서도 코드를 살펴보았습니다. NotificationManager에 알림 설정 로직은 이미 구현되어 있어서 공통적으로 사용 가능해보입니다. 또한 현재 뷰모델에서 구현해두신 로직 또한 AlarmSettingView의 UI에 의존적이라 VM + Extension으로 한다면 조금 더 코드가 간결해질 것 같습니다.
  3. 지금까지 구현된 바로는 모든 로직이 뷰(UI)에 의존적이라서 .. 뷰에 종속되지 않은 로직이 필요한가 싶습니다. 백그라운드 업데이트나 로딩 등의 작업이 필요할 때에는 뷰에 종속되지 않은 로직이 꼭 필요한데, 현재는 그런 부분이 딱히 없어서요.

결론 : 기존 아키텍처는 유지하되, Extension으로 코드 분리 및 관리는 어떠신지 제안드리고 싶습니다~

네~ 그러면 우선은 extension으로 기능을 분리한 후 추가적인 논의 해보면 좋을 것 같아요! 해당 브랜치 close 하고 extension 방식으로 다시 리팩토링 해볼게요~
해당 리팩토링은 배포와는 관계가 없기 떄문에 배포 작업이 먼저 마무리 된다면 그 이후에 추가로 작업해보도록 하겠습니다!!

@Hyungeol94
Copy link
Collaborator

네~ 그러면 우선은 extension으로 기능을 분리한 후 추가적인 논의 해보면 좋을 것 같아요! 해당 브랜치 close 하고 extension 방식으로 다시 리팩토링 해볼게요~ 해당 리팩토링은 배포와는 관계가 없기 떄문에 배포 작업이 먼저 마무리 된다면 그 이후에 추가로 작업해보도록 하겠습니다!!

넹 :) 브랜치를 닫고 작업을 하면 반복작업을 하시게 될 것 같아서 살짝 우려가 되는데, 괜찮으시다면 지금 해두신 작업에서 수정하시는 건 어떠실까요? ViewModel -> Extension으로 전환하고, 관련 코드만 수정하는 식으로 한다면 조금 더 수월하시지 않을까 생각이 드네요. 그렇지만 새로 파서 작업하셔도 저는 찬성입니다~ 그리고 괜찮다면 GirdView 관련해서 Extension으로 코드 분리하는 작업은 제가 이어서 해도 괜찮을까요 ?

@l1004ga
Copy link
Collaborator Author

l1004ga commented Jan 13, 2025

네~ 그러면 우선은 extension으로 기능을 분리한 후 추가적인 논의 해보면 좋을 것 같아요! 해당 브랜치 close 하고 extension 방식으로 다시 리팩토링 해볼게요~ 해당 리팩토링은 배포와는 관계가 없기 떄문에 배포 작업이 먼저 마무리 된다면 그 이후에 추가로 작업해보도록 하겠습니다!!

넹 :) 브랜치를 닫고 작업을 하면 반복작업을 하시게 될 것 같아서 살짝 우려가 되는데, 괜찮으시다면 지금 해두신 작업에서 수정하시는 건 어떠실까요? ViewModel -> Extension으로 전환하고, 관련 코드만 수정하는 식으로 한다면 조금 더 수월하시지 않을까 생각이 드네요. 그렇지만 새로 파서 작업하셔도 저는 찬성입니다~ 그리고 괜찮다면 GirdView 관련해서 Extension으로 코드 분리하는 작업은 제가 이어서 해도 괜찮을까요 ?

혹시 PR이미 올린거에 이어서 작업하면 코드 확인하실 떄 번거로울까봐 따로 작업할까 했던건데, 여기서 계속 작업해도 괜찮으시다면 저도 이미 분리해 놓은 부분만 옮기는게 편하기는 합니다!
GridView 관련 Extension 작업은 그러면 해당 코드 제가 따로 VM 코드 분리해 놓은 부분을 다시 extension으로 바꾼 이후에 작업을 하시는걸까요?

@Hyungeol94
Copy link
Collaborator

혹시 PR이미 올린거에 이어서 작업하면 코드 확인하실 떄 번거로울까봐 따로 작업할까 했던건데, 여기서 계속 작업해도 괜찮으시다면 저도 이미 분리해 놓은 부분만 옮기는게 편하기는 합니다! GridView 관련 Extension 작업은 그러면 해당 코드 제가 따로 VM 코드 분리해 놓은 부분을 다시 extension으로 바꾼 이후에 작업을 하시는걸까요?

네넵 지금까지 작업에서 아키텍처 관련 수정할 부분만 작업해서 머지해 주시고, 그 다음에 새로 브랜치를 파서 각자 작업을 이어가면 좋을것 같습니다. extension으로 바꾸고 나서 머지가 되면 저도 브랜치를 파서 GridView에 있는 코드를 분리하는 작업을 해보겠습니다.

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

Successfully merging this pull request may close these issues.

전체 폴더 및 파일 구조 변경
2 participants