[todo-mvp-dagger] Fragments get constructed twice every time an Activity is restored

[peterholak]

The code currently @Injects the StatisticsFragment in StatisticsActivity, which can lead to a very confusing situation when the Activity is restored (configuration change, low memory, etc.):

• Dagger creates a new StatisticsFragment instance and injects it into the field.
• A new, different StatisticsFragment instance gets created and attached by super.onCreate as part of the restore process.
• The code detects that the restored fragment already exists, so it doesn't use the Dagger-created fragment instance.
• The injected fragment field now contains an unused, unattached fragment.

The fact that StatisticsFragment's constructor was called twice is also a problem.

A similar situation can occur with the other activities.

[MikeMitterer]

@google: If you at Google are not able to properly inject things with Dagger than something goes really wrong with this DI-Tool.

[peterholak]

I think the main problem is with the sample assuming that you can even @Inject fragments into activities, which is simply not the case, because of their lifecycle.

As it stands, the todo-mvp-dagger sample is likely to give wrong information to people who are trying to learn how to work with dagger, which is pretty bad for a pseudo-"official" repository.

I guess a more general issue is also the fact that Dagger even lets you do this. Dagger2 is supposed to be all about compile-time safety and checks, but dagger-android mostly fails at runtime whenever you screw something up - but that's a problem with Dagger, not this repository.

[digitalbuddha]

Thanks for caching it! This was a bug I introduced due to me trying to make everything a nail when dagger was a hammer. I'll refactor shortly.

FYI, this project is maintained both by Google dev advocates & members of the community. Your constructive feedback is always appreciated and will be taken into account.

[digitalbuddha]

hi @peterholak would you like to take a stab at fixing the fragment injection issue? I see 2 solutions that would work:

1. Change the injection to be Lazy<Fragment> so that we don't create the fragment until we are sure it did not get restored
2. Remove the injected fragment completely and create it not using dagger as needed.

Since you found the issue I figured I'd give you a chance to get some internet points :-) If not I'd be happy to fix. Thank you and enjoy your day

[peterholak]

Thanks, but I'm a bit busy right now to take a look at this.

I would probably lean more towards the second option (any DaggerFragment will inject its fields in onAttach anyway), but you might want to ask Dagger authors about which solution they view as the correct one.