CODEWITHSUNDEEP
javaandroidmvvmkotlindagger-2
Onur D.
Onur D.

Reputation: 570

My ViewModel does not survive configuration changes

I'm trying out Dagger 2 with MVVM. Here is what I have so far,

My ViewModel binding module:

@Module
public abstract class ViewModelModule {

    @Binds
    @IntoMap
    @ViewModelKey(MedicationsViewModel.class)
    @Singleton
    abstract ViewModel bindMedicationsViewModel(MedicationsViewModel viewModel);

    @Binds
    @Singleton
    abstract ViewModelProvider.Factory bindViewModelFactory(ViewModelFactory factory);
}

My Fragment:

class MedicationsFragment : DaggerFragment() {
    private lateinit var binding : FragmentMedicationsBinding
    private lateinit var viewModel: MedicationsViewModel

    @Inject lateinit var viewModelFactory : ViewModelFactory

    override fun onCreateView(
            inflater: LayoutInflater,
            container: ViewGroup?,
            savedInstanceState: Bundle?
    ): View? {
        binding = FragmentMedicationsBinding.inflate(inflater, container, false)

        viewModel = ViewModelProviders.of(this, viewModelFactory).get(MedicationsViewModel::class.java)

        val adapter = MedicationAdapter()
        binding.rvMedications.adapter = adapter

        viewModel.getMedications().observe(viewLifecycleOwner, Observer { items ->
            if (items != null) {adapter.submitList(items); adapter.notifyDataSetChanged()}
        })

        return binding.root
    }
}

And my ViewModel is like this:

class MedicationsViewModel @Inject constructor(
    private val repository: MedicationRepository,
    ): ViewModel() {
        private var medications : MutableLiveData<MutableList<Medication>> = MutableLiveData()
        private val disposable: CompositeDisposable = CompositeDisposable()

        fun getMedications() : MutableLiveData<MutableList<Medication>>{
            getMockMedications()
            return medications }

        private fun getMockMedications(){
            val medication1 = Medication(1,"Mock Med 1", "2018-01-01","once per day",true,null)
            val medication2 = Medication(2,"Mock Med 2", "2018-01-02","once per day",false,"before 15 minutes")

            val mockList: MutableList<Medication> = mutableListOf()
            mockList.add(medication1)
            mockList.add(medication2)

            medications.postValue(mockList)
        }

        fun addMedication(medication: Medication){
            medications.value?.add(medication)
            medications.notifyObserver()
        }

        fun <T> MutableLiveData<T>.notifyObserver() {
            this.postValue(this.value)
        }
    }

Through a button in my fragment, I am opening an activity and adding another Medication item to my viewmodel. After finish() out from my activity, I can see the newly added Medication item in my RecyclerView in fragment. However, when I rotate the screen I lost the newly added item and left with my mock Medication items again. This is where I think a new instance of my viewmodel is being created.

The problem might be a cause of Dagger creating a new instance of my viewmodel everytime my fragment (and container activity) is recreated. But I thought annotating my ViewModelFactory with @Singleton scope would have fixed it.

Can anyone see what I am missing?

Thanks.

Upvotes: 0

Views: 2392

Answers (2)

VIN
VIN

Reputation: 6947

In case this helps anyone else, you may need to move your viewModel instantiation to onActivityCreated to ensure that the viewModelStore has been created.

@Override
    public void onActivityCreated(@Nullable Bundle savedInstanceState)
    {
        super.onActivityCreated(savedInstanceState);
        viewModel = ViewModelProviders.of(this, viewModelProviderFactory).get(MedicationsViewModel.class);
    ... 
    }

Upvotes: 0

David Medenjak
David Medenjak

Reputation: 34532

You're mixing at least 4 different things here.

The problem might be a cause of Dagger creating a new instance of my viewmodel everytime my fragment (and container activity) is recreated.

You declare viewModel to be field injected when you inject your fragment (since you added an @Inject annotation on the property...)

@Inject lateinit var viewModelFactory : ViewModelFactory

This means that Dagger will inject the field when you call AndroidInjection.inject(..). Since you use constructor injection (@Inject on the constructor of MedicationsViewModel) Dagger will create a new object (you didn't scope the class) every single time, but you're never using the object anyways because...

Right after that happened, you override the field with the viewModel from the ViewModelFactory

viewModel = ViewModelProviders.of(this, viewModelFactory).get(MedicationsViewModel::class.java)

Since the ViewModelFactory uses the @Binds @IntoMap @Singleton bindMedicationsViewModel this should reuse the same object and be @Singleton scoped. You didn't share your implementation, so if it isn't the same object you might not properly handle your @Singleton component.

More issues...

But I thought annotating my ViewModelFactory with @Singleton scope would have fixed it.

Usually you should put scopes on the class and not on the binding.

@Singleton // scope the class!
class MedicationsViewModel @Inject constructor()

This is because it is usually an implementation detail what scope something has. But in any case, you should not use @Singleton here at all, since you are trying to use the ViewModel thingy. If you want to use @Singleton for your ViewModel then you don't need the whole ViewModelProviders bit. As you can see this will only lead to errors and confusion as both have different scopes and are handled differently.

A better setup

Assuming you want to use ViewModels you do not scope the ViewModel with Dagger. You let the Android Arch components handle things. You only use Dagger to remove the boilerplate of the object creation so that ViewModel.Factory can easily create a new ViewModel when necessary. You do so by binding the ViewModel to the collection that gets passed to your factory (the map with <Class, Provider<ViewModel>>) Calling provider.get() will thus will always create a new object, as expected. The lifecycle gets handled by the arch components after all.

@Binds
@IntoMap
@ViewModelKey(MedicationsViewModel.class) // no scope here!
abstract ViewModel bindMedicationsViewModel(MedicationsViewModel viewModel);

// neither here!
class MedicationsViewModel @Inject constructor()

With the setup in place ViewModelStoreOwner (ViewModelProviders.of(this)) will manage the ViewModels lifecycle and decide when to reuse or recreate the model. If you were to introduce a Dagger scope on the ViewModel it will either do nothing (if the scope is shorter lived) or might lead to bugs or unexpected behavior (when using @Singleton or longer lived ones)

Further the ViewModel.Factory should also be unscoped

@Binds // no scope here either!
abstract ViewModelProvider.Factory bindViewModelFactory(ViewModelFactory factory);

This is so that you can also add ViewModels on your Fragment / Activity scoped components and will enable you to inject classes that are not singleton scoped as well. This will greatly improve flexibility. A scope on the class wouldn't have any benefits either, since the ViewModels don't get stored in there during configuration changes and it doesn't hold any state.

Last but not least, you don't inject the field with Dagger, but you use ViewModelProviders to initialize it. While it shouldn't do anything, since you override it again moments later, it's still a bad smell and will only lead to confusion.

// in your fragment / acitvity
lateinit var viewModelFactory : ViewModelFactory // no @Inject for the viewmodel here!

// it will be assigned in onCreate!
viewModel = ViewModelProviders.of(this, viewModelFactory).get(MedicationsViewModel::class.java)

I hope this clears some things up!

Upvotes: 4

Related Questions