-
Notifications
You must be signed in to change notification settings - Fork 0
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
base: dev
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
리팩토링 노력해주신 부분 고맙습니다, 룰루 :)
다만, 현재 코드로는 정상적인 CRUD가 이루어지지 않아서 수정이 좀 필요해 보여요.
제가 QA 진행하면서 발견한 사항을 공유해 드릴게요.
- 먼저, 폴더 리스트에서 폴더 추가 및 삭제가 제대로되지 않아요. 아래 동영상 확인해 주세요.
https://github.com/user-attachments/assets/621d6a18-27cd-4346-8db3-5fbfac4435d3
그리고 기본 폴더가 삭제되어버렸어요.. 🥲
- 폴더 삭제 후 앱 재진입 시 이런 에러도 발생했습니다.

현재 MV 패턴으로 코드가 구성되어 있는데, MVVM 패턴스러운 로직들을 시도하시면서 꼬인 부분들이 있는 것 같습니다. 뷰 내부가 아니라, 뷰모델을 생성해서 그 부분에서 삭제처리 등등을 하는 것이 적합한지 한번 확인해보는 것도 좋을것 같아요.
저도 테스트를 하면서 폴더가 정상적으로 삭제되지 않는 것을 확인하여, 이슈에 해당 내용 따로 기록해두었습니다. #83 <- 이슈 참고 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
아, 그렇네요. 그 부분은 확인했습니다!
There was a problem hiding this 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() |
There was a problem hiding this comment.
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 { | |||
|
|||
// 환경변수 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
순서 정리한 부분 좋아요 :)
혹시 생각하신 구조와 다르거나 다른 의견이 있으시다면~ 전달주세용!! 저도 확신을 가지고 했다기 보다, 우선 이런 방향성으로 가보면 어떨까 해서 말로하기보다 코드로 전달드리는게 논의하기 편리할 것 같아서 한번 전체 구조만 대략적으로 잡아봤어요!! |
답변 감사합니다 ~
결론 : |
네~ 그러면 우선은 extension으로 기능을 분리한 후 추가적인 논의 해보면 좋을 것 같아요! 해당 브랜치 close 하고 extension 방식으로 다시 리팩토링 해볼게요~ |
넹 :) 브랜치를 닫고 작업을 하면 반복작업을 하시게 될 것 같아서 살짝 우려가 되는데, 괜찮으시다면 지금 해두신 작업에서 수정하시는 건 어떠실까요? ViewModel -> Extension으로 전환하고, 관련 코드만 수정하는 식으로 한다면 조금 더 수월하시지 않을까 생각이 드네요. 그렇지만 새로 파서 작업하셔도 저는 찬성입니다~ 그리고 괜찮다면 GirdView 관련해서 Extension으로 코드 분리하는 작업은 제가 이어서 해도 괜찮을까요 ? |
혹시 PR이미 올린거에 이어서 작업하면 코드 확인하실 떄 번거로울까봐 따로 작업할까 했던건데, 여기서 계속 작업해도 괜찮으시다면 저도 이미 분리해 놓은 부분만 옮기는게 편하기는 합니다! |
네넵 지금까지 작업에서 아키텍처 관련 수정할 부분만 작업해서 머지해 주시고, 그 다음에 새로 브랜치를 파서 각자 작업을 이어가면 좋을것 같습니다. extension으로 바꾸고 나서 머지가 되면 저도 브랜치를 파서 GridView에 있는 코드를 분리하는 작업을 해보겠습니다. |
프로젝트 전체 구조 변경
뷰 : 메인파트(온보딩, 메인탭뷰, 주간알람), 투두리스트, 폴더리스트, 투두디테일, 대시보드로 그룹화
기능
기타