[Work 31] ViewController의 화면전환/조립 책임을 Coordinator와 ScreenFactory로 분리했습니다. by snughnu · Pull Request #20 · HmmCorn/WhereAreYou_iOS

snughnu · 2026-09-03T04:30:36Z

JIRA

📝 작업 내용

📌 요약

ViewController가 UseCase/Repository를 직접 조립하고 화면전환(push/present)까지 담당하던 구조를,
Coordinator(화면전환 전담)와 ScreenFactory(조립 전담)로 분리했습니다.
여러 화면에 중복 구현돼 있던 조립 로직(예: ChatViewModel 조립, SearchPlaceCardViewController 조립)을
ScreenFactory 쪽 공용 메서드 하나로 통합했습니다.

🔍 상세

1. Coordinator / ScreenFactory 기반 구조 도입

문제 상황
- ViewController 하나가
  "Repository resolve → UseCase 조립 → ViewModel 생성 → push/present"까지 전부 담당하고 있었습니다.
해결 방법
- Coordinator
  - start()만 갖는 최소 프로토콜입니다.
  - 내비게이션 스택이 필요 없는 화면전환(AppCoordinator의 루트 화면 교체)도 채택할 수 있도록
    UINavigationController 요구사항을 넣지 않았습니다.
- NavigationCoordinator
  - Coordinator를 상속하며 navigationController: UINavigationController를 요구하는 프로토콜입니다.
  - push, present, presentInNavigationController 등 push/present 보일러플레이트를 extension으로 제공합니다.
- ScreenFactory
  - DIContainer를 들고 있으면서,
    화면별 조립 메서드(makeChatViewController, makeAppointmentRouteViewController 등)를
    화면 단위 extension 파일로 나눠 제공합니다.
  - Repository/UseCase 조합 코드는 이 레이어에만 존재하고, Coordinator와 ViewController는 이 코드를 갖지 않습니다.

2. 탭/화면별 Coordinator 도입

AppCoordinator / TabBarCoordinator
- SceneDelegate가 직접 하던 로그인 화면 표시,
  로그인 성공 시 탭바 전환(cross-dissolve 애니메이션 포함),
  3개 탭의 UINavigationController 및 탭 Coordinator 생성을 이전했습니다.
HomeCoordinator / AppointmentListCoordinator / MyPageCoordinator
- 각 탭의 루트 화면 및 하위 화면전환을 담당합니다.
- AppointmentListCoordinator는
  AppointmentListViewController와 PastAppointmentListViewController가
  같은 내비게이션 스택을 공유하므로 하나의 Coordinator로 통합했습니다.
ChatCoordinator
- ChatViewController가 모달로 띄우던 화면(약속 경로, 약속 정보, 내 위치 공유, 장소 검색/공유, 공유 장소 목록)과
  그 안의 AppointmentInfoViewController가 갖고 있던 하위 전환(날짜 선택, 장소 검색, 장소 지도 선택)까지 함께 담당합니다.
RouteSearchCoordinator
- RouteSearchViewController가 항상 자체 UINavigationController로 감싸져 모달로 뜨는 독립 흐름이라,
  그 내비게이션 컨트롤러를 직접 생성해 소유하는 전용 Coordinator로 분리했습니다.

💬 리뷰 노트

1. Coordinator 단독이 아닌 Coordinator + Factory 조합을 선택한 이유

Coordinator 패턴만 단독 도입
- 화면전환 흐름을 한눈에 볼 수 있다는 장점은 있지만,
  결국 Coordinator 자체가 UseCase 조립까지 떠안게 되어 "조립" 문제는 해결되지 않습니다.
DIContainer/ScreenFactory 확장만 단독 도입
- 조립 문제는 해결되지만, push/present 같은 전환 책임은 여전히 ViewController에 남습니다.
Coordinator + Factory 조합 (채택)
- ViewController: 화면 표시 / ScreenFactory: 조립 / Coordinator: 전환으로 책임을 3분할해,
  두 문제를 함께 해결할 수 있다고 판단했습니다.

2. Coordinator 프로토콜에서 UINavigationController를 필수로 두지 않은 이유

처음에는 Coordinator 프로토콜 자체에 navigationController: UINavigationController를 요구사항으로 뒀는데,
AppCoordinator처럼 UIWindow.rootViewController를 직접 교체하는 화면전환에는 내비게이션 스택 자체가 필요 없었습니다.
Coordinator(최소 프로토콜)와 NavigationCoordinator(내비게이션 스택이 필요한 경우에만 채택)로 분리해,
push 기반 전환과 루트 화면 교체 양쪽을 모두 자연스럽게 표현할 수 있게 했습니다.

3. ChatCoordinator를 각 탭 Coordinator가 어떻게 공유하는지

Chat 화면은 Home/AppointmentList 어느 탭에서 진입했는지에 따라 속한 내비게이션 스택이 다릅니다.
그래서 ChatCoordinator를 미리 만들어두지 않고,
HomeCoordinator/AppointmentListCoordinator가 채팅 화면을 push하는 시점에
자신의 navigationController를 넘겨 ChatCoordinator를 생성하고,
private var chatCoordinator: ChatCoordinator?로 강하게 소유하는 방식을 택했습니다.

github-actions

🤖 AI 코드 리뷰 - 아키텍처 관점

SceneDelegate에 인라인으로 작성되어 있던 화면 전환 로직 전체를 Coordinator 패턴으로 분리한 PR이다. ScreenFactory를 통해 DI를 일원화하고, View에서 직접 DIContainer를 참조하던 코드를 제거한 점은 의존성 방향 측면에서 개선이다. 다만 몇 가지 구조적 문제가 잔존한다. 첫째, Presentation 계층의 ViewController가 Coordinator의 구체 타입을 직접 참조하고 있어 계층 간 결합이 강하게 남아 있다. 둘째, HomeCoordinator가 UseCase를 직접 생성·실행하는 joinAppointment 메서드를 보유해 Coordinator의 책임 범위를 벗어난다. 셋째, ChatCoordinator.showAppointmentInfo가 ViewController를 반환하는 설계는 Coordinator 추상화와 일관성이 없다.

github-actions · 2026-09-03T04:31:28Z

 final class AppointmentCreationViewController: UIViewController {

    private let viewModel: AppointmentCreationViewModel
+    weak var coordinator: HomeCoordinator?


[HIGH] weak var coordinator: HomeCoordinator?처럼 Presentation 계층이 Coordinator 구체 타입을 직접 참조하고 있다. 동일한 패턴이 AppointmentListViewController, ChatViewController, HomeViewController 등 전체 ViewController에 반복된다. Coordinator를 프로토콜로 추상화하지 않으면 ViewController가 특정 Coordinator 구현에 강결합되어, 화면 흐름 변경 시 ViewController 코드도 함께 수정해야 하고 단독 테스트도 불가능하다. 각 ViewController가 필요로 하는 화면전환 인터페이스만 선언한 프로토콜(예: AppointmentCreationCoordinatorProtocol)을 정의하고, coordinator 프로퍼티를 해당 프로토콜 타입으로 선언해야 한다.

weak var coordinator: HomeCoordinator? 처럼 구체 타입을 직접 참조하던 부분을
각 ViewController가 필요로 하는 화면전환 메서드만 선언한 프로토콜로 추상화했습니다.

프로토콜 단위는 실제로 그 프로토콜을 쓰는 ViewController 기준으로 나눴습니다.
예를 들어 ChatCoordinator 하나가 ChatViewController와 AppointmentInfoViewController 양쪽에 쓰이는데,
두 화면이 실제로 호출하는 메서드가 겹치지 않아서
ChatCoordinating / AppointmentInfoCoordinating 두 프로토콜로 분리하고,
ChatCoordinator가 둘 다 채택하도록 했습니다.

AppointmentRouteViewController의 RouteSearchCoordinator 직접 생성/소유 문제도 함께 정리했습니다. AppointmentRouteViewController는 이제 AppointmentRouteCoordinating 프로토콜의 showRouteSearch(from:destination:)만 호출하고, RouteSearchCoordinator의 생성과 소유는
이 화면을 실제로 present한 상위 Coordinator가 담당하도록 옮겼습니다.

github-actions · 2026-09-03T04:31:28Z

+
+    // MARK: - Join
+
+    func joinAppointment(code: String, completion: @escaping (Result<Void, Error>) -> Void) {


[HIGH] joinAppointment(code:completion:) 메서드 안에서 JoinAppointmentUseCase를 직접 생성하고 실행하고 있다. Coordinator의 역할은 화면 전환 조율이며, 비즈니스 로직 실행은 ViewModel 또는 UseCase 계층의 책임이다. 현재 구조에서는 screenFactory.container.resolve(...)를 통해 Repository를 꺼내 UseCase를 만드는 코드가 Coordinator에 존재한다. 이 로직은 HomeViewModel 혹은 별도의 ViewModel로 이동시키고, Coordinator는 결과에 따른 화면 전환만 담당해야 한다.

로직을 HomeViewModel로 옮겼습니다.
같은 패턴이 MyPageCoordinator.start()에도 있어서(MyPageViewModel을 UseCase까지 직접 조립) 함께 정리했습니다.

3119c8a

github-actions · 2026-09-03T04:31:28Z

+
+    // MARK: - Appointment Info
+
+    func showAppointmentInfo(appointmentID: String) -> AppointmentInfoViewController {


[MEDIUM] showAppointmentInfo(appointmentID:) -> AppointmentInfoViewController가 ViewController 인스턴스를 반환한다. 호출부인 ChatViewController에서 _ = coordinator?.showAppointmentInfo(...)로 반환값을 버리고 있어, 반환 타입 자체가 현재 사용되지 않는다. Coordinator가 ViewController를 반환하는 설계는 Coordinator의 책임 범위(화면 전환)를 벗어나며, 나머지 show* 메서드가 모두 Void를 반환하는 것과 일관성이 없다. 반환값이 실제로 필요한 경우라면 콜백 패턴으로 대체하고, 그렇지 않으면 반환 타입을 Void로 변경해야 한다.

showAppointmentInfo(appointmentID:)의 반환값(AppointmentInfoViewController)이
호출부(ChatViewController)에서 _ =로 버려지고 있던걸 Void를 반환하도록 수정했습니다.

github-actions · 2026-09-03T04:31:28Z

-        tabBarController.viewControllers = [homeNav, listNav, myPageNav]
-        tabBarController.tabBar.tintColor = .blue1
-        return tabBarController
+        let coordinator = AppCoordinator(window: window!)


[MEDIUM] window! 강제 언래핑을 사용하고 있다. 바로 윗줄(17)에서 window = UIWindow(windowScene: windowScene)으로 할당했으므로 nil이 아님을 알 수 있지만, guard let으로 안전하게 꺼낸 뒤 사용하는 방식이 구조적으로 더 안전하다. guard let window 바인딩 이후 AppCoordinator(window: window)로 전달하면 강제 언래핑 없이 동일하게 동작한다.

강제 언래핑을 제거했습니다. 🫡

github-actions · 2026-09-03T04:31:28Z


    private let viewModel: AppointmentRouteViewModel
    private var cancellables = Set<AnyCancellable>()
+    private var routeSearchCoordinator: RouteSearchCoordinator?


[MEDIUM] private var routeSearchCoordinator: RouteSearchCoordinator?를 ViewController가 직접 소유하고 있다. AppointmentRouteViewController는 ChatCoordinator 또는 HomeCoordinator에 의해 present되는 화면인데, 그 안에서 다시 하위 Coordinator를 생성·보유하는 구조가 된다. Coordinator 트리의 소유권이 Coordinator가 아닌 ViewController에 분산되어, 생명주기 관리가 불명확해진다. RouteSearchCoordinator의 생성과 소유는 AppointmentRouteViewController를 present한 상위 Coordinator가 담당하거나, 최소한 AppointmentRouteViewController에 coordinator 프로퍼티를 주입하는 방식으로 일관성을 맞춰야 한다.

#20 (comment)

같이 처리했습니다.

…y로 이전

sangYuLv

조립하는 ScreenFactory를 만든 점이 정말 좋은 것 같아요!
수고 많으셨습니다 🦦

중요한 작업인만큼 한 번 더 AI 리뷰를 받아보는 건 어떨까요?
재리뷰에서도 좋은 얘기를 해주기도 하더라구요!

이번 작업을 읽으면서 느낀 건데, 전체적으로 네이밍 수정/검토가 필요할 것 같아요.
단어 조합(e.g. PlaceSelection, PlaceSearch, RouteSearch)이 어떤 화면/기능을 의미하는지 잘 떠오르지 않거나 헷갈리기도 하네요.
큰 변화는 없을 것 같지만, 괜찮으시다면 나중에 한 번 작업을 진행해보겠습니다!

제 코멘트에 대한 답변이나 수정 작업이 모두 완료되면 리뷰 재요청 부탁드립니다. 파이팅 🫡

sangYuLv · 2026-09-04T14:24:45Z

+        chatCoordinator = coordinator
+        chatViewController.coordinator = coordinator
+        push(chatViewController)
+    }


제가 파악하기로는 이 coordinator에서 채팅 화면을 표시하는 로직이 3갈래 존재합니다.

func replaceAppointmentCreation(_ appointmentCreationViewController: UIViewController, withChatFor appointmentInfo: AppointmentInfo) : 약속 생성 후 채팅 화면 이동

showChat(appointmentInfo: AppointmentInfo) : 약속 코드로 참여 후 채팅 화면 이동

showChat(appointmentID: String): 이외 케이스

위 메소드 안에서 쓰이는 screenFactory의 메소드도 다른데, 약속 정보를 조회한 후 표시하는지와 가지고 있는 약속 정보를 기반으로 표시하는지의 차이점을 확인했습니다.

showChat(~)과 달리 replaceAppointmentCreation()은 push()대신 직접 viewControllers를 관리하는 로직을 포함하는데, 어떤 차이를 두고자 하셨는지 궁금합니다.

sangYuLv · 2026-09-04T14:30:39Z

sangYuLv · 2026-09-04T14:45:01Z

showDatePicker(= showTimePicker)나 showPlaceSearch(=showSearchPlace)처럼 반복되는 coordinator 메소드 구현부는 한 곳으로 모으면 어떨까요?
최대로 반복되는 횟수가 3번이어서, 만약 바꾸지 않는 쪽을 선호한다면 메소드 이름이라도 통일하면 좋을 것 같습니다!

sangYuLv · 2026-09-04T14:49:17Z

+
+    init(
+        navigationController: UINavigationController,
+        screenFactory: ScreenFactory = ScreenFactory()


ScreenFactory를 필요로 하는 coordinator들은 초기화 메소드에서 기본값으로 새 인스턴스를 생성하고 있습니다.
전역에서 한 screenFactory만 가져도 괜찮지 않을까요?

더해서 각 coordinator마다 필요로 하는 스크린 생성 메소드가 다른데, 프로토콜로 분리해 접근을 제한하면 어떨까요?
구현부는 screenFactory의 extension으로 잘 분리되어있지만, 특정 coordinator가 필요한 메소드만 알고 있도록 해도 좋을 것 같아서 제안드려봅니다!

sangYuLv · 2026-09-04T14:50:59Z

이 로직이면 앱 실행마다 로그인을 요구할 것 같아서 일정 기간 로그인 상태를 유지하는 기능을 추가하면 좋을 것 같습니다!
어떻게 생각하시나요?
괜찮으시다면 백로그에 추가하겠습니다.

snughnu added 7 commits September 3, 2026 09:07

REFACTOR: Coordinator/ScreenFactory 기반 구조 추가

6ee885d

REFACTOR: Home 화면전환 로직을 HomeCoordinator로 이전

7317244

REFACTOR: AppointmentList 화면전환 로직을 AppointmentListCoordinator로 이전

66cf8fe

REFACTOR: MyPage 화면전환 로직을 MyPageCoordinator로 이전

3925a36

REFACTOR: Chat 화면전환 로직을 ChatCoordinator로 이전

7ae8188

REFACTOR: AppointmentCreation/RouteSearch 화면전환 로직을 Coordinator로 이전

2e4cd7b

REFACTOR: SceneDelegate 화면전환 로직을 AppCoordinator로 이전

97ea70d

snughnu requested a review from sangYuLv September 3, 2026 04:30

snughnu self-assigned this Sep 3, 2026

snughnu added 🧱 REFACTOR review:architecture labels Sep 3, 2026

github-actions Bot reviewed Sep 3, 2026

View reviewed changes

snughnu removed the review:architecture label Sep 3, 2026

snughnu added 4 commits September 3, 2026 14:00

REFACTOR: ViewController가 Coordinator 구체 타입 대신 프로토콜을 참조하도록 변경

6a4489d

REFACTOR: Coordinator에 남아있던 UseCase 조립/실행 로직을 ViewModel과 ScreenFactor…

3119c8a

…y로 이전

REFACTOR: showAppointmentInfo가 사용하지 않는 반환값을 갖지 않도록 수정

cb1865d

FIX: SceneDelegate의 window 강제 언래핑 제거

497a383

sangYuLv reviewed Sep 4, 2026

View reviewed changes


		// MARK: - Join

		func joinAppointment(code: String, completion: @escaping (Result<Void, Error>) -> Void) {


		// MARK: - Appointment Info

		func showAppointmentInfo(appointmentID: String) -> AppointmentInfoViewController {

Uh oh!

Conversation

snughnu commented Sep 3, 2026

📝 작업 내용

📌 요약

🔍 상세

1. Coordinator / ScreenFactory 기반 구조 도입

2. 탭/화면별 Coordinator 도입

💬 리뷰 노트

1. Coordinator 단독이 아닌 Coordinator + Factory 조합을 선택한 이유

2. Coordinator 프로토콜에서 UINavigationController를 필수로 두지 않은 이유

3. ChatCoordinator를 각 탭 Coordinator가 어떻게 공유하는지

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

🤖 AI 코드 리뷰 - 아키텍처 관점

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 3, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sangYuLv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sangYuLv Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sangYuLv Sep 4, 2026 •

edited

Loading