Should the Viewmodel of a DetailFragment be shared amongst instances?

I wrote a master fragment that shows a list of stores, call it StoreFeedFragment. If you click on a store it calls newInstance of StoreDetailFragment to provide details of the clicked store. Both fragments exist in MainActivity that swaps the 2 fragments using replace().

class MainActivity : AppCompatActivity() {

    @Inject
    lateinit var storeItemViewModelFactory: ViewModelFactory<StoreItemViewModel>
    private val storeItemViewModel: StoreItemViewModel by lazy {
        storeItemViewModelFactory.get<StoreItemViewModel>(
            this
        )
    }

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        TCApplication.getAppComponent().inject(this)
        setContentView(R.layout.activity_main)
        if (savedInstanceState == null) {
            val storeFeedFragment = StoreFeedFragment()
            supportFragmentManager.beginTransaction()
                .replace(
                    R.id.container, storeFeedFragment,
                    StoreFeedFragment.TAG
                )
                .commit()
        }

        storeItemViewModel.selectedItem.observe(this) { event ->
            event.getContentIfNotHandled()?.apply {
                val fragment = StoreDetailFragment.newInstance(id)
                supportFragmentManager.beginTransaction()
                    .replace(
                        R.id.container, fragment
                    ).addToBackStack("feed_to_item_tag")
                    .commit()
            }
        }
    }
}

Event is just an event wrapper to prevent observing the selected store again after a backpress from StoreDetailFragment:

class Event<T>(private val content: T) {

    var hasBeenHandled = false
        private set

    fun getContentIfNotHandled(): T? {
        return if (hasBeenHandled) null
        else {
            hasBeenHandled = true
            content
        }
    }
}

Recently I noticed a bug

1. Click into store 1 on StoreFeedFragment.
2. StoreDetailFragment shows details for store 1.
3. Click back to return to StoreFeedFragment.
4. After returning to StoreFeedFragment click on store 2.
5. StoreDetailFragment shows details for store 1, then store 2. It should not show details for store 1 because store 2 was clicked.

class StoreDetailFragment : Fragment() {

@Inject
lateinit var viewModelFactory: ViewModelFactory<StoreDetailViewModel>
private val viewmodel: StoreDetailViewModel by lazy {
    viewModelFactory.get<StoreDetailViewModel>(
        requireActivity()
    )
}

companion object {
    private const val SELECTED_ID = "selected"

    fun newInstance(storeId: String): StoreDetailFragment {
        val fragment = StoreDetailFragment().also {
            it.arguments = bundleOf(Pair(SELECTED_ID, storeId))
        }
        return fragment
    }
}

override fun onCreate(savedInstanceState: Bundle?) {
    TCApplication.getAppComponent().inject(this)
    super.onCreate(savedInstanceState)
}

override fun onCreateView(
    inflater: LayoutInflater,
    container: ViewGroup?,
    savedInstanceState: Bundle?
): View? {
    val view = inflater.inflate(R.layout.fragment_store_detail, container, false)
    viewmodel.storeDetailResult.observe(viewLifecycleOwner) { resultState ->
        when (resultState) {
            is ResultState.Loading -> { }
            is ResultState.Success -> { showDetails(resultState.value) }
            is ResultState.Failure -> {
                Snackbar.make(
                view,
                "Error getting store details ${resultState.ex.message}",
                500
            ).show() }
        }
    }
    arguments?.getString(SELECTED_ID)?.let { storeId -> viewmodel.loadStoreDetails(storeId) }
    return view
}

private fun showDetails(storeDetails: StoreDetail) {
    view?.apply {
        findViewById<TextView>(R.id.name).text = storeDetails.name
        findViewById<TextView>(R.id.phoneNo).text = storeDetails.phoneNo
    }
}
}


class StoreDetailViewModel @Inject constructor(
    private val storeDetailRepository: StoreDetailRepository
): ViewModel(), CoroutineScope by MainScope() {

    private val _storeDetailResult = MutableLiveData<ResultState<StoreDetail>>()
    val storeDetailResult: LiveData<ResultState<StoreDetail>> = _storeDetailResult

    fun loadStoreDetails(storeId: String) {
        viewModelScope.launch {
            try {
                storeDetailRepository.getStoreDetail(storeId)
                    .collect { storeDetail ->
                        _storeDetailResult.postValue(
                            ResultState.Success(
                                storeDetail
                            )
                        )
                    }
            } catch (e: Exception) {
                _storeDetailResult.postValue(
                    ResultState.Failure(
                        "getStoreDetail($storeId)", e
                    )
                )
            }
        }
    }
}

From log statements it became clear that this occurred because ViewModelFactory only creates a single instance of StoreDetailViewModel that is shared between StoreDetailFragment instance for store 1 and StoreDetailFragment instance for store 2. StoreDetailFragment for store 2 was observing the latest emission from StoreDetailViewModel (info for store 1), then calling loadStoreDetails for store2.

class ViewModelFactory<T: ViewModel>
@Inject constructor(private val viewModel: Lazy<T>) : ViewModelProvider.Factory {

    @Suppress("UNCHECKED_CAST")
    override fun <T : ViewModel> create(modelClass: Class<T>): T {
        return viewModel.get() as T
    }

    /**
     * Returns an instance of a defined ViewModel class.
     */
    inline fun <reified R: T> get(viewModelStoreOwner: ViewModelStoreOwner): T {
        return ViewModelProvider(viewModelStoreOwner, this)[R::class.java]
    }
}

Should StoreDetailViewModel be shared among instance 1 and 2 of StoreDetailFragment? What do I need to change in my code to follow the typical solution for this problem? Thanks in advance.

> Should StoreDetailViewModel be shared among instance 1 and 2 of StoreDetailFragment?

Yes, because you scoped the ViewModel the the Activity.

@Inject
lateinit var viewModelFactory: ViewModelFactory<StoreDetailViewModel>
private val viewmodel: StoreDetailViewModel by lazy {
    viewModelFactory.get<StoreDetailViewModel>(
        requireActivity()
    )
}

See: https://developer.android.com/guide/fragments/communicate#host-activity

> What do I need to change in my code to follow the typical solution for this problem?

Scope your ViewModel to the Fragment.

@Inject
lateinit var viewModelFactory: ViewModelFactory<StoreDetailViewModel>
private val viewmodel: StoreDetailViewModel by lazy {
    viewModelFactory.get<StoreDetailViewModel>(
        this
    )
}

See: https://developer.android.com/topic/libraries/architecture/viewmodel/viewmodel-apis