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

Releasing child coordinators #25

Closed
teameh opened this issue Jan 31, 2020 · 2 comments
Closed

Releasing child coordinators #25

teameh opened this issue Jan 31, 2020 · 2 comments
Assignees
Labels

Comments

@teameh
Copy link

teameh commented Jan 31, 2020

Hi Igor,

Thanks for sharing this repo, lots of good stuff! I wasn't sure where to ask you a question regarding the code so I'll add an issue, hope that's okay.

Regarding releasing the child coordinators in the AppCoordinator, is there a reason you don't clean up the other child coordinator when switching view's? eg: childCoordinators[.setup] = nil

And I was wondering, in the FeedCoordinator, you check if it's transitioning from the setup (with !navigationController.viewControllers.isEmpty. Wouldn't it be better to check this in the AppCoordinator to separate out the responsibilities better? The FeedCoordinator does not need to know anything about the setup right?

Oh and I like your child-coordinator enums a lot, will use this for my next project!

Cheers!

@igorkulman
Copy link
Owner

Regarding releasing the child coordinators in the AppCoordinator, is there a reason you don't clean up the other child coordinator when switching view's? eg: childCoordinators[.setup] = nil

You are right, the AppCoordinator does not do any cleanup, I jut forgot about it there I guess. Thanks for noticing, I will add it.

And I was wondering, in the FeedCoordinator, you check if it's transitioning from the setup (with !navigationController.viewControllers.isEmpty. Wouldn't it be better to check this in the AppCoordinator to separate out the responsibilities better? The FeedCoordinator does not need to know anything about the setup right?

I think this is just a problem with naming. The FeedCoordinator always wants to push its first VC as the top level VC, so it clears the navigation stack if empty. I will make the naming better so there is no mention if setup.

@teameh
Copy link
Author

teameh commented Jan 31, 2020

That makes sense, thanks for clarifying

@igorkulman igorkulman self-assigned this Feb 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants