mutualmobile / VIPER-TODO

Example project architected using VIPER
MIT License
70 stars 15 forks source link

VIPER Issues #4

Open scottrhoyt opened 9 years ago

scottrhoyt commented 9 years ago

Sorry to repost here, but figured this would be better than the ObjCIO Github.

Here's a few of things I am having a hard time figuring out using VIPER architecture:

If I have an entity that is just a data structure, and I have a presenter which is responsible for formatting data, why would I not want to pass the entity from interactor to presenter? I find myself unnecessarily creating two separate data structures (one on the interactor/entity side and one on the presenter side) which the interactor does a 1 to 1 mapping on. Is this correct? Also, if you need to modify an entity based off of user interactions, how do you do this without violating layer boundaries and passing some reference to the entity to the presenter and/or view? The only other option seems to be an ugly remapping in the interactor.

How does data get passed from one module to the next? Let's say I have a module that lists categories and a module that lists items in a category. When a user selects a category, that selections needs to be passed from the category module to the item module so that the item module interactor knows which category to retrieve. Data should be owned by the interactors, yet the transition between modules happens at the presenter/wireframe level. It seems I am stuck passing data from one interactor to another through their respective presenters/wireframes which results in a lot of unnecessary code and seems to violate the single responsibility of the presenter only presenting data to a view and a wireframe just transitioning between views. There is also the layer boundary issue again because if you pass enough information for the second interactor to interact with the entity, then you are basically passing the entity... that or you have to do another ugly remapping the in second interactor. Should the interactors in different modules know about each other or communicate via a protocol? What's the best way to pass data between modules?

And what is the proper role of wireframes? Shouldn't they instantiate the entire stack (presenter, interactor, etc.) for their module and wire them up? Seems like this is mostly happening in the AppDependencies class which feels like it will get out of hand quickly. If wireframes own and configure the entire stack, then it feels like this might help a bit in passing information from one interactor to another, but would still violate boundaries or require interactors to communicate.

fatuhoku commented 9 years ago

+1 — I have a few ideas about the problems you're addressing.

For me, I think many developers including myself are left confused because the VIPER article is very vague about wireframes and relationships between modules — vitally, how they come together to form an application. A good post, but it leaves the community to fill the gaps. Your post is a good place to start.

The concept of 'wireframes' in the original VIPER post is just wrong. I would take what's been written about the routing section with a pinch of salt. The problem lies in that the wireframe class is acting as a a façade for the module's instantiation as well as acting as an endpoint (an internal interface, if you like) for the presenter to communicate with things outside of the module. There are two responsibilities here, and they must be separated.

What's more, there's no 'W' in VIPER! The intention was good but it's a bad execution. If you let wireframes talk to other wireframes directly (thereby creating a dependency), then you get something really anti-modular: a situation like this:

screen shot 2015-02-02 at 12 19 48

I like thinking of VIPER modules be the 'dots' ONLY. Routing and navigation (i.e. the wireframe) is what ties the dots together (embed segues / container view controllers count as navigation I'm having trouble with embed segues. See below). They cannot belong in the modules themselves, otherwise your modules won't be reusable.

The key is to recognise that modules live within the context of an application and therefore we can split the wireframe's internal responsibilities from its external ones. I call the internal responsibility the 'wireframe' in this case, and the external one 'a router'.

A few words on using segues. I found that you can just about have data passed around even while using segues, but the ways you can do this is limited. You'll have to

  1. from SourcePresenter, call SourceWireframe protocol method [self.wireframe setDataForNextModule:data]. The implementing Router object holds on to this data temporarily.
  2. instantiate TargetPresenter, injecting the aforementioned Router instance as its TargetWireframe
  3. access the previously stored value via TargetWireframe protocol method [self.wireframe dataToDisplay] to effectively pull down the data from the wireframe.

Admittedly, this is not particularly elegant, but it does work, and neither module needs to know about the other. Router, however, knows about both.

More notes:

fatuhoku commented 9 years ago

The original post also says that you shouldn't pass entities from Interactor to Presenter. This follows the Clean architecture dependency rules and is probably a good idea. Though some have interpreted VIPER differently, like [over at Brigade]:

They [entities] embody some type of data and act as the “payload” that gets passed around between the other classes. For example, the data manager returns an entity to the interactor, which returns an entity to the presenter, which then uses that entity to tell the view what it should display.

So by all means do pass entities to the presenter if it's quicker and easier to do so. But the separation should exist.

sebastianwr commented 9 years ago

This would be so much easier if someone official actually commented on that issue. Which leaves me to think that they didn't really solve it themselves.

scottrhoyt commented 9 years ago

@fatuhoku Thanks for the thoughtful comments. I think we have arrived at exactly the same point as to what needs to be changed to make VIPER workable on an app scale. Unfortunately, I dove into the architecture head first for a green pasture project and now find myself doing a lot of refactoring to undo my naive application of the architecture. At the end of the day, it seems that the key innovative piece of VIPER over standard MVC is the presenter/router layer and extracting some view logic and navigation out of the view controller. In this way, it has a lot of similarities to MVVM or the Presentation Model pattern.

@sebastianwr I agree that the lack of official commentary on the issue is concerning. An idea, even a good one, without proper community support tends to stay just an idea.

sebastianwr commented 9 years ago

What I find is that a very reasonable and efficient approach is to have a class that does all the "dirty work" accompanying each view controller. This class is the entry point to the view (as in the Wireframe) and gives the VC his data (like the Interactor and Presenter). That means the view is decoupled from the other implementations and concerns (if you like you can convert the NSManagedObjects passed to the VC to a "stupid" POOO... POObjCO... POSO... whatever) to even more decouple the view from the data.

Each "worker" or "service" has a designated method for the entry point that is called by other workers.

Also, when using MagicalRecord or something you can skip the data store and still have decoupling so that the VCs don't touch the database layer.

So basically, I combine Wireframe/Interactor/Presenter and maybe sprinkle a View-Model onto it, if it's necessary to decouple. And of course, if anything is worth having its own class (because it's an own concern) I split it from the "service" class to a separate class.

Key principle as usual is dependency injection and testability. I think that's also the basic message behind VIPER (although DI is terribly implemented in the example).

samwyndham commented 9 years ago

In this sample application data manager classes are contained within each module. It seems that works in this case but in other applications data managers might need to be shared between modules. Am I misunderstanding the role of data managers?

scottrhoyt commented 9 years ago

That's less answered by the requirements of that architecture and more by the requirements of optimization/code readability and maintainability. There are good reasons to go both routes. For example:

If your data manager is creating a large memory footprint, is needed for a prolonged period, requires a good amount of time to establish a session, and might have session dependence (e.g. a network connection to an SQL database using a database driver) you might want to share the object that owns that session.

If your data manager is creating a smaller memory footprint, is only needed sporadically, is fast to establish a session, and has no session dependence (e.g. retrieving an object from a memory or disk-based cache) you might want to have each module with its own data manager that is only kept alive when needed and separates data responsibilities on a per-module basis.

The common case of retrieving objects from a HTTP-based RESTful service that might require authentication falls somewhere in between. Ultimately this will also depend on how you break out data responsibilities between an interactor and a data manager and also whether or not your data manager is doing the heavy lifting or is an adapter to another service that might already be shared (e.g. core data, icloud or a BaaS).

I find myself thinking a lot about these sort of (potentially unnecessary) optimizations. To axioms I wind up proving true time and time again are Keep It Simple Stupid, and premature optimization is the root of all evil.

On Thu, Feb 5, 2015 at 2:21 AM, Sam Wyndham notifications@github.com wrote:

In this sample application data manager classes are contained within each module. It seems that works in this case but in other applications data managers might need to be shared between modules. Am I misunderstanding the role of data managers?

— Reply to this email directly or view it on GitHub https://github.com/mutualmobile/VIPER-TODO/issues/4#issuecomment-73024148 .

[image: Wild Onion Labs] http://wildonion.io/

Scott Hoyt / CoFounder 708.704.6804/ scott@wildonion.io

Wild Onion Labs http://wildonion.io

[image: Facebook] https://www.facebook.com/scotthoyt [image: Twitter] https://twitter.com/scotthoyt [image: Linkedin] https://www.linkedin.com/in/scottrhoyt

This e-mail message may contain confidential or legally privileged information and is intended only for the use of the intended recipient(s). Any unauthorized disclosure, dissemination, distribution, copying or the taking of any action in reliance on the information herein is prohibited. E-mails are not secure and cannot be guaranteed to be error free as they can be intercepted, amended, or contain viruses. Anyone who communicates with us by e-mail is deemed to have accepted these risks. Wild Onion Labs LLC is not responsible for errors or omissions in this message and denies any responsibility for any damage arising from the use of e-mail. Any opinion and other statement contained in this message and any attachment are solely those of the author and do not necessarily represent those of the company.

samwyndham commented 9 years ago

@scottrhoyt Thanks for the comments. I've been messing around with viper a little and watching some of Robert C. Martin's videos on clean architecture which help illustrate the architecture a little further. I definitely see where you are coming from about KISS. It seems quite easy to over engineer.

fatuhoku commented 9 years ago

@scottrhoyt Yes — in practise I have also found the Interactor and DataManager can easily be excess baggage if all your VIPER module does is to display data. i.e. not every module has business logic or needs to fetch data.

I ended up creating 'Presentation-only' VIPER modules, which consists only of the View+ViewController, Presenter and Wireframe.

torstenlehmann commented 9 years ago

I like this discussion a lot. There is not much discussion at all about VIPER, so I ask my questions here.

Is there a good example of reusing a VIPER module? I think of a module with View, Presenter, Interactor and DataManager. How could this be reused in another app? It's tied to application and business logic through bundling all layers together. Even a reuse in the same app is quite unlikely or not? A good example would be helpful.

The whole concept of Clean Architecture is layer independence through following The Dependency Rule.

@fatuhoku I like your advice of separating the wireframe. But I have a small problem with this one:

The presenter is programmed against this interface. e.g. the presenter might send the wireframe: [self.wireframe showFooBarWithData:data fromViewController:vc].

I think this violates layer independence because the presenter has to know a framework class (a kind of UIViewController).

So do we really need module independence or do we need layer independence? Or do we need both? How can we accomplish both with an external router in place? I can't figure out the best way to do this. I'm searching for a way to accomplish an ui framework independent router working in the same abstraction layer as the presenter.

Thanks a lot to all of you in advance

torstenlehmann commented 9 years ago

Ok. So module independence is not intended for module reuse instead it is targeting on organisational concerns in the app itself.

There are a few benefits to designing your app as a set of modules. One is that modules can have very clear and well-defined interfaces, as well as be independent of other modules. This makes it much easier to add/remove features, or to change the way your interface presents various modules to the user.

scottrhoyt commented 9 years ago

@torstenlehmann I think reusability is still about the same as a strictly MVC approach. The problem VIPER intends to sole is not one of reusability as much as testability. The main difference between VIPER and MVC is that the navigation/UI logic/data formatting that is normally thrown into a view controller in MVC is now pulled out into a router/presenter/presenter, respectively, in VIPER.

This makes the code easier to test, read, and maintain. But just as a view controller in MVC, there is still a high degree of coupling between a presenter and router and a specific "screen", "view", or module in VIPER. The likelihood of reusing these components is dependent on how often you reuse the same UI paradigm in your app. The equivalent of the MVC model layer--interactor, entities, data manager--should be just as reusable as model components in MVC and perhaps more so because of a more explicit separation of different components in the model layer.

So, in summary, module reuse will probably be low. Layers, on the other hand, become more reusable as you move lower from UI to data/model.

alessandroorru commented 9 years ago

I found this discussion really helpful, however I have a question.

In an app I'm developing, I have a conditional navigation at app startup. In case the app is first launched, I have to show a walkthrough, otherwise I will land directly on one of two screens: a password view controller, or the main interface. The view controller that should be shown is inferred by a couple of values stored on the device (user defaults, keychain and so on).

The question is: where should I implement all this logic? It doesn't seem to belong to the presenter (or to the interactor), since I don't have a single one. It seems to fit well inside the wireframe, but I'm not sure, since it seems to break SRP (it would implement both the animation logic and the what-to-show-next logic, based on stored data). Or would be better to simply create a companion object that mimics the interactor purpose but coupled with the wireframe instead of the presenter?

Ideas are welcome :)

fatuhoku commented 9 years ago

@alessandroorru really good questions that I had been wondering for a while too. I like your thinking starting from ground up. I ran into similar issues and have some thoughts.

People using VIPER seem to have found the concept of a 'root' module useful. Check out https://github.com/mutualmobile/VIPER-TODO/blob/master/VIPER%20TODO/Classes/Common/View/VTDRootWireframe.m for example.

Work with more than one root view controller

In an app I'm developing, I have a conditional navigation at app startup. In case the app is first launched, I have to show a walkthrough, otherwise I will land directly on one of two screens: a password view controller, or the main interface.

The first thing to note is that each of:

... are all rightfully top-level, or root view controllers of your application. i.e., they have a right to be shown directly in your UIWindow, free from other storyboard / segue animated:NO trickery — because let's admit it, those practises have always been a hack, and yet it's prevalent because it's the easiest.

Further, let's say, each of walkthrough, password and main interface were implemented as VIPER modules.

Then what's going to orchestrate these three different modules? Well, the question becomes... How can you even manage root view controllers without resorting to the storyboard (and thus being tied to one 'decision making' ViewController...)?

Well, I found https://github.com/timshadel/TSAnimatedRootViewSwitcheroo. It helps you switch between roots. So. Cool.

Let's elaborate a little bit.

Root VIPER Wireframe provides the Window and manages navigation between root view controllers

Starting with a blank slate, you have an application with storyboards disabled, so when you Cmd+R, all you see is black.

  1. AppDelegate obtains a RootWireframe (either create it or have it injected). AppDelegate performs some basic initialization tasks and then asks the wireframe for a window to display (see my gist here https://gist.github.com/fatuhoku/d4f39cb6e7075c3a70d1). This window is, from now on, managed by your application.
  2. RootWireframe needs to decide: do I show the login screen, the walkthrough, or the main interface right now? — actually, this is essentially a navigation decision. This is normally driven by presenters. So okay, let's make (or inject) a RootPresenter and ask it which interface should be shown.
  3. RootPresenter might need additional information like "Is the user signed in?" or "Is this the user's first launch?" to decide whether it should show the walkthrough, signup or main interface screens. This information might come from your database. In any case, RootPresenter goes and asks a RootInteractor about this.
  4. RootInteractor comes back and says that the user is signed in already. Great. RootPresenter decides to tell RootWireframe that it should show the main interface.
  5. RootWireframe goes off and tells the MainInterfaceWireframe to initialize itself, and asks it for a UIViewController to display. The returned UIViewController is assigned onto window.rootViewController. Alternatively wrap a TSAnimatedRootViewSwitcheroo around the returned view controller before setting (see below...)
  6. AppDelegate does [window makeKeyAndVisible], and stuff appears.

When a user decides to log out...

  1. MainInterfaceWireframe tells the RootWireframe that the sign up screen should be shown again.
  2. RootWireframe asks SignUpWireframe for a UIViewController, and then assigns it directly onto window.rootViewController. Alternatively, if using TSAnimatedRootViewSwitcheroo then you can switch to the main interface with an animated transition of your choice with relative ease.
fatuhoku commented 9 years ago

@torstenlehmann about layer independence, you are right — Presenters really shouldn't know anything about UIViewControllers.

I have been using storyboards and really wanted to keep using segues, which drove me to pass UIViewController references around. I can see clearly now why Wireframe objects must own UIViewControllers and why segues sort of introduce bad architecture in this respect.

alessandroorru commented 9 years ago

@fatuhoku thank you for your helpful answer.

I was already ending up with a solution similar to the one you proposed, but I was using a sort of interactor directly linked to the wireframe. My mistake was that I was missing the concept that is the presenter that leads the navigation, while the wireframe/router duty is to just perform the requested navigation.

I have another question for you. I'm trying to integrate a DI framework (typhoon) in this project, and I have some doubts about what should be injected. I surely would construct with DI the object graph of wireframe, presenter and interactor, but what about view controllers? Do you think it is ok to instantiate them (and link to the presenter through protocols) directly in the wireframe, or should they be injected too? I use storyboard only as containers, so I instantiate VC by code through storyboard IDs.

fatuhoku commented 9 years ago

@alessandroorru I'm exploring the topic as well myself — so I haven't really figured out the best formula. I personally quite like the idea of using Typhoon it because it makes your components feel so much more tangible: if you forgot to initialise something, it barfs at you. Also by having to group your dependencies up it helps you think about application architecture much more clearly.

I've found Typhoon's storyboard support to be quite solid, and so you could probably have them injected into wireframes as properties! If you let wireframes reference other wireframes like in the original post, you'll have to have other wireframes injected as well so that the view controllers are automatically loaded too.

Again, when doing that, you can't really use storyboard segues. Wireframes must hold references to UIViewControllers. When you use segues, UIKit instantiates your view controllers and so it's not so easy to get hold of references of them in your wireframe.

torstenlehmann commented 9 years ago

My observation is that this discussion on issue #4 is going to grow. :D

Any recommendations for a better communication channel? What about a VIPER discussion repo? Or architecture case studies in general.

I have some reservations regarding too much flexible architecture upfront. Like premature optimization is the root of all evil, I tend to say that premature flexibility could be also problematic.

So I like this discussion a lot to review my own approaches.

daria-kopaliani commented 9 years ago

I have a question about communication between modules, I think @fatuhoku was discussing something similar here earlier, so I hope you could give me some advice. The use case I’m working on is dead simple - I have a list of categories, user selects one of it and I push a view controller displaying category contents. The first question is wether this should be one module or two modules. I think that it should be 2 modules (list module and category module) so the second question is how I should pass an entity between modules? What kind of entity should it be? Probably not a NSManagedObject, may be entity created by the interactor from NSManagedObject? I guess not a struct that a presenter operates with? Although it was a presenter that got notified that a particular category was selected, so should it ask interactor for its entity and then pass it to wireframe so that it could pass it to the next module? Is it okay for a wireframe to know about entities at all? It seems like this is the easiest problem and I’m the only one missing it, so I would appreciate any thoughts on this.

alessandroorru commented 9 years ago

@daria-kopaliani My 2 cents:

  1. Surely you have to abstract away all the persistence layer. Nobody should know about CoreData but one or many data stores, and interactors should only talk with data stores. NSManagedObjects should be converted to application models by the data stores, possibly with structs if you are using swift in order to keep their immutability. Keep in mind that models should not have any business logic inside, just variables and convenience methods.
  2. Interfaces between interactor and data stores should be implemented through protocols. In this way, together with the previous point, if one day you decide to totally or partially remove CoreData, you should only switch to different data stores (conforming to the same protocols) and you don't have to change all the app. Moreover, writing tests, you won't have to relay on fixtures on CoreData or a bunch of mocks! You just need a different data store that conforms to the same protocol that the interactor expects and that returns the test data. Anyway read again the VIPER guide, it describes this process a lot better than me.
  3. Following VIPER original guidelines, the interactor should not pass models to the presenter, but should convert them to a minimal aggregation of information that the presenter need. Anyway, IMHO, this is quite limiting and a bit redundant. Many times I have found myself writing another struct that just contains the same data. So I think that it's not worthy to introduce another layer here, and I think it is ok to pass around models, as long as all the presenter do with them is just consume their data to update the UI.
  4. About passing around data, I have some doubts too, maybe @fatuhoku has better ideas. I think it is ok to pass the data to the wireframe as long as its purpose is just configure the next view, for example I think it is ok if you have something like this:
func presentDetailInterfaceForCategory(category: Category) {
       let viewController = CategoryDetailVC()
       viewController.category = category
       // present view controller ...
}
fatuhoku commented 9 years ago

@torstenlehmann hmmm. I could make my Presenter not have a direct reference to the UIViewController by using delegates, but my Presenter acts as a collection view data source, and therefore knows about UIKit. Perhaps it's time to move all of my UICollectionViewCell logic back into the ViewController, and have it mediate calls to the presenter. Hmmm.

torstenlehmann commented 9 years ago

@fatuhoku Yeah. Teasing details. At first I think you have to decide if it's really worth it to completely isolate the UIKit stuff. As I mentioned maybe it's too cumbersome to do this and you don't have enough resources (time, money, co-workers) to make it this clean. That's a point where I'm searching for the sweet spot and guidelines.

But if you want to divide the two layers completely, you can try the following ways maybe. It all depends on what you want to archieve. Is your view only asking for data? Should your view being updated by external events?

  1. One View Controller meets One Presenter (updating entire list in one go) Put all you CollectionViewDelegate and DataSource methods to the viewController. The presenter should not know, how the view gets its work done. A good test is to imagine how it would be if you switch your collection view with a webView. This must not change your presenter. The pattern now is to pipe all this data through one method. Define a method showCollectionItems on the viewController and add this method to the view protocol which the viewController implements. In this method reload the collectionView with the items you are passing as argument to showCollectionItems. This updates your entire list in one go. But in some cases you have to pass a really big list. And updating individual cells is also impossible this way.
  2. One View Controller meets Multiple Presenters (updating each cell) The pattern here is to pass multiple mini presenters from your main presenter to the viewController. This gives you fine grained control over which cell to update instead of getData-mapData-pushData in your main presenter. With regards to layer independence its okay that the view has to handle the mini presenters. This pattern is a step into the direction of view binding. I don't have much experience with this. I thought of this last time I build a list with downloads and each item should show its progress. That's quite annoying to do with one presenter only, because the main view has to handle each update on any cell. I don't know how good ReactiveCocoa fits in here. So the main presenter pushes the mini presenters to the view if the collection of data changes. The mini presenter updates a cell if the cell content for this data item changes. This sounds for abstraction and a framework in the long run questioning the clean architecture way. :D
  3. Your idea to let the presenter implement the delegate methods is also a nice one I did not think of. Sure, you have to create separate methods which the delegate methods can call on the presenter. This is kind of a pulling concept and is nice for big data sets. The collection view can even ask the main presenter for a range of items or mini presenters (a kind of window on the whole data set). This makes it possible to execute batches of fetches on the database or webservice. You can also push a kind of virtual proxy to the view which encapsulates this on demand methods. Its kind of a NSFetchedResultsController but independent of the data layer.

Yes. Maybe this combi is the most powerful. I will consider this.

torstenlehmann commented 9 years ago

@alessandroorru

Following VIPER original guidelines, the interactor should not pass models to the presenter, but should convert them to a minimal aggregation of information that the presenter need.

I have my problems with this, because it violates layer independence. The interactor should do nothing what the presenter is there for. You can increase reusability of the interactor if the presenter maps the items from the interactor to view items (if this is really needed).

I like your common sense to use the entities if they are the same for the view. All what this shows to me is that apps often aren't business level systems. They can evolve to that later. Immutable data is no problem at all and a presenter that uses entities still follows the dependency rule to depend on inner layers.

Edit: Ok. I watched a Clean Architecture video again and can now better understand how you meant this. Robert C. Martin calls this "minimal aggregation of information" a response model. I think he wants to avoid passing an entity with domain logic to an outer layer. For data only entities this still might be no problem. An important point for me is that the interactor returns this response model regardless of the "delivery mechanism". I could be a presenter, web socket (not usual) or anything else. But I can't help watching this video and thinking "wow this produces bloated code", at least for apps mostly presenting data.

torstenlehmann commented 9 years ago

@daria-kopaliani I would pass the minimal amount of data needed to do the work in the category module. Most of the time this is a categoryID (but not a rowId of a database, a database independent id, maybe an id coming from your webservice, a system wide id if you have a webservice and app combination). The categoryPresenter asks a categoryInteractor which asks a dataManager which asks a concrete dataStore for this category with this id. Yeah a lot of layers which are often not needed. This is caused by the simplicity of getting data. It's kind of the simplest use case imaginable. You have no filtering, searching, calculating in there. That's what I mean by not using too flexible architecture upfront. View, presenter and dataManager and dataStore is enough flexibility for this case. And you can reuse the dataManager and dataStore on many other modules in this app. If you have more logic than getting data introduce an interactor between presenter and dataManager. But even if you replace your local database with a webservice api you only need to introduce a second concrete dataManager. I tend to say that you can even omit the presenter if fetching and presenting this category is the only thing that you are going to do. It makes no difference if you build this new layer on demand at a later time or upfront. But you take the risk that this flexibility is not needed later. You can argue that you want to test the presenter. But testing this glue code has no real value. Testing domain logic has real value. Testing mapping code in the presenter has value (yes I have thrown in 4 bugs last time I wanted to put a forename, surname and a separator together: null, "", "someName" on both fields, and you have 9 possible states to handle – doubled by the separator included or not, results in 18 states). That's what tests are for – to narrow down this complexity.

So like alessandroorru said. First step wrap core data and use plain objects. Then choose alessandroorru's advice or my advice. I think they both work quite well, because it's not a complex thing at all unless you have any external events which should update the category view (e.g. some new items arrived in the database and should be shown). VIPER (and clean architecture) is a pattern for managing complexity. I have written an app recently and it felt quite bloated because its such a simple app and i used VIPER. That's why I'm searching for this process to introduce new layers on demand. I liked the book Refactoring by Martin Fowler a lot. But it aims on refactoring objects not layers. So I appreciate any advices.

alessandroorru commented 9 years ago

What about NSFetchedResultsController? They have a great value through index path based updates, but their place should be the data manager layer, before the interactor, abstracting away the CoreData dependency. This becomes a VIPER limit, if I cannot reproduce the same behavior all the way up to the presenter.

What are your thoughts or ideas about it?

torstenlehmann commented 9 years ago

@alessandroorru my thoughts on this

What are the requirements?

  1. On demand fetching of data to save memory
  2. Using strictly separated layers
  3. anymore?

The important point to get is: If you have memory constraints you have to pull the data on demand. This is what NSFetchedResultsController basically does. Don't push a big list of data on viewDidLoad.

A shortcoming of Uncle Bobs explanation on clean architecture (as far as I understand) is this entity fetching behaviour. He often speaks about systems where every object can practically fit into RAM. For him a database is only the persisting mechanism. As you can see Core Data and SQLite is a little bit more. It's also a search and filter mechanism that is quite fast and memory efficient. If Core Data would only be the persisting mechanism you should fetch everything into RAM and do searching and filtering in RAM with predicates on collections. For big data sets this is not realistic (or am I wrong?).

So lets add a third requirement:

  1. We want fast and memory efficient searching and filtering on large data sets.

Can we build this on our own? I don't think so. We have to made a decision now. I don't know of any library that does this for us in memory. But Core Data solved our problem quite good. This presents an important insight. Frameworks are here to help us! Sometimes you can write things on your own to not depend on a framework and sometimes you want to safe time or are overchallenged. For my own skills writing the SQLite database would be a bit hard. So thx to the SQLite-Team and thanks to Apple. This is how life works. If you need help, you depend on someone else.

Now we want to use this searching and filtering capabilities of the database. The definition of fetching now changes from loading all data into RAM to loading searched and filtered data into RAM. This leads us to the following solution:

Use the NSFetchedResultsController only in the Core data layer. That way you need a dataManager per fetchRequest because the NSFetchRequest belongs to Core Data. You can build a flexible dataManager taking every configured fetchedResultsController (configured with a fetchRequest) as a parameter in the constructor. You can ask you presenter for viewModelAtIndexPath: which asks an interactor for resultModelAtIndexPath: which asks the dataManager for entityAtIndexPath:. So it all stays the same but you are mapping only one entity and you are doing this on demand. If this mapping is worth while depends on the complexity of the system. Passing the indexPath trough all layers is ok, because it's a complex version of a position.

alessandroorru commented 9 years ago

@torstenlehmann Thanks for your answer, anyway I think that the biggest problem is the other way around. Think about the fetched results controller delegate: its methods are called whenever a new object is added/modified/deleted from the current result set. This is a behavior that I want to preserve: I want to be able to get notified at presenter level about new objects that has been updated and at which index path.

I think that your idea of a dataManager per fetch request is a way to go, I have to give it a thought ;)

torstenlehmann commented 9 years ago

@alessandroorru If it's not about persistence at all you can use KVO on arrays to build your own EntityListObserver in your use case layer.

If you want to preserve layer independence every delegate call has an equivalent delegate call in every layer if the event is originating from inner layers. Is this good design? Hard to see why this should be. The use of an NSFetchedResultsController means that you are only presenting data from a database. There is no business logic than presenting data. I would wrap Core Data, because I don't like the API that much and call this wrapping API from the presentation layer (presenter/view). No interactors needed in this case.

shaneowens commented 8 years ago

@fatuhoku The gist you reference above i.e. "https://gist.github.com/fatuhoku/d4f39cb6e7075c3a70d1" is no longer available. Any chance you still have it around - would love to read it.

Many Thanks.

shaneowens commented 8 years ago

If you have an application that talks to both a web api and a database how many dataManagers should you have one, two or three?

Case: a) dataManager b) APIDataManager and LocalDataManager c) dataManager, APIDataManager and LocalDataManager

Where in

a) The interactor talks to a single dataManager that talks to any services you may have (remote or local). b) The interactor knows the difference between local and remote information - and calls either the APIDataManager or the LocalDataManager, which talk to remote and local services respectively. c) The interactor on talks to a general dataManager, the general dataManager then talks to the APIDataManager and LocalDataManager

"But even if you replace your local database with a webservice api you only need to introduce a second concrete dataManager." - @torstenlehmann. Any thoughts?

jeffgilbert commented 8 years ago

@sao101 It depends on where the abstraction lies within your app, that is distinguishing what you do from how you do it. Who is defining that there are two different data stores?

If local and remote data stores are part of the problem domain itself (e.g. sometimes the problem requires fetching remote data, and other times it requires fetching local data), it is sensible for the interactor to know about the two different data stores.

If the Interactor only cares about what data is requested, but it does not care about how the data is retrieved, it would make sense for a single data manager to make the determination of which data source to use.

There are two different roles at play here—the business designer, and the data designer. The interactor is responsible for satisfying the needs of the business designer, i.e. the business logic, problem domain, etc. The data layer is responsible for satisfying the needs of the data designer, i.e. the server team, IT team, database team, etc.

Who is likely to change where you look to retrieve data, the business designer, or the data designer? The answer to that question will guide you to which class owns that responsibility.

torstenlehmann commented 8 years ago

@sao101 "But even if you replace your local database with a webservice api you only need to introduce a second concrete dataManager."

What I'm describing here is an example of programming against an interface/protocol not implementation, so you can switch out data managers on demand.

Your question is perfectly answered by jeff. It's a question of making the data retrieval process visible to the interactor or not. Try to write the interfaces and objects for both cases. What is your intuition? What feels better? Can you find facts why this feels better? What is the most likely change that can occur to the requirements? Which things must be rewritten at that point?

I don't use VIPER upfront. I write, grow/rewrite and refactor until the modules fall into place. Software requirements are changing constantly. For me it's not reasonable to have one pattern for all of them. VIPER reminds me that I ask this question more often: Can I separate concerns better here?

shaneowens commented 8 years ago

@jeffgilbert @torstenlehmann - Thank you very much for your explanations and feedback guys. I really appreciate it.

egze commented 8 years ago

@fatuhoku Hi. Your gist https://gist.github.com/fatuhoku/d4f39cb6e7075c3a70d1 is not available anymore. Can you please post it again?

fatuhoku commented 8 years ago

@egze Sorry I think I've removed it! Might not be able to recover that snippet now. :/

Vespen commented 8 years ago

Hi, I am using VIPER for application written in Swift. I have a situation when app contains a lot of actions based on dialogues with reusable views. So I decided to split each dialog to the following modules:

  1. Dialog - dialog module that is responsible for navigation between dialog screens and for updating state of the dialog target.
  2. Dialog Screen - dialog screen module that is responsible for displaying screen UI, fetching data from data source. It does not know anything about next dialog screen and about dialog target, but it can notify listeners through module output protocol about changes.

Dialog module listens screen modules for changes and decides to perform routing to the next screen. With this approach I can reuse any view of the dialog (and I think it is pretty SOLID solution), but is this a good approach? It seemed to me that VIPER article implies that module should know how to advance to the next module.

Also I have decided to use Swift module (separate build target) for every VIPER module and I have a question about it. What is the best practice about providing entry point to VIPER module. Currently I use module function that creates router and presents module.

fatuhoku commented 8 years ago

Hmmm. I think 'VIPER' as a concept is given more credit than it deserves. As we've discussed there, there's components whose responsibility are half-baked.

I recently saw this video and understood where VIPER came from.

Then I found this: http://clean-swift.com/clean-swift-ios-architecture/. A certain Raymond Law wrote up how he thought of applying Clean to Swift. I like that it really gets you right back to the first principles again... to realise that whatever you're doing with your code, whatever you want to call it, whether it be VIPER, MVVM or MVC.

You really want to be pushing your implementation details to the edges. Protect the details of your business logic.

fabb commented 7 years ago

Who should own whom?

I've seen in some VIPER demo, that no weak references are used at all, probably resulting in massive memory leaks. In another blog I read that the wireframe should own the viewcontroller, which makes sense hierarchically, but on the other hand a navigationcontroller will retain its viewcontrollers too. So when our wireframe retains the view, we need a rootwireframe that manages the navigationcontroller, and watches its delegate calls, as it might pop the viewcontroller due to the back button, and the rootwireframe would need to release the child wireframe too. For simplicity's sake, in our case we decided that the viewcontroller owns the presenter, and the presenter owns both the wireframe and interactor, but it kind of feels wrong.

I'd be interested on other takes on this.

JeffGilbert-MM commented 7 years ago

If there are no weak references between view–presenter–interactor, that is a bug. Typically, no-one should have a strong reference to the view controller, because we can depend on iOS to control the lifetime of the view controllers.

Assuming the view controller is properly managed by iOS, you want strong references to flow from the view controller to its dependencies: view -> presenter, and presenter -> interactor. You would want weak references flowing back to the view controller: interactor -> presenter, and presenter -> view. This also makes sense because the weak references can be considered channels to communicate results. As far as dependency management, I like strong references to objects that you need to perform your work, and weak references to communicate results.

fabb commented 7 years ago

That's like we have it now + the presenter has a strong reference to the wireframe, same for you?

JeffGilbert-MM commented 7 years ago

My current approach is to have a factory and wireframe for each different lifetime within the app. e.g. for objects with the lifetime of the app, I have an AppFactory, and an AppWireframe; for objects with the lifetime of a modal transaction (i.e. modal view), I have a ModalFactory, ModalWireframe, etc. The lifetimes are usually associated with some containing view in iOS. e.g. app life objects are associated with the main window, modal lifetimes are associated with a modal view, etc.

I use objc_setAssociatedObject to associate the wireframe with its containing view. e.g. AppWireframe is an associated object of the main UIWindow, a ModalWireframe is associated with its modal view, etc. This way, when the modal view is dismissed, the wireframe, etc. is released.

Doing the above, I use a weak reference from the presenter to its wireframe, since a wireframe will have a longer lifetime than a particular presenter.

But, if the wireframe is already going to outlive the presenter, it's probably ok to have a strong reference to the wireframe :-)

scottrhoyt commented 7 years ago

Nice architecture @JeffGilbert-MM! Though the objc_setAssociatedObject makes it a little less applicable to Swift.

I have to ask the question though. How do you pass data between views? Is it through all the presenters and wireframes?

V1 -> P1 -> W1 -> W2 -> P2 -> V2

Or is there another channel created to pass data directly from presenter to presenter or view to view?

I have done it the former way. I have even had cases where the data had to essentially go from interactor to interactor through presenters and wireframes. It certainly adds a lot of boilerplate

JeffGilbert-MM commented 7 years ago

Thanks!

You can do object association from Swift. I wrote this code:

extension NSObject
{
    /**
     controlLifetimeOf(navigator:) allows the lifetime of a navigator to match the lifetime of another object.
     This is useful to avoid retain cycles between a view controller, navigator, and presenter.

     - parameter navigator: the navigator whose lifetime you want to match to the target of the function.
     */
    func controlLifetimeOf(navigator: AnyObject)
    {
        objc_setAssociatedObject(self, &AssociatedKeys.Navigator, navigator, .OBJC_ASSOCIATION_RETAIN_NONATOMIC)
    }

    fileprivate struct AssociatedKeys
    {
        static var Navigator = "navigator"
    }
}

PS I've recently taken to referring to wireframes as navigators, as the navigator/wireframe is only responsible for knowing how to navigate. It is up to other objects to know when, and where to navigate.

Depending on what needs to be passed, the presenter can pass data to it's wireframe. Other times, it's appropriate for the data layer to implicitly pass data between interactors. e.g. one interactor can write data into its data store, and the next interactor can read data from the data store.

scottrhoyt commented 7 years ago

That would definitely work for Swift classes that inherit from NSObject. We like to stay in pure Swift land as much as possible though. It also forces your hand on using reference semantics, and I think value semantics have a place particularly in I, P, E, and W layers to discourage creating shared global state and encourage unidirectional data flow.

The direct interactor to interactor passing of data is something i have tried, but not using a data store to accomplish it--again trying to reduce global state as much as possible. Instead I used a protocol that allowed the two to communicate directly. But that basically boils down to passing the data:

I1 -> P1 -> W1 -> W2 -> P2 -> I2

because you have to pass the protocol object to the other interactor and wireframes don't have direct access to interactors.

fabb commented 7 years ago

We discarded using structs for I, P, W, as it prohibits to have weak references to them, and we consider every object having a property of some class object to be a class itself, as it encapsulates changing state. http://faq.sealedabstract.com/structs_or_classes/

fabb commented 7 years ago

@JeffGilbert-MM: In an experiment I tried the Wireframe owning the ViewController. Works very nicely, and makes sense IMHO, as the Wireframe manages the ViewController, not the other way around. We have to track state very closely though, so the Wireframes (and thus their ViewControllers) get deallocated properly when the system executes a navigation action beside Wireframes (e.g. using the back button or back swipe). Here is the example project: https://github.com/willhaben/Wireframes

Ronaldoh1 commented 7 years ago

@JeffGilbert-MM your explanation about strong vs weak helped me a lot. Thanks you

@scottrhoyt I'm also still confused about passing data between views (modules for that matter). I guess I'm still confused about passing objects (entities) between modules. If the presenter or the router don't know about the entity. I'm not sure if the confusion comes from the VIPER diagrams on the internet. Why can't we just simply initialize the view controller (module) that we're navigating to and pass the object that we need?

alphabikram commented 7 years ago

@jeffgilbert @scottrhoyt @sebastianwr @torstenlehmann @fatuhoku @samwyndham

Guys I got a Facebook like feed in my view controller so I decided to rewrite code with VIPER . I got one question, where should I maintain my array of posts (MyPostArray) in the VIPER .

Any suggestion would be highly appreciated.

Ronaldoh1 commented 7 years ago

@octopusbaba Presenter ....