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

Who should be responsible for building a ViewController? #14

Closed
AndrewSB opened this issue Aug 31, 2016 · 4 comments
Closed

Who should be responsible for building a ViewController? #14

AndrewSB opened this issue Aug 31, 2016 · 4 comments
Labels

Comments

@AndrewSB
Copy link

From what I see, the coordinator is in charge of initing the viewController, injecting itself as its handler, and setting it's corduxContext.

Example ForgotPasswordViewController:

class AuthenticationCoordinator {
    func createForgotPasswordViewController() -> ForgotPasswordViewController {
        let forgotPasswordViewController = storyboard.instantiateViewController(withIdentifier: "ForgotPassword") as! ForgotPasswordViewController
        forgotPasswordViewController.inject(self)
        forgotPasswordViewController.corduxContext = Context(RouteSegment.fp, lifecycleDelegate: self)
        return forgotPasswordViewController
    }
}

Why not instead choose to make the viewController responsible for it's own creation? Factory function style:

class ForgotPasswordViewController {
    static func build(withCoordinator coordinator: Coordinator, context ctx: Context) -> ForgotPasswordViewController {
        ...
   }
}

There could even be a default implementation of this function, from same BaseVC, or protocol conformance of the viewController, so a client of the library wouldn't have to remember to create the VC, and then inject a coordinator & context.

@AndrewSB
Copy link
Author

Would love to hear your thoughts on why it was implemented this way 😄

@AndrewSB AndrewSB changed the title Who is responsible for building a ViewController? Who should be responsible for building a ViewController? Aug 31, 2016
@aenewton
Copy link

Hi @AndrewSB,

One of the principles behind Cordux is that view controllers don't need to know about it. You'll notice that ForgotPasswordViewController.swift doesn't even import Cordux. This makes view controllers even smaller and usable in non-Cordux projects.

@AndrewSB
Copy link
Author

AndrewSB commented Aug 31, 2016

That makes a lot of sense! I like how it reduces complexity for the viewControllers.

But on the other hand, I don't like that the responsibility of viewController initialization & injection is given to the coordinator.

Thinking out loud now:
In a larger project, one might create a module for each feature. So for the forgot password feature, you'd create a ForgotPassword module, which might have these 4 files:

* ForgotPassword
    * ForgotPassword.ViewModel
    * ForgotPassword.Handler
    * ForgotPassword.ViewController
    * ForgotPassword.Builder

what do you think of handing the responsibility of creating the viewController into a sub-object, a Builder (or creator, or [insert other name])? This way the view controller is still oblivious to cordux, and the coordinator becomes simpler

@ianterrell
Copy link
Contributor

Hi @AndrewSB,

I do agree that the common lines of injecting the coordinator as a handler and then setting a view controller's corduxContext are less than ideal. I certainly welcome ideas for how to clean that up!

In my opinion, the coordinator is ultimately responsible for view controller creation, even if it chooses to delegate that responsibility to a builder object. Or even if it did so via a class level build or make (I think Swift's preferred factory method prefix) method. Controller reusability is not a goal of every project after all, and right now we're cheating anyway by having controller file reusability and stuffing extra Cordux stuff in extensions in another file; you could do that too!

My main concern is simplifying controller management; factory methods or builder objects are a valid choice for projects — I wouldn't say no to them! — but both seem only to move around but not simplify the total complexity of injecting a handler and setting up and assigning a context.

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

4 participants