0

Can I use one ViewModel instead of my two view models(AboutViewModel and AboutListItemViewModel)?

Here is my code:

class AboutAdapter(private val clickListener: OnItemClickListener?) : AbstractListAdapter() {

    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): AboutItemViewHolder {
        val binding: AboutBinding = DataBindingUtil
            .inflate(
                LayoutInflater.from(parent.context), R.layout.view_item_about,
                parent, false
            )
        return AboutItemViewHolder(binding = binding)
    }

    override fun onBindHolder(holder: RecyclerView.ViewHolder, position: Int, item: Any) {
        (holder as AboutItemViewHolder).bind(item as SettingsItemViewModel, listener = clickListener)
    }
}

My AboutFragment:

class AboutFragment : BaseFragment() {

    private lateinit var viewModel: AboutViewModel
    private lateinit var viewModelFactory: AboutViewModelFactory
    private var onItemClickListener: OnItemClickListener = object : OnItemClickListener {
        override fun onItemClick(titleName: Int) {
            when (titleName) {
                R.string.about_terms_service -> {
                    activity?.addFragment(WebViewFragment.newInstance(TERMS_LINK, getString(R.string.about_terms_service)))
                }
                R.string.about_open_source_licenses -> activity?.addFragment(LicensesFragment())
            }
        }
    }

    override fun onCreateView(
        inflater: LayoutInflater,
        container: ViewGroup?,
        savedInstanceState: Bundle?
    ): View {

        viewModelFactory = AboutViewModelFactory(requireContext(), onItemClickListener)
        viewModel = ViewModelProviders.of(this, viewModelFactory)
            .get(AboutViewModel::class.java)

        return inflater.inflate(R.layout.fragment_base_list, container, false)
    }

    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        super.onViewCreated(view, savedInstanceState)
        activityComponents?.updateAppbarTitleWithFabAction(getString(R.string.about_feature_title))
        setupViews()
    }

    private fun setupViews() {
        try {
            val layoutManager = LinearLayoutManager(activity)
            recyclerView?.layoutManager = layoutManager
            val dividerItemDecoration = DividerItemDecoration(context, layoutManager.orientation)
            ContextCompat.getDrawable(context ?: return, R.drawable.shape_full_divider)?.let {
                dividerItemDecoration.setDrawable(it)
            }
            recyclerView?.addItemDecoration(dividerItemDecoration)
            recyclerView?.adapter = AboutAdapter(viewModel.onItemClickListener).apply {
                data = viewModel.getListItems()
            }
        } catch (e: Exception) {
            e.log()
        }
    }
}

My AboutItemViewHolder:

class AboutItemViewHolder(
    binding: AboutBinding
) : RecyclerView.ViewHolder(binding.root){

    private var aboutBinding: AboutBinding? = null

    init {
        this.aboutBinding = binding
    }

    fun bind(item: SettingsItemViewModel, listener: OnItemClickListener?) {
        aboutBinding?.about = AboutListItemViewModel(item)
        aboutBinding?.onItemClickListener = listener
        aboutBinding?.executePendingBindings()
    }
}

interface OnItemClickListener {
    fun onItemClick(titleName: Int)
}

My first ViewModel which I used in adapter AboutListItemViewModel:

class AboutListItemViewModel(item: SettingsItemViewModel) {

    val titleName: Int = item.titleId
    val subTitleName: String? = item.value
    var isVisible: Boolean = item.value != null
}

My second ViewModel which I used in fragment AboutViewModel:

class AboutViewModel(val appContext: Context, val onItemClickListener: OnItemClickListener): ViewModel() {

    fun getListItems(): List<SettingsItemViewModel> {
        return listOf(
            SettingsItemViewModel(
                titleId = R.string.about_app_version,
                value = AppTools.getAppVersionName(appContext)
            ),
            SettingsItemViewModel(
                titleId = R.string.about_copyright,
                value = appContext.getString(R.string.about_copyright_description)
            ),
            SettingsItemViewModel(
                titleId = R.string.about_terms_service,
                itemIsClickable = true
            ),
            SettingsItemViewModel(
                titleId = R.string.about_open_source_licenses,
                itemIsClickable = true
            )
        )
    }
}

My AboutViewModelFactory:

class AboutViewModelFactory(val appContext: Context, private val onItemClickListener: OnItemClickListener) : ViewModelProvider.Factory {
    override fun <T : ViewModel?> create(modelClass: Class<T>): T {
        if (modelClass.isAssignableFrom(AboutViewModel::class.java)) {
            return AboutViewModel(appContext, onItemClickListener) as T
        }
        throw IllegalArgumentException("Unknown ViewModel class")
    }
}

SettingsViewModel is simple data class:

data class SettingsItemViewModel(
    @StringRes val titleId: Int,
    val value: String? = null,
    val switchEnabled: Boolean? = null,
    val isChecked: Boolean = false,
    val itemIsClickable: Boolean = false,
    val colorResId: Int = R.color.black
)
Jason Aller
  • 3,541
  • 28
  • 38
  • 38
Morozov
  • 4,968
  • 6
  • 39
  • 70

1 Answers1

1

ViewModels are for the MVVM pattern, in which a ViewModel is ignorant of any View type classes, such as an OnItemClickListener, so you have an unintended usage where you are passing listeners to ViewModel factories. This also will leak your Fragment or Activity if there is a configuration change.

I don't see you even using the listener from within the ViewModel, so you could completely eliminate this property and the factory altogether. You can inherit from AndroidViewModel and take a single Context argument in your constructor. Then you can use by viewModels() for your ViewModel property.

Without these view callback arguments, there would be nothing to limit you from putting all of this in a single ViewModel implementation.

It's also odd that your view model has a function that returns a list of other ViewModels. Why can these just be simple data classes? A ViewModel is shouldn't be directly instantiated yourself (except by a factory implementation). Otherwise, its lifecycle won't be managed.

Tenfour04
  • 83,111
  • 11
  • 94
  • 154
  • SettingsItemViewModel is simple `data class`, i updated question. So if i undestood you correctly i need to remove clicklistener from my factory? – Morozov Mar 26 '20 at 15:35
  • Now i updated code like this https://gist.github.com/mnewlive/41da1828713e9a37e5d36036ccb607da Can u check please? – Morozov Mar 26 '20 at 15:54