-
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] #244 - 마이페이지 MVVM + Combine #255
Open
jeongdung-eo
wants to merge
25
commits into
develop
Choose a base branch
from
feat/#244
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🫧 작업한 내용
[공통 컴포넌트 NOTTODO navigation view 구현]
추천 상세뷰와 마이페이지 상세뷰에서 네비게이션이 사용이되는데
이를 공통 컴포넌트로 구현함.
[데이터 유연성과 유지 보수성 개선]
기존의 myPageAcccountViewController는 custom한 stack뷰 4개로 구현이 되어있었음.
현재 구조는 뷰의 개수가 고정되어 있어 데이터가 늘어날 때마다 새로운 스택 뷰를 추가하고 관리해야 하기 때문에 유연성이 떨어짐.
이러한 문제를 해결하기 위해서 컬렉션뷰로 리팩토링 진행함.
데이터의 양에 상관없이 유연하게 처리할 수 있기때문에 코드의 재사용성과 유지 보수가 용이해짐.
[deinit 호출되지 않는 문제 해결]
myAccountpage → mypage로 pop이 되었을 때 myAccountpage vc의 deinit이 호출이 되어야하는데 호출되지 않음.
[문제 원인]
nested closure의 하위 closure에 weak를 선언하여 최상위 closure는 여전히 strong 참조를 하여 retain cycle이 발생함.
CellRegistration는 UICollectionView.CellRegistration를 typealias한 것으로 내부를 보면 아래와 같이 구현이 되어있고, Cell을 구성하는 부분에 클로저를 사용하는 것을 알 수 있음.
한마디로,,, cellRegistration는 nested closure를 사용하고 있음 ,,,
nested Closure로 구현이 되어있는 경우 하위 closure에만 weak를 선언하게 되면 최상위 closure는 여전히 strong 참조를 하게 되어 retain cycle이 발생하게 됨.
이를 해결하기 위해서는 최상위 closure에 weak를 선언하여 strong 참조를 끊어야함!
[해결]
최상위 strong 참조를 weak 참조로 접근하게하니 메모리에서 해제됨.
🔴 Instance of MyPageAccountViewController is deallocated
deinit되는걸 확인할 수 있음
🙋♀️ nested closure인 경우 외부, 내부 closure 모두 weak를 선언해야하는가 ?
❌ 최상위 closure에서 이미 강한 참조를 끊었기 때문에 하위 closure에서 순환참조가 발생하지 않음.
🔫 PR Point
📸 스크린샷
📮 관련 이슈