화면 전환 방식에 관하여 논의합니다. (Router, Coordinator)

[dlwogus0128]

화면 전환 방식을 리팩토링하기 위해, 구체적인 방안 논의를 위한 Discussion을 열었습니다. 🙇🏼

논의할 주제는 크게 두 가지인데요.

논의 1. Router를 제거하자
논의 2. VC의 생명주기에 맞춰서 Coordinator가 메모리에서 해제될 수 있도록 하자

---

💡 논의 1: Router를 제거하자

문제 상황

• 현재 저희 프로젝트는 Router가 직접적인 화면 전환을, Coordinator는 전체적인 플로우를 관리하는 역할을 담당하고 있습니다.
• 그러나 일부 코드를 살펴보면, Router에게 화면 전환을 요청하는 것이 아닌 Coordinator 안에서 직접 화면 전환을 진행하고 있는 부분들이 있습니다.
• 아래 두 사진은 해당 부분에 대한 예시 코드인데요. (위: present, 아래: push)
• present의 경우, 플로우가 시작되며 Router의 rootController는 어떠한 VC를 present하기 때문에, Router의 rootController는 더 이상 present할 공간이 없습니다. 따라서 새로운 VC를 present하기 위해선, Router의 rootController가 present한 VC 위에서 present해야 합니다.
  
• push의 경우, 플로우가 시작되며 Router의 rootController가 어떠한 VC를 present 했기 때문에, NavigationController가 가려져 Router를 활용할 경우 뒷쪽에서 push되는 상황이 발생합니다. 따라서 push를 위해선, Router의 rootController가 present한 VC를 UINavigationController로 래핑해 활용해야 합니다.
  
• 이를 도식화해보면 아래와 같은 모습인데요.

• Router가 최상단에 위치해, 모든 화면 전환을 담당하고 있기 때문에 생긴 문제입니다.

Router의 실효성

• Router의 초기 구현 목적은 화면 전환 처리에 대한 완전한 책임 분리였습니다. Coordinator는 화면 전환 흐름을 관리할 뿐, 직접적인 라우팅에 대한 책임을 지어선 안된다는 목적에 의해 분리되었는데요. 참고 자료

• 그러나 이를 위해 내장 함수를 단순 래핑하는 것이 실질적으로 필요한가? 에 대한 의문점이 들었습니다. (아래 사진 참고)
  

• 또한, Coordinator는 본래 네비게이션 흐름을 담당하는 객체인데, 화면 전환 로직을 꼭 몰라야 하는지에 관해서도 생각해 볼 부분입니다.

프로젝트에 미치는 영향

• 물론 Router를 제거하지 않아도 기능을 구현하는 것에는 문제가 없지만, 화면 전환 컨텍스트에 대한 완전한 이해가 수반되지 않는다면 원활한 기능 개발에 병목이 될 것이라고 생각합니다. (저희도 처음에 화면 전환에서 애를 먹었던 것을 생각해보면...)
• 이후 다른 분께서 유지보수를 맡게 되시더라도 화면 전환에서 똑같이 문제를 겪으실 것이라 생각합니다. 저희 프로젝트가 6개월마다 인원이 바뀔 수 있는 기수제임을 고려했을 때, 매 기수마다 같은 문제가 반복된다면 유지보수가 힘들 것 같습니다.
• 그래서... 기회가 된다면 리팩토링을 함께 진행해보시는 게 어떠실지 살짝 제안해봅니다. 👀

예시 코드

public final class TestSplashCoordinator: TestDefaultCoordinator {
    private let factory: SplashFeatureViewBuildable
    private let cancelBag = CancelBag()
    
    private let navigationController: UINavigationController?  // TODO
    
    public init(
        navigationController: UINavigationController,  // TODO
        factory: SplashFeatureViewBuildable
    ) {
        self.navigationController = navigationController  // TODO
        self.factory = factory
    }

    private func showSplash() {
        var splash = factory.makeSplash()

        splash.vm.onNoticeSkipped = { [weak self] in
            self?.navigationController?.popViewController(animated: true)  // TODO
        }
        
        navigationController?.pushViewController(splash.vc.viewController, animated: true)  // TODO
    }

• 지금 예시 코드를 작성해보고 있는데, Router 대신 UINavigationController를 주입받아 더 이상 Router가 아닌 네비컨에게 직접 화면 전환을 요청하는 방식으로 전면 수정하면 될 것 같습니다.

---

💡 논의 2: VC의 생명주기에 맞춰서 Coordinator가 메모리에서 해제될 수 있도록 하자

문제 상황

• 현재 저희 프로젝트는 어떠한 Coordinator의 수명이 다하면, 이를 메모리에서 해제하기 위해 finishFlow()를 수동으로 불러 이를 받은 부모 Coordinator가 의존성을 삭제해주는 방식입니다.

• 이 방식은 실수로 finishFlow()를 부르지 않을 경우, 의존성이 제거되지 못해 메모리에서 해제되지 못하고 Coordinator가 잔존한다는 문제가 있습니다.
• (실제로 이전 PR에서 finishFlow()가 누락된 부분에 메모리 누수가 존재하고 있어 제거했었습니다.)

해결 방안

• 한 Coordinator 안에서 가장 아래에 존재하는 VC의 생명주기에 맞추어 해제될 수 있는 방법을 시도할 수 있을 것 같습니다. (변수나 프로토콜 명칭은 더 적절하게 수정해보겠습니다.)
• SplashViewControllable에서 VC가 해제될 때 호출될 onDeinit 클로저를 만듭니다.

public protocol SplashViewControllable: ViewControllable, TestCoordinatorFinishOutput {
    var onDeinit: (() -> Void)? { get set }
}

• 기존의 Coordinator 프로토콜에 종료될 때 실행될 stop 클로저와, 메모리에서 해제될 때 onDeinit을 부를 VC를 지정하도록 합니다.

/// 모든 Coordinator가 따르는 Coordinator 프로토콜입니다.
public protocol TestCoordinator: AnyObject {
    ...
    var stop: (() -> Void)? { get set}
    func setDeallocallable(with object: TestCoordinatorFinishOutput)
    var deallocallable: TestCoordinatorFinishOutput? { get set }
}

• 설정된 VC가 메모리에서 해제될 경우, stop을 실행합니다.

open class TestBaseCoordinator: TestCoordinator {
    public var stop: (() -> Void)?
    
    public func setDeallocallable(with object: any TestCoordinatorFinishOutput) {
        deallocallable?.onDeinit = nil // 기존 연결 해제
        object.onDeinit = { [weak self] in
            self?.stop?()
        }
        
        deallocallable = object
    }
    
    public weak var deallocallable: TestCoordinatorFinishOutput?
    

생각해볼 것

• 수동으로 finishFlow를 호출하지 않아도 되지만, 이 방법 또한 deallocallable 변수에 지정될 VC를 직접 등록해줘야 하는 불편함이 존재합니다. (이 또한 등록해주지 않으면 메모리에서 해제되지 않습니다.)

• 여러 방법들을 시도해봤는데 뾰족한 방법이 떠오르지 않네요... 더 좋은 의견 있으시면 남겨주세요 👀
• @yungu0010 @meltsplit

---

🚀 참고 자료

• 6년 묵은 레거시, RootController 리팩토링하기
• A deallocation approach for the Coordinator pattern

[meltsplit]

논의 1: 라우터를 제거하자.

저도 라우터의 필요성을 크게 못느껴서 직관적으로 NavigationController로 사용하는 것이 좋을 것 같아요.
현재 라우터에서 사용되고있는 아래 함수들을 어떻게 처리할지 논의 후에 제거해보면 좋을 것 같습니다~

func replaceRootWindow() { }
func presentAlertVC() { }
func presentNetworkAlertVC() { }
func showBottomSheet() { }

논의 2:VC의 생명주기에 맞춰서 Coordinator가 메모리에서 해제될 수 있도록 하자

문제 상황 너무 공감해요. finishFlow에서 removeDependency를 하는 것을 저도 까먹을 때가 종종 있어서 휴먼에러 발생 위험이 높더라구요
제시해주신 해결책이 finsishFlow를 작성하지 않아도 되는 문제를 해결해주신 것 같아요 :)
다만, VC용 protocol 및 deinit 함수를 작성해야한다는 것과 setDeallocallable에 직접 넣어줘야한다는 특징이 있는 것 같네요.
두 방법 모두 장단점이 있는 것 같아서, [논의 2]는 [논의 1라우터 제거] 작업을 끝낸 후에 다시 고민해보는 것은 어떨지 제안드려봅니다!

[dlwogus0128]

의견 남겨주셔서 감사해용가리 🐉 (케켘)

현재 라우터에서 사용되고있는 아래 함수들을 어떻게 처리할지 논의 후에 제거해보면 좋을 것 같습니다~

presentAlertVC, presentNetworkAlertVC의 경우, 알럿을 present하고자 하는 VC를 Router에 넘겨서, Router의 rootController에서 present하고자 하는 목적으로 만들어진 함수 같습니다. 그러나 프로젝트 전반적으로 알럿을 띄울 때 Router 내부의 함수가 아니라, AlertUtils의 함수를 사용하고 있는데요. (따라서 Router의 presentAlertVC, presentNetworkAlertVC는 사실상 구현만 되어있을 뿐, 사용하지 못하는 함수입니다.)

그 이유는 presentAlertVC, presentNetworkAlertVC 또한 앞서 언급한대로, Router의 rootController는 플로우가 시작된 상태에서 더 이상 새로운 VC를 present할 수 없기 때문입니다. AlertUtils의 함수들은 getMostTopViewController라는 커스텀 함수를 사용해, 재귀로 현재에서의 최상위 VC를 찾아 present하고 있기 때문에 알럿을 띄우는 것이 가능합니다.

그래서 Router의 presentAlertVC, presentNetworkAlertVC는 제거해도 될 것 같은데, 알럿을 띄우는 방식을 현재처럼 AlertUtils에게 최상위 VC를 찾아서 present하도록 책임을 맡기는 것이 아닌, 다른 VC들처럼 코디네이터 내에서 알럿을 생성해 present하는 방식으로 책임을 명확히 했으면 좋겠다는 의견입니다.

---

showBottomSheet은 Router의 rootController가 그 이상 present할 수 없다는 문제점이 있기 때문에, Router에게 present를 띄울 뷰와, 띄워질 대상 두 개의 뷰를 모두 인자로 받는 방식을 사용하고 있습니다. (rootController에서 present되지 못한다는 점에서 앞선 상황과 동일하지만, 최상위뷰를 주입받아 불필요한 로직을 줄인다는 목적으로 구현된 것 같습니다.)

그러나 해당 코드는 새로운 뷰를 띄울 VC를 강하게 참조할 경우, 아래와 같이 해당 VC가 메모리에서 해제되지 못할 수도 있습니다. 해당 PR

공통적으로 presentAlertVC, presentNetworkAlertVC, showBottomSheet들은 Builder의 역할만 해야 하는데, 화면 전환의 책임까지 맡게 되어 생기는 문제인 것 같습니다. 그래서 알럿이나 바텀시트 같이 잠깐 화면에서 present 되었다가 사라지는 VC들 또한 이 화면을 띄우는 역할을 코디네이터에서 해야 한다고 생각합니다.

---

replaceRootWindow는 커스텀된 함수인 것 같은데, 넘겨받은 VC를 네비컨으로 감싸고, 내부에 self.rootController를 변경하고자 하는 목적으로 만들어진 것 같습니다. 애니메이션 커스텀이 아니라면 Router가 제거되면서 자연스럽게 필요없어질 것 같아요!

---

이런 식으로 함께 Router가 들고 있는 함수들의 필요성을 얘기해보는 시간이 필요할 것 같네요...!! 🙇🏼

---

두 방법 모두 장단점이 있는 것 같아서, [논의 2]는 [논의 1라우터 제거] 작업을 끝낸 후에 다시 고민해보는 것은 어떨지 제안드려봅니다!

맞아요 저도 어제 생각해봤는데.. 일단 가장 큰 문제인 Router를 제거하고 나서 고민해도 늦지 않을 것 같더라구요?! 함께 얘기해보아요 👀

[meltsplit]

추가로 이번주 목요일까지 발생 가능한 화면 전환 경우의 수를 조금 정리해서 올게요!
이것 기반으로 라우터 제거 작업 안전하게 리팩토링 해보면 좋을 것 강아요~

[yungu0010]

답변이 늦어서 죄송합니다 ㅎ.ㅎ

논의 1: Router를 제거하자

저도 라우터를 제거하는 것에 동의합니다.
지난 회의에서 논의한 것처럼, Router 도입 이유로 추정되는 SRP 준수와 커스텀 화면 전환이 현재 프로젝트에서는 제대로 지켜지지 않고 있는 것 같아요.
선임개발자 의도와 달리 저희 프로젝트에서는 커스텀 화면전환이 불필요하다는 점, SRP가 지켜지지 않고 있다는 점에 공감했습니다.

앞서 두 분이 Router 내부 함수들의 필요성을 논의해보자고 하셔서 우선 존재하는 함수들에 대한 역할과 필요 여부를 간단히 정리해보았어요. 개인적인 의견도 살짝 적어두었습니다!_!
🔗 라우터 내부 함수들의 쓰임과 필요성

그래서 Router의 presentAlertVC, presentNetworkAlertVC는 제거해도 될 것 같은데, 알럿을 띄우는 방식을 현재처럼 AlertUtils에게 최상위 VC를 찾아서 present하도록 책임을 맡기는 것이 아닌, 다른 VC들처럼 코디네이터 내에서 알럿을 생성해 present하는 방식으로 책임을 명확히 했으면 좋겠다는 의견입니다.

저도 이 의견에 동의합니다. 성격이 비슷한 Alert, BottomSheet, SFSafariViewController는 커스텀VC를 생성하는 Utils(또는 Manager) 로 객체 생성 함수를 두고, 화면전환은 코디네이터에서 하는 것으로 리팩토링 진행해보아요!

추가) OPNavigation

현재 사용되고 있는 커스텀 Navigation 중 솝탬프에서 사용되는 OPNavgation만 init 시 VC를 주입받아 NavigationBar에서 pop해주고 있는데, 이 부분도 코디네이터에서 처리하는건 어떠신가요 ?

[dlwogus0128]

왕 벌써 Router에서 쓰이는 함수들을 정리해주셨네요 감사합니다! 더 살펴봐야하겠지만 라우터에 구현되어있는데 사용되지 못하고 있었던 함수들이 많았던 것 같아요.

현재 사용되고 있는 커스텀 Navigation 중 솝탬프에서 사용되는 OPNavgation만 init 시 VC를 주입받아 NavigationBar에서 pop해주고 있는데, 이 부분도 코디네이터에서 처리하는건 어떠신가요 ?

저도 그동안 네비게이션바에서 화면 전환 로직을 맡고 있었던 것이 어색하게 느껴졌는데, 네비게이션바에서 발생하는 이벤트에 따른 화면 전환도 코디네이터에서 담당해야 한다고 생각합니다.

[yungu0010]

논의 2: VC의 생명주기에 맞춰서 Coordinator가 메모리에서 해제될 수 있도록 하자

논의 1을 함께 리팩토링 한 후 논의 2를 진행하자는 의견에도 동의합니다!
대신 논의 2와 관련된 아티클을 하나 찾아 공유드려요. 🔗 관련 아티클 해당 아티클의 Self-deallocated child coordinators approach 부터 읽어보시면 됩니다!

다만 저희 프로젝트에서는 한 VC당 하나의 코디네이터를 소유하고 있는 것이 아닌 부분도 있는 것 같아서 그대로 적용하긴 어려워보여요. 논의 1에 대한 리팩토링 후 이 부분 다시 한 번 살펴보겠습니다.

아티클 내용에 대한 이해를 바탕으로 수도코드를 작성해보았는데 참고해주세요!

/// 코디네이터
class ACoordinator {
    private weak var navigationController: UINavigationController?    // 순환참조 방지
    
    ...
    let viewModel = AViewModel()
    let VC = AViewController(viewModel: viewModel)
    
    // ViewModel이 Coordinator 강한 참조
    viewModel.coordinator = self
}

/// 뷰모델
class AViewModel {
    var coordinator: ACoordinator?     // ViewModel이 Coordinator 강한 참조
}

/// 뷰컨트롤러
class AViewController {
    var viewModel: AViewModel
    
    init(viewModel: any ViewModel) {
        self.viewModel = viewModel
    }
}

VC -> VM (강한참조)
VM -> Coordinator (강한참조)
Coorinator -> NavigatoinController (약한참조)

VC메모리에서 해제 -> VM메모리에서 해제 -> Coordinator 메모리에서 해제

[dlwogus0128]

이 방법이 더 좋아보이는데요...! 이렇게 하면 코디네이터가 들고 있는 VC들의 생명주기에 맞춰서 자연스럽게 Coordinator가 메모리에서 해제될 수 있을 것 같아요!! 코디네이터를 메모리에 존재하게 하기 위해서 상위 코디네이터에 dependency를 등록할 필요가 없어져서 훨씬 간단해보여요.