sockeqwe / mosby

A Model-View-Presenter / Model-View-Intent library for modern Android apps
http://hannesdorfmann.com/mosby/
Apache License 2.0
5.49k stars 841 forks source link

Mosby MVI + AdapterDelegates integration (presenters for RV items) #250

Closed sevar83 closed 6 years ago

sevar83 commented 7 years ago

Hi, I have a RecyclerView with items which became quite complex. I already use Mosby MVI and your AdapterDelegates library. They are doing a great job but I wish to integrate MVI into the adapter and have MVI presenter for each RV item. There's a similar implementation here: https://github.com/remind101/android-arch-sample/blob/master/app/src/main/java/com/remind101/archexample/MvpRecyclerAdapter.java

I'll try to do it myself but it doesn't look easy. Any advice and narration is very welcome.

sockeqwe commented 7 years ago

I don't think this is a good idea. We did that at work 2 years ago, didn't worked out to well and increased complexity.

I have already answered this question somewhere on my blog or somewhere on a github issue where I provided some more details why i think that this is a bad idea. I'm on holidays right now and have no pc with me. I will try to find that answer and copy & paste it here when I'm back from holidays (Thursday)

sevar83 commented 7 years ago

Have a nice holiday, let's speak when you're back.

sockeqwe commented 7 years ago

Sorry for my late response, it took me some time to find the answer I have written some time ago, but here it is:

Original question was:

Question is actually how to approach that in a clean way? Should I create an instance of presenter for every type of item in the RecyclerView (how and where should I do it then using delegate pattern) or should I create some "god" presenter that will handle click actions for every type of item? Thanks in advance!

My answer was: (on MVP, not MVI but I think that doesn't matter)

First it depends on the way how you interprete MVP or which variant of MVP you have implemented. I'm a fan of passive view. In my opinion it's definitely a good idea to split big views in smaller views with own tiny presenter. So every view is decoupled, has it's own Presenter and a clear separation between model and view and the source code for each class is more understandable, testable and a smaller number of lines of code per class. So having a MVP View (i.e. a ViewHolder) and a Presenter for each is a item sounds like a good solution, i.e. every view (i.e. ViewHolder) can handle it's own model independently with his own presenter. So in your example clicking on a button (which is part of ViewHolder) can be handled by his presenter and then update the model and this might trigger the presenter to update the view. For example you could have a Fragment that loads a list of items by using a Presenter and then displays the items in a RecyclerView. The RecyclerView's adapter can be configured (i.e. by using this AdapterDelegates) to instantiate a ViewHolder (which is seen as MVP View) and its own Presenter. The problem here is that your views gets recycled during scrolling. It gets even more complicated if you want to do async background tasks in your ViewHolder's Presenter (i.e. on button click make an http call). So when the view gets recycled you have to tell the presenter to stop somehow or at least detach the view from presenter. But what if you scroll back in RecyclerView to the original ViewHolder where you have clicked the Button which has executed an http call. How do you reattach the view to the original Presenter instance with the running http request? Not that easy since the ViewHolder itself will be recycled and bound. It get even more complex once we start to think about Presenter for ViewHolder that survive screen orientation changes.

Another problem I see with this approach (Presenter for each ViewHolder) is that the datasource of the adapter is managed by another presenter (i.e. Presenter of "parent" Fragment). So this one can add / remove and also update items. Maybe it is easier if you follow the convention that Presenter of "parent" fragment is only responsible to add / remove items from data set and only presenter of ViewHolder is responsible to update the item itself, but I still think that this is much more error prone and might leave to inconsistent state since parent Presenter and ViewHolder Presenter might want to touch a certain ViewHolder.

So in theory it sounds like a good idea to split that single big Fragment (MVP View) into smaller views, but in practice this is doable but hard to implement efficiently. Therefore, unfortunately, my recommendation is to use a "god" presenter. Usually I have implemented somehow like this: a AdapterDelegate gets a OnClickListener as constructor parameter, which will be set as OnClickListener on the Button of the ViewHolder. The Adapter gets a list of AdapterDelegates as constructor parameters (or an DelegatesManager). So the Fragment itself reacts on the OnClickEvent and invokes the corresponding "god" presenter method.

TL;DR: I recommend to got with the "god" presenter. However, I strongly recommend to split a big View (i.e. Activity) into smaller views (i.e. Fragments with own presenters), but applying MVP on RecyclerView items is very tricky and I would avoid that.

So back to your question. You said you want to introduce Presenter for each ViewHolder because:

I have a RecyclerView with items which became quite complex

Can you elaborate what exactly is complex in your concrete example? I think that this complexity will not be solved by giving each ViewHolder it's own Presenter (maybe the complexity is then just moved somewhere else).

sevar83 commented 7 years ago

Sorry, I've just seen your reply. Quite a lot of emails coming from the Mosby maillist. My screen is a horizontally swipeable RV that loads page by page in both directions starting from an arbitrary page. The item is usually fullscreen and inside the item I have a CoordinatorLayout > CollapsibleLayout, NestedScrollView with a bunch of widgets and one or more nested RVs. I have to add even more UI. So, the "meat" of the presentation is inside the VeiwHolder. I already discarded the idea of item presenters. It became overly complicated and I couldn't keep the whole picture in my head. Instead I keep some of the logic in the VH but just the low level formatting details, animating childs, etc. The Presenter should not be responsible for all the view details, I guess.