willowtreeapps / cordux

https://willowtreeapps.github.io/cordux/
MIT License
61 stars 4 forks source link

Who should be responsible for building a ViewController? #14

Closed AndrewSB closed 8 years ago

AndrewSB commented 8 years ago

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 commented 8 years ago

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

aenewton commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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.