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

Different PresenterManager implementations for MVI and MVP #212

Closed mkoslacz closed 7 years ago

mkoslacz commented 7 years ago

Hi, I found out that in the 3.0 version you have introduced aPresenterManager. It has two different implementations for MVI and MVP. I've got some (rather conceptual) questions about these implementations:

  1. Why have you decided to let them rely on a completely different mechanism of retaining presenters?
  2. Why have you decided to abandon (kinda platform-provided) MVP retention mechanisms in MVI Fragments and Activities? Why have you decided to use PresenterManager there?

I know that these questions are rather close to the metal but I would like to get the general feeling of these decisions to understand the possible usecases better.

sockeqwe commented 7 years ago

I have to apologize, I simply haven't found time to refactor MVP modules. For the final release 3.0 release both, MVI and MVP will use the same PresenterManager from the presenter manager module.

mkoslacz commented 7 years ago

Well, I was aware that it may be like this because of the alpha stage, no need to apologize ;)

@sockeqwe, could you tell me why you have decided to use this particular implementation, not the second one? I'm still curious about the answer for the second question: Why have you decided to abandon MVP retention mechanisms in the MVI Fragments and Activities? Do you plan to do the same in the MVP ones?

sockeqwe commented 7 years ago

The implementation of the presenter manager in MVP currently (3.0 SNAPSHOT) uses a retaining Fragment. The problem is that is is not guaranteed when this retaining Fragment is committed. Perhabs that this happens after another MVP fragment is already committed. No guarantees.

The MVI implementation basically uses a static map with some additional tricks to ensure that it is scoped to the parent activity. This also unifies the way a presenter is stored internally. Activity, Fragment, retaining Fragments, child Fragmetns, ViewGroups (like MvpFrameLayout) now all use this static map.

Why have you decided to abandon MVP retention mechanisms in the MVI Fragments and Activities?

Not sure what you exactly mean / referring to, but I guess you mean that there is no shouldRetainInstance() / isRetainInstance() method any more? This has now simply be moved into the corresponding MviDelegate and is configureable with a constructor parameter.

Do you plan to do the same in the MVP ones?

Yes, MvpDelegates will take that responsibility similar to MviDelegate