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] today coordinator에 adaptor pattern 적용 (#152) #154

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

kimscastle
Copy link
Contributor

@kimscastle kimscastle commented Oct 16, 2023

[#152] REFACTOR : today coordinator에 adaptor pattern 적용

🌱 작업한 내용

  • adpator pattern을 적용하기전에 팀내에서 참고할수있게 todaycoordinator에 adaptor pattern을 적용했습니다
  • 자세한 adaptor pattern적용 이유는 추후에 작성할 예정입니다

🌱 PR Point

스크린샷 2023-10-16 오후 8 26 52

(위) 기존의 coordinator 구조 (아래) adaptor가 적용된 coordinator 구조

📮 관련 이슈

@kimscastle kimscastle added 🦁민재 민재's 🦁의성 의성's 🦁찬미 찬미's ✨Feat 새로운 기능 구현 ♻️Refactoring 리펙터링 labels Oct 16, 2023
@kimscastle kimscastle added this to the 🦁1차 Refactor🦁 milestone Oct 16, 2023
@kimscastle kimscastle merged commit 258e421 into main Oct 16, 2023
@kimscastle kimscastle deleted the refactor/#152-adaptor branch October 17, 2023 01:45
@LentoAssai
Copy link

LentoAssai commented Oct 17, 2023

엄청 빠르게 결정하고 도입까지 했네요 ㄷㄷ

  1. 어제 얘기 나눈 것과 같이 아래에서 TodayAdaptor의 생성 또한 분리하면 더 좋은 코드가 될 것 같고 (항상 생성하는 부분, 사용하는 부분은 분리하여 구현하는게 좋으니까요)
func showTodayViewController() {
    let todayAdaptor = TodayAdaptor(coordinator: self)    // 생성을 여기에서 함
    let todayVC = factory.makeTodayViewController(adaptor: todayAdaptor)
    self.navigationController.pushViewController(todayVC, animated: true)
}
  1. 나중에 리팩터 등을 대비해서 팀원들과 Adaptor, Adapter 단어를 통일하여 사용하면 좋을 듯 하네요~ 실제로는 둘 다 사용하니까 팀원들과 맞추기만 하면 될 듯 합니당

  2. 그리고 ViewController에서 변수 이름을 adaptor로 하는 것은 조금 어색하게 느껴집니다.
    어댑터라는 것은 인터페이스를 연결할 수 있도록 도와주는 친구이므로 다른 부분에서도 어댑터를 다양하게 구현하여 사용할 수 있기 때문에 나중에 다른 어댑터를 사용하게 되면 변수 명이 햇갈리게 느껴질 수 있고,
    ViewController에서 사용하는 어댑터 인스턴스의 역할은 navigating이므로 해당 프로토콜의 이름을 기반으로 변수 명을 짓거나 '액션에 따른 처리를 하는 친구' 등으로 수정되면 좋겠네요

  3. 다이어그램이 조금 이상하게 느껴지네용

그리고 마지막으로 코드를 제대로 뜯어보지는 않았지만, 메모리 릭 검사를 확실히 해서 순환참조가 발생하지 않는지는 꼭 확인하면 좋겠습니다

@ffalswo2 ffalswo2 changed the title [REFACTOR] today coordinator에 adaptor pattern 적용 [REFACTOR] today coordinator에 adaptor pattern 적용 (#152) Oct 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨Feat 새로운 기능 구현 ♻️Refactoring 리펙터링 🦁민재 민재's 🦁의성 의성's 🦁찬미 찬미's
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[REFACTOR] TodayCoordinator Adapter Pattern 적용
4 participants