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

앱, 탭바 코디네이터 구현 #63

Merged
merged 30 commits into from Nov 10, 2021
Merged

Conversation

jeonyeohun
Copy link
Collaborator

📕 Issue Number

Close #56

📙 작업 내역

구현 내용 및 작업 했던 내역

  • AppCoordinator 구현
  • TabBarCoordinator 구현
  • HomeCoordinator 구현

📘 작업 유형

  • 신규 기능 추가
  • 버그 수정
  • 리펙토링
  • 문서 업데이트

📋 체크리스트

  • Merge 하는 브랜치가 올바른가?
  • 코딩컨벤션을 준수하는가?
  • PR과 관련없는 변경사항이 없는가?
  • 내 코드에 대한 자기 검토가 되었는가?
  • 변경사항이 효과적이거나 동작이 작동한다는 것을 보증하는 테스트를 추가하였는가?
  • 새로운 테스트와 기존의 테스트가 변경사항에 대해 만족하는가?

📝 PR 특이 사항

PR을 볼 때 주의깊게 봐야하거나 말하고 싶은 점

  • 이번구현에 참고한 레퍼런스:

  • TabBarController 삭제

    코디네이터에서 TabBarController를 하나 들고 각 탭에 대한 전환을 관리해주기 때문에 탭 바 컨트롤러의 내용이 탭 바 코디네이터로 합쳐지면서 기존 탭바컨트롤러가 필요하지 않게 되었습니다. 탭바에 대해서는 여러가지 방법들이 인터넷에 있는데 위에 첨부한 첫번째 래퍼런스의 방법을 공부하면서 따라하다보니 이렇게 되었는데, 분리가 필요하다고 생각하시면 다시 또 고민해보겠습니다. 지금은 SceneDelegate에서
    AppCoordinator를 만들고, AppCoordinator를 시작하면 TabBarCoordinator가 실행되면서 기본화면인 HomeViewController를 기준으로 각 탭마다 자식 코디네이터를 가지는 구조입니다.

스크린샷 2021-11-09 오후 5 32 28

그림처럼 설계를 고민하고 있는데, 마음처럼 될지는 실제로 구현해보면서 계속 검증을 해봐야할 것 같습니다.

  • HomeViewController
    유즈케이스, 뷰모델 외부주입과 화면전환을 테스트하기 위해서 이번 피알과는 큰 관련이 없지만 HomeViewController를 수정하고 HomeViewModel과 HomeUseCase를 추가했습니다. 일단 만들어두고 runningSetting을 넘기는 작업을 우선적으로 구현하겠습니다.

  • Coordinator 위치
    코디네이터는 두 가지 이유때문에 뷰모델에 위치하도록 했습니다.

    1. 뷰모델이 화면에서 발생하는 이벤트를 몰라도 괜찮은가?
      이게 맞는 생각인지 확신은 없지만 저는 뷰모델에는 화면에서 발생하는 모든 이벤트가 전달되어야 한다는 생각이 있습니다. 뷰모델로 구조를 나누는 이유가 뷰에서 일어나는 일을 테스트할 수 있게 하기 위해서라고 생각하는데 단위테스트나 시나리오 테스트를 진행할 때 화면 전환이 어떤 경우에서 트리거되는지 테스트 하려면 이벤트가 무조건 뷰모델에는 한번 가야하고 뷰모델에서 코디네이터를 호출해야한다고 생각했습니다. 그렇지 않으면 간단한 예로, 버튼이 눌렸을 때 다른 화면으로 전환이 되는지를 테스트할 수 없고, 뷰모델에 이벤트를 보낸뒤에 다시 뷰컨트롤러에서 코디네이터를 호출하기에는 뷰모델에서는 아무일도 하지 않은 채로 뷰 컨트롤러에 함수만 호출하게 되는 격인 것 같습니다.
    2. 코디네이터를 통해 다른 화면에 전달할 데이터가 유즈케이스에 있다면?
      RunningSetting 을 유즈케이스에 둔다면, 뷰 컨트롤러에서 코디네이터를 호출할 때 뷰모델 -> 유즈케이스까지 들어가서 값을 가지고나와야하는데 두번거쳐서서 뷰컨트롤러가 유즈케이스를 간접적으로라도 접근하는 것이 좋게 느껴지지는 않았습니다..

탭바컨트롤러나 코디네이터 위치에 대해서 다른 의견이 있으시면 알려주세요! 다음작업은 이 브랜치를 한번 더 브랜칭해서 진행하도록 하겠습니다!



jeonyeohun added 25 commits November 9, 2021 11:43
- 프로토콜의 이름을 Coordinating에서 Coordinator로 변경했습니다.
- Common 내에 protocol 디렉토리를 하나 더 만들고 그 안에 프로토콜을 두었습니다. AppCoordinator 구현체가 있어서 구분을 위해 이렇게 나눴습니다.
@jeonyeohun jeonyeohun added feature New feature or request review Review is required 여훈 refactor 코드 리팩토링 labels Nov 9, 2021
@jeonyeohun jeonyeohun self-assigned this Nov 9, 2021
@jeonyeohun jeonyeohun added this to In progress in 3주차 via automation Nov 9, 2021
Copy link
Collaborator

@mingd1023 mingd1023 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

너무 깔끔하고 좋네요..! 잘 작성해주셔서 코드 이해도 쉬웠습니다 수고 많으셨어요!

MateRunner/MateRunner/Application/SceneDelegate.swift Outdated Show resolved Hide resolved
Comment on lines +25 to +27
func showLoginFlow() {
// 로그인 화면 구현 후 작성
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로그인까지 큰 그림을 생각하시다니...👍🏻

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 이건 의도한건 아닌데 예제를 따라하다보니까 생겼어요ㅋㅋㅋㅋzedd님의 큰그림..

Copy link
Member

@devJungwonLee devJungwonLee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아직 Coordinator 패턴에 대해 완전히 이해하지 못해서 제대로 된 리뷰를 못한 것 같아 죄송하네요 🥲
이 부분은 여훈님이 스크럼 시간에 한번 말로 설명해주셔도 좋을 것 같아요!

}

func start() {
let pages: [TabBarPage] = [.home, .record, .mate, .mypage]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TabBarPage 라는 enumCaseIterable을 채택한다면
TabBarPage.allCases.sorted~
이렇게도 쓸 수 있을 것 같아요 ☺️

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

검색해보니 allCases의 순서는 정의한 순서대로라네요
그럼 정렬도 안해도 될 것 같아요..!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

정렬은 제가 참고했던 글에서 사용해서 저도 처음에 의아했는데 pages에 나중에 새로운 탭 추가돼도 실수로 순서 안꼬이게 하려고 그런 것 같더라구요! 근데 말씀하신것 처럼 allCases 쓰면 될 것 같아서 수정해두겠습니다!


import Foundation

enum TabBarPage: String {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드를 보면 TabBarController의 index로 주로 쓰이는 것 같은데, 그렇다면 String 대신 Int는 어떤가요?
별도의 initializer없이 TabBarPage(rawValue: index)이렇게 생성할 수 있을 것 같아서요
page.pageOrderNumber()page.rawValue로 쓸 수 있을 것 같네요..!

그리고 tabIconName()CustomStringConvertible 채택해서 description으로 처리해도 좋지 않을까요? ㅎㅎ
UIImage(named: "\(page)") 이런식으로요!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

두 방법 다 좋은 방법인 것 같습니다! 그런데 저는 rawValue 사용을 지양해야된다고 생각하는게 코드의 의미가 불분명해진다고 느꼈습니다..
TabBarPage(rawValue) 보다는 TabBarPage(index)가 더 의사전달이 분명하지 않을까요?
page.description 이나 page.rawValue도 같은 맥락으로 의미가 불분명해지는 것 같습니다..! 제 생각이니 의견 있으시면 말씀해주세요!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 enum안의 switch문을 줄이고자 생각해본 방법인데 여훈님 말씀 들어보니까 잘못된 접근인 것 같네요 😂

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아닙니다 좋은 의견 주셔서 감사해요!

Copy link
Collaborator

@lee-yujinn lee-yujinn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

항상 좋은 구조를 위해 고민 해주셔서 감사합니다!! 🙇‍♂️

return HomeViewController()
}
return homeViewController
case .record: return HomeViewController()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분에서 record, mate,mypage일때 왜 HomeViewController를 리턴하는건지 알 수 있을까요..!?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기록, 친구, 마이페이지 탭의 뷰컨이 없어서 홈뷰로 일단 다 채웠어요..



func scene(_ scene: UIScene, willConnectTo session: UISceneSession, options connectionOptions: UIScene.ConnectionOptions) {
guard let windowScene = (scene as? UIWindowScene) else { return }
self.window = UIWindow(windowScene: windowScene)
self.window?.rootViewController = TabBarController()
self.window?.makeKeyAndVisible()
self.window?.tintColor = .mrPurple
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분이 사라진다면 전역적으로 tintColor가 적용되지 않을것 같은데..!!혹시 다른부분에서 해주셨는데 제가 놓친걸까요 👀

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

헐..제가 모르고 중요한것까지 삭제했네요.. 다시 돌려놓겠습니다! 꼼꼼하게 봐주셔서 감사해요!


import Foundation

enum CoordinatorType {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

혹시 이렇게 type별로 enum을 만들고 분리한 이유를 알 수 있을까요?!
제가 못찾는건지 코드상으로 type을 활용하신 부분이 없는것 같아서 혹시 어떤식으로 활용하시는지가 궁금합니다..!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

제가 childCoordinator가 종료됐을 때의 로직을 아직 구현을 안했는데, 래퍼런스로 첨부한 글에서는 부모 코디네이터에서 자식 코디네이터의 타입을 통해서 자식 코디네이터를 삭제하고 있어요

extension AppCoordinator: CoordinatorFinishDelegate {
    func coordinatorDidFinish(childCoordinator: Coordinator) {
        childCoordinators = childCoordinators.filter({ $0.type != childCoordinator.type })

        switch childCoordinator.type {
        case .tab:
            navigationController.viewControllers.removeAll()
           // action after remove 
            showLoginFlow() 
        case .login:
            navigationController.viewControllers.removeAll()
           // action after remove  
            showMainFlow()
        default:
            break
        }
    }
}

여기서는 코디네이터가 완료됐을 때 타입으로 필터링을 한번해서 배열에서 자식 코디네이터를 없애고 이후에 어떤 작업을 할지 switch로 다시 정의하고 있습니다! 위 예제에서는 tab 타입의 코디네이터가 종료되면 longin으로 이동하고, longin 코디네이터가 종료되면 탭바화면으로 이동해요!

지금 코드에서는 프로토콜만 만들어두고 어디에서도 채택하고 있지 않은데, 아직 제가 자식 코디네이터의 종료에 대해 정의를 확실하게 못해서 내일 서치를 좀 더 해보고 적용하려고 보류해두었습니다!

눈썰미가 좋으십니다..👍

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

오.. 이렇게 관리를 하는군요.. 👀 여훈님이 플젝에 잘 적용해주시는 덕분에 많이 배워갑니다 🙇‍♂️

@devJungwonLee devJungwonLee merged commit 85aa598 into dev Nov 10, 2021
3주차 automation moved this from In progress to Done Nov 10, 2021
@jeonyeohun jeonyeohun deleted the refactor/app-coordinator branch November 10, 2021 01:21
@jeonyeohun jeonyeohun linked an issue Nov 17, 2021 that may be closed by this pull request
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request refactor 코드 리팩토링 review Review is required 여훈
Projects
4 participants