Dagger 2 with MVP, avoid creating extra presenter

2019-02-11 08:31发布

问题:

I have an app implementing the MVP pattern with a Loader to maintain the presenter object on view recreation (there's an article about this here). I'm new to Dagger 2, trying to implement it together with the current code.

I've managed to make it work, but now my presenter is created twice. At first it was created using a factory class which was initialized in onCreateLoader, but then, when adding Dagger 2 implementation, I had two objects created (in factory class and when injecting).

Now I avoid creating a new presenter in onCreateLoader and passes the injected presenter instead. The problem is on view recreation: each time the view is destroyed and recreated, a new presenter is injected in OnCreate / OnCreateView. This is the scenario:

  1. A new presenter is injected:

    @Override
    public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
        ...
        getControllerComponent().inject(this);
        ...
    }
    
  2. Initializing Loader, onCreateLoader is called if the Loader doesn't exist. Note that we pass the presenter that was injected:

    @Override
    public void onActivityCreated(Bundle savedInstanceState) {
        super.onActivityCreated(savedInstanceState);
        getLoaderManager().initLoader(PRESENTER_LOADER_ID, null, this);
    }
    
    @Override
    public Loader<MyPresenter> onCreateLoader(int id, Bundle args) {
        switch (id) {
            case PRESENTER_LOADER_ID:
                return new PresenterLoader<>(getContext(), presenter);
                //return new PresenterLoader<>(getContext(), new MyPresenterFactory());
        }
    
        return null;
    }
    
  3. Assigning the presenter received from the Loader. If it was just created, we assign the same object that's already assigned so nothing happens. But, if the view was recreated, then Dagger 2 injected a new presenter and here we throw away the new presenter and replace it with the old presenter from the Loader.

    @Override
    public void onLoadFinished(Loader<MyPresenter> loader, MyPresenter data) {
        this.presenter = data;
    }
    

    I want to maintain the presenter instance so it's ± what I want to happen; my problem is with creating a redundant presenter object on each view recreation. First, it's unnecessary, and additionally, the view is holding a reference to a different presenter until the load is finished. Obviously I am not using the presenter during this period (after injection and before the load is finished), but I definitely don't like it and afraid that this new presenter will be mistakenly used in the future.

Dagger 2 experts, is there a way to create the presenter the first time (before Loader is created) but avoid it on view recreation? Thanks a lot!

回答1:

First off I just want to mention that if you inject your presenter, you should not assign it again later from your loader.

Either use injection to provide an object, or provide it yourself. If you do both, you are just at risk to introduce bugs.


is there a way to create the presenter the first time (before Loader is created) but avoid it on view recreation?

tl;dr You need a scope for your presenter that reflects the lifetime of your component which might survive configuration changes. This scope must not keep any reference to the Activitys Context.

Components follow some lifecycle. You usually have some @Singleton annotated component that you keep within your Application and some @PerActivity or similarly scoped components that you create per Activity, which will (and should) be recreated when an activity goes through configuration changes, since those dependencies often reference the Activity context and should live and die with the Activity.

The primary problem you face here is a scoping problem.

If your presenter is unscoped, you will recreate a new presenter every time you request one, which will inadvertently lead to bugs, as soon as you inject it somewhere else. Usually presenters are kept within the activity and often scoped by some @PerActivity scope.


If your presenter is part of the @PerActivity scope it should (like all the other @PerActivity dependencies) be recreated along with all the other dependencies. If you keep the presenter, but recreate all the other objects, the old presenter will still reference the old dependencies, creating memory leaks. Scoped objects should just exist within their own scope.

Objects in the same scope can reference one another and thus keeping one scoped object alive outside of its scope will also inadvertently lead to bugs, memory leaks, etc.

So you don't want to keep this presenter in your Loader either.


If on the other hand you say No, that presenter is one step higher in the hierarchy, part of @PerScreen, where I keep longer living objects then you need to find a way to actually keep this @PerScreen component alive while your @PerActivity component will be recreated along with the activity.

Assume the following scope hierarchy:

`X > Y` read X is subcomponent of Y
@Singleton > @PerScreen > @PerActivity

@Singleton: Application wide
@PerScreen: Multiple activty lifecycles, keep alive during config changes
@PerActivity: Maybe Context dependent, recreate with every activity

When a configuration change occurs you now can discard all of the @PerActivity objects and recreate them, while keeping the reference to your @PerScreen ones.

You might notice how I keep talking about keeping the @PerScreen component, not keeping the presenter, and that is the important part here:

On the @PerScreen scoped component, calling

myPerScreenComponent.getMyPresenter()

will always return the same @PerScreen scoped presenter.

Now if your @PerActivty scoped component is a subcomponent of MyPerScreenComponent, injecting your activity will always give you the same @PerScreen scoped presenter, which will survive orientation changes.

To prevent memory leaks, no object from your @PerScreen scope can reference the Activity, and the presenter should only keep a WeakReference on its view (or you have to make damn sure to set the view to null on destroy).

That's what scopes are for and that's how you avoid creating extra presenter object on view recreation.

So instead of keeping your presenter in your loader you should try to keep the component in your loader to avoid unnecessary recreation of your objects.


All of this will probably introduce a lot more complexity since you now have 2 scopes per activity and more callbacks.

I have also seen other approaches where you keep your presenters as Singletons in your Application, but this introduces the same problems where you have to make sure not to keep any references to your Activity.

Personally, I would just recreate the presenters and restore the state, but if you choose to go with your approach you should make sure that you have a solid understanding of scopes and dependencies.