Refactor to increase Separation of Concerns

[astigsen]

This is really just to facilitate discussion, and not something that necessarily have to be merged.

While having a central store that all interactions go through have some benefits (and is how it is done in most uni-directional dataflow implementations), the way it is currently implemented makes it really hard to get any kind of Separation of Concerns.

Ideally each component (view) should only know about the subset of the Store that it is responsible for (which for the ViewController in this case is the list of Projects), and it should be able to work with them independently of which Store (if any) it comes from.

The refactorings I have done covers a few things to facilitate this:

• Methods have been moved to the Model classes, so that the View can operate directly on them, rather than having to go through the top-level Store object.
• Transactions are done at the View level, so that the View has the ability to group multiple operations together in a single transaction.
• The ViewController obtains the list of Projects from the Store at creation, and from that point on it only interacts with the list, it never needs to reference the Store after that.
• The View ask for notifications specifically from the Results it is holding, so that it won't end up updating on unrelated changes to other classes.

Things I would like to do better:

• Currently the ViewController obtains the list of Projects from a global Store object. Ideally, to have full Separation of Concerns and reusability, it should not have any dependency or knowledge of the Store at all, but that is pretty hard to do with Storyboards without getting into Segues and all.
• Our current syntax for doing writes via the objects themselves (using the object's own realm reference) is a bit cumbersome, but I can't see an easy way to simplify it without polluting our API with a lot of extra methods.

Any suggestions to how this could be done even better would be appreciated.

[samritchie]

I’m unsure about moving away from the global store - one of the primary drivers of the dispatcher architecture was handling cases where actions had broad-ranging effects on numerous components; the complexity of coordinating these via view model updates became intractable on larger applications.

I can appreciate the idea of running the transaction from the View, I’ve certainly run into problems in the past nesting the transaction at too low a level, however I would not expect the View to “group multiple operations together” - this doesn’t strike me as a view responsibility. It may be that you’d end up railroaded into this approach if the broader actions are broken down to separate model updates, but it still seems like it should be elsewhere.

Ultimately, this architecture seems to be pushing towards treating the data models as ViewModels - tightly coupled to the view/component model, using fine-grained notifications for updates. I think this a actually a very good approach for simple applications, but I suspect it will not scale to more complex apps, and TBH I don’t think we’d get away with calling it unidirectional.

I definitely think its worth continuing to explore - would be great to have a recommended architecture that makes the most of Realm’s strengths, and could have a positive impact on the API. (Projections? 🙂)

[AndyDentFree]

Is it necessary to have a global store if the methods are being added by extensions?

Can extensions be separated out into multiple Swift files so only some of them are visible to any given part of application code?

That would seem to narrow the information surface about the model but still keep the idea of having that code in something other than the View.

[astigsen]

I think this a actually a very good approach for simple applications, but I suspect it will not scale to more complex apps

In my experience it is exactly in complex apps that you need to be stringent about Separation of Concerns. For simple apps you can easily get away with global state and all views being tightly coupled to the Store, but as it grows you really need to isolate the individual components so that you can reason about their state individually.

one of the primary drivers of the dispatcher architecture was handling cases where actions had broad-ranging effects on numerous components;

Individual components should obviously have access to call whatever action they need, even if it means they need access to the full Store. But that should be a rarity. Most components only need to work with a subset of the full data model.

I don’t think we’d get away with calling it unidirectional.

How can you make it more unidirectional than Actions (which can come from any source, Views included) affecting the Model and Views reacting on the results?

[astigsen]

Is it necessary to have a global store if the methods are being added by extensions?

You can add methods to the Model via extensions. Both on the Realm itself, for global methods that affect everything, and on individual classes (like the method added on Results<Project> in this example).

[samritchie]

For simple apps you can easily get away with global state and all views being tightly coupled to the Store

The approach you’re describing tightly couples the view to the model, and heavily implies the component hierarchy will mirror the model hierarchy (or vice versa). My comment regarding “more complex apps” is referring to situations where this is not that case (i.e. views that don’t map easily to subsets of the data model) rather than a very large data model with clear, mechanical mappings to equivalent view components.

Maybe it’s worth trying to build something more complicated so we can work out how it’s likely to fall out in practice.

How can you make it more unidirectional than Actions (which can come from any source, Views included) affecting the Model and Views reacting on the results?

The Actions here have been reduced to nothing more than mutating methods on model objects. I can’t reconcile this with my understanding of Unidirectional Data Flow, sorry. It’s essentially identical to the old MVC architecture we used in Cocoa predating KVO & Bindings - view (or scripting) action methods set model properties, property setters manually trigger notifications, notifications trigger view updates.

[AndyDentFree]

Maybe it’s worth trying to build something more complicated so we can work out how it’s likely to fall out in practice.

That's what the private repos are for, so we can embarrass ourselves within the family ;-)

[samritchie]

The more I think about it, the more I suspect an MVVM approach is a better fit for Realm than full unidirectional. Building a ViewModel while sticking to the zero-copy philosophy is feasible, and should more cleanly deliver the desired separation of concerns. Consider something like:

  class AddBlogPostModel {
        let realm = try! Realm()

        var subject = ""
        var body = ""
        var urlSlug = ""
        var category: Category?
        var selectedTags: [Tag]()

        var isValid: Bool {
            return !subject.isEmpty 
                 && !body.isEmpty 
                 && realm.objects(Post).filter("urlSlug ==[c] '\(urlSlug)'").isEmpty
        }
        var allCategories: Results<Category> { return realm.objects(Category.self) }
        var activeTags: Results<Tag> { return realm.objects(Tag.self).filter("isDisabled = false") }
        var currentUser: User { return realm.objects(User.self).filter("isLoggedIn = true").first! }

        func save() throws {
              try realm.write {
                     let post = Post()
                     post.subject = subject
                     post.body = body
                     post.urlSlug = urlSlug
                     post.date = NSDate()
                     post.category = category
                     post.tags.appendContentsOf(selectedTags)
                     realm.addObject(post)
              }
        }
  }

An approach like this limits the scope of how much of the data model is available to the view and removes the need for global realm reference, but allows for more complex view/model relationships than a simple “This view does Posts”.

It definitely has some rough edges - e.g. notifications on calculated properties. However, I just thought I’d throw it out for further discussion.

[astigsen]

It’s essentially identical to the old MVC architecture we used in Cocoa predating KVO & Bindings

Yes, and that is pretty much the point. The original MVC architecture (from smalltalk) was a pure unidirectional dataflow architecture. It was Apple and Microsoft that later changed it to make the Controllers role a kind of middleware.

In the original MVC pattern, Controllers could only send events to the Model. See this diagram from wikipedia:

The current unidirectional dataflow craze is really about reinventing the original MVC model (this time with a functional touch).

I know that the MVVM pattern is really popular, but I still think that putting those kinds of service objects in between is an anti-pattern for any kind of coherent design. Martin Fowler said it better than I: http://www.martinfowler.com/bliki/AnemicDomainModel.html

The catch comes when you look at the behavior, and you realize that there is hardly any behavior on these objects, making them little more than bags of getters and setters. Indeed often these models come with design rules that say that you are not to put any domain logic in the the domain objects. Instead there are a set of service objects which capture all the domain logic. These services live on top of the domain model and use the domain model for data.

The fundamental horror of this anti-pattern is that it's so contrary to the basic idea of object-oriented design; which is to combine data and process together.

@cmelchior @jpsim or @bigfish24 might want to join in with their thoughts.

[samritchie]

Right, in the wikipedia diagram the ‘Controller’ is the equivalent of what we would call the ‘Store’ (or “Dispatcher’ if a separate one is employed) in Unidirectional terminology - it receives actions and is responsible for manipulating the model.

What you’re describing appears to require collapsing the controller & view responsibilities into a single class, and then making it ‘unidirectional’ by only performing view responsibilities in one section of the class, and only performing controller responsibilities in another section. This is very hard to explain and enforce, and EXTREMELY easy to unintentionally get wrong. As I’ve mentioned previously, at that point the pattern is not much more than a (non-obvious) convention, and it would be difficult for someone new to the concepts to be able to distinguish it from every other Realm Cocoa app.

I feel we may be talking at cross-purposes, and there might be better ways to address the issue. Is the main problem that the full (readable) realm is available to the view controller? There’s nothing in the design that requires this, we should be able to come up with an alternative.

Regarding MVVM - the main driver of this approach is to align the view & model so data binding can be more easily employed. Obviously it’s better if your model already matches your view, but generally larger applications will require re-using the same model from multiple, quite different views, and the likelihood of a good match decreases. Certainly I wouldn’t recommend adding that layer unless you had to.

[astigsen]

The point is more that all business logic is the provenance of the Model (in the original MVC pattern, the Controller should not contain any logic, it should just relay user actions to the Model).

A large part of the MVVM pattern is about pulling business logic out of the Model and into a service layer, which is what Fowler terms as an anti-pattern that goes against the basic idea of object-oriented programming.

That does not mean that it is not sometimes a good idea to add additional (non-persistent) classes or methods to the Model to handle concerns that cross-over between many types (or is relevant for so few views that you don't want to clutter the core Model classes with it). But he does say that these should be rare and very thin, and would still be part of the Model.

The key to the pattern is just that Views should only react on changes from the Model and not short-circuit it by updating directly from changes they do themselves. I agree that this is not enforced in any way, so it is really just a convention, but I would still say that it is a much better architecture as long as you follow it.

To make it much more clear what goes on, and at least somewhat enforce the pattern, it should really be best practice to do all changes in async transactions (which would make it clear that you cannot react to it right away, but have to wait to receive the resulting change events).

I feel we may be talking at cross-purposes, and there might be better ways to address the issue. Is the main problem that the full (readable) realm is available to the view controller? There’s nothing in the design that requires this, we should be able to come up with an alternative.

The main point is that it should be easy to work from a component model, where each View only know about of the subset of the Model that is responsible for.

I agree that you can still model Separation of Concerns with using a service layer, simply by splitting that layer into separate classes for each view/use-case. But I feel that it is just adding a lot of unneeded overhead. Our goal is to simplify app development.

[samritchie]

The key to the pattern is just that Views should only react on changes from the Model and not short-circuit it by updating directly from changes they do themselves. I agree that this is not enforced in any way, so it is really just a convention, but I would still say that it is a much better architecture as long as you follow it.

IMO it’s a stretch to label it as an architecture if there’s no actual structure supporting it. If a 'traditional' bidirectional architecture looks something like this:

 ______               _______
| View | ----------> | Model |
 ______  <----------  _______

Then the convention-based unidirectional flow looks like this:

 ______               _______
| View | ----------> | Model |
 ______  <----------  _______

Pretty much the only way of verifying one over the other is to audit every line of View code. If the goal is to separate the responsibilities of manipulating the model vs the responsibilities of updating the view, as a design principal you’d really want this reflected in your class responsibilities & collaborations.

To make it much more clear what goes on, and at least somewhat enforce the pattern, it should really be best practice to do all changes in async transactions (which would make it clear that you cannot react to it right away, but have to wait to receive the resulting change events).

It shouldn’t be relevant whether the updates are performed synchronously or not - personally, I think the existing design achieves this clarity better. The main rough edge, and I think your original objection, was the store dependency in the view component. What would you think of:

protocol Dispatcher {
    func doTheThing(foo: String)
}

extension Realm: Dispatcher {
...
}

// and then in the View

class View {
    // inject these somehow
    var dispatcher: Dispatcher!
    var myObjects: Results<MyClass>!

    func viewDidLoad() {
        token = myObjects.addNotificationBlock(self.render)
    }

    @IBAction func buttonTapped() {
        dispatcher.doTheThing(foo: "Thing")
    }
}

Dispatcher could be renamed to something that’s less-unidirectional-jargony if necessary, but the point is it’s a single class/protocol which is only responsible for translating or forwarding user actions into relevant model interactions. The View only has access to the specific subset of the model that’s been provided. However, I do think it’s important that Actions are global - I’ve been down the path before of trying to predefine which actions belong to which views, and it leads to a lot of unnecessary code churn.

A large part of the MVVM pattern is about pulling business logic out of the Model and into a service layer, which is what Fowler terms as an anti-pattern that goes against the basic idea of object-oriented programming.

I would argue that putting business logic in a ViewModel means you’re doing it wrong. Ultimately the ViewModel should just involve cleanly separating out the model-relevant parts of a view from the UI-relevant parts of the view, and it’s only necessary if there’s significant transformation/mapping required to get the model into a shape suitable for that screen.

BTW I’ve built my fair share of anaemic domain models, and I believe the causality is opposite - service layers end up being created to house logic, rather than logic incorrectly being put in service layers because they’re there. I’m happy to expand on this if you’re interested, but I’ll warn you it’s mostly unsubstantiated opinion 😉.

Our goal is to simplify app development.

I hate to say it, but building a (good) unidirectional data flow app using frameworks like UIKit and AppKit is most definitely NOT simple. If simplicity is the goal you’re much better off recommending ViewModels (or direct binding models to views for simple mappings). Otherwise you typically end up with enormous amounts of code laboriously diffing the current state of the view with the new state of the model (e.g. Adam’s RBQFRC code https://github.com/Roobiq/RBQFetchedResultsController/blob/master/RBQFetchedResultsController/Source/RBQFetchedResultsController.m). Ultimately this is why Facebook essentially wrapped the entirety of UIKit for React Native.