Home > Back-end >  Android RecyclerView items list doesn't change appropriately after chang settings and navigate
Android RecyclerView items list doesn't change appropriately after chang settings and navigate

Time:01-18

I have single activity android application, a service app with task management. For navigation i use navigation component with bottom navigation. I also use data binding and Dagger2 DI, if it could be important for problem investigation.

After user successfully logged in, home screen with a list of queues (horizontal recyclerview) appears.

Home Fragment : -

Each queue (recyclerview item) has appropriate list of tasks available to perform by the user.

You can select any queue item which is active (contains at least one task in it). Tasks of the selected queue displayed below the recyclerview like another vertical recylcerview. There is also the summary queue item (very left item on the picture) which calculated and shows all tasks from all available queues.

Appearance of this summary queue item depends on the switch which is on the profile screen which is represented as Profile Fragment

Profle fragment : -

Scenario:

  1. Summary queue item shown on Home screen by default;
  2. I navigate to Profile screen and set switcher off. Here in ProfileFragment i call the method updateGeneralQueueState in view model which save at room db parameter isShouldBeShown (false in this case);
  3. I navigate back to the Home screen. Here i retrieve isShouldBeShown parameter in my Home Fragment with calling apropriate method in view model which returns a earlier saved parameter from room db.

Problem: I expect to see that summary queue item is not in the list of queues and most often it is, but sometimes when i repeat this scenario it is not. If not i go to profile fragment or any other screen, then go to home screeen again and then the summary queue item is not in the list as expected.

There are probably some architectural mistackes, thats why I'm asking for real help and explaining the reason for problem occurrence, as I would like not just only solve it, but also to understand this strange behavior. I will attach below all related code! Many thanks in advance!

HomeFragment.kt

class HomeFragment : BaseFragment<HomeFragmentBinding>(), MenuItem.OnActionExpandListener {

    @Inject lateinit var factory: HomeViewModelFactory
    @Inject lateinit var viewModel: HomeViewModel
    private lateinit var ticketsListAdapter: TicketsListAdapter
    private lateinit var queuesListAdapter: QueuesListAdapter
    private var searchView: SearchView? = null
    private var pageLimit: Long = 10
    private var offset: Long = 0L
    private var selectedQueueId: Long = 0L
    private var selectedQueueIndex: Int = 0
    private var prevTicketsThreshold: Int = 0 // new
    private var ticketsThreshold: Int = 0
    private var lockId: Int = 1
    private var allQueueIds: List<Long> = listOf()
    private var isGeneralShoudlBeShown: Boolean = false
    private var favoriteMode: Boolean = false
    private lateinit var prefs: Prefs
    private var selectedQueue: Queue? = null

    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        ComponentsHolder.getComponent().inject(this)
        super.onViewCreated(view, savedInstanceState)

        prefs = Prefs(requireContext())

        (activity as MainActivity).showBottomNavigation()
        (activity as MainActivity).getUnreadNotificationsCount()

        val toolbar = view.findViewById(R.id.tickets_search_toolbar) as Toolbar
        (activity as MainActivity).setSupportActionBar(toolbar)
        toolbar.title = "Главная"
        setHasOptionsMenu(true)

        viewModel = ViewModelProvider(this, factory)[HomeViewModel::class.java]
        binding.model = viewModel
        binding.lifecycleOwner = this

        with(viewModel) {
            (activity as MainActivity).getPushToken { t ->
                registerPushToken(t)
                getUserSettings()
                getUnreadNotificationsCount()
            }

            notificationscount.observe(viewLifecycleOwner) {
                it?.let {
                    if (it.unreadCount > 0) {
                        (activity as MainActivity).setUnreadNotificationsCount(it.unreadCount)
                            .also { (activity as MainActivity).getUnreadNotificationsCount() }
                    }
                }
            }

            checkUserSettings.observe(viewLifecycleOwner) {
                isGeneralShoudlBeShown = it.isGeneralChecked
                favoriteMode = it.isFavoritesChecked!!
                getQueues(isGeneralShoudlBeShown, favoriteMode, selectedQueueIndex)
            }

            queueIds.observe(viewLifecycleOwner) {
                it?.let {
                    allQueueIds = it
                }
            }

            queues.observe(viewLifecycleOwner) {
                it?.let {
                    when (it.responseCode) {
                        200 -> {
                            queuesListAdapter.submitList(it.queues)
                            queuesListAdapter.notifyDataSetChanged()
                            retrieveSelectedQueue(it.queues)
                            getTickets(
                                if (selectedQueueId == 0L) 0 else selectedQueueId,
                                if (selectedQueueId == 0L) allQueueIds else emptyList(),
                                lockId,
                                pageLimit,
                                offset
                            )
                        }
                    }
                }
            }

            tickets.observe(viewLifecycleOwner) {
                it?.let {
                    binding.refreshDate.text = getLastRefreshDateTime()
                    Log.i("hmfrgmnt", it.toString())
                    when (it.responseCode) {
                        401 -> {
                            binding.bottomProgress.visibility = View.GONE
                            if (mayNavigate()) {
                                findNavController().navigate(
                                    HomeFragmentDirections
                                        .actionHomeFragmentToSplashFragment()
                                )
                            }
                        }
                        200 -> {
                            binding.bottomProgress.visibility = View.GONE
                            ticketsListAdapter.submitList(null)
                            ticketsListAdapter.notifyDataSetChanged()
                        }
                        else -> (activity as MainActivity).showErrorDialog(
                            it.responseMessage!!,
                            null
                        )
                    }
                }
            }

            navigateToTicketDetails.observe(viewLifecycleOwner) { ticketId ->
                ticketId?.let {
                    if (mayNavigate()) {
                        findNavController().navigate(
                            HomeFragmentDirections
                                .actionHomeFragmentToTicketDetailsFragment(ticketId)
                        )
                    }
                    viewModel.onTicketDetailsNavigated()
                }
            }
        }

        with(binding) {
            tabs.selectTab(tabs.getTabAt((lockId - 1)), true)
            (queueList.layoutManager as LinearLayoutManager).scrollToPositionWithOffset(selectedQueueIndex, queueList.top)

            ticketsListAdapter = TicketsListAdapter(TicketsListListener { ticketId ->
                viewModel.onTicketDetailsClicked(ticketId)
            })

            queuesListAdapter = QueuesListAdapter(
                QueuesListListener { queue ->
                    setActiveQueueData(queue)
                    tabs.selectTab(tabs.getTabAt((lockId - 1)), true)

                    viewModel.onQueueClicked(if (queue.queueId == 0L) 0 else selectedQueueId, if (queue.queueId == 0L) allQueueIds else emptyList(), lockId, pageLimit, offset)
//                    ticketsListAdapter.notifyDataSetChanged()
                }
            )

            ticketsList.adapter = ticketsListAdapter
            queueList.adapter = queuesListAdapter

            tabs.addOnTabSelectedListener(object : TabLayout.OnTabSelectedListener {
                override fun onTabSelected(tab: TabLayout.Tab?) {
                    when (tab?.position) {
                        1 -> {
                            offset = 0
                            lockId = 2
                            viewModel.onQueueClicked(if (selectedQueueId == 0L) 0 else selectedQueueId, if (selectedQueueId == 0L) allQueueIds else emptyList(), lockId, pageLimit, offset)
                        }
                        else -> {
                            offset = 0
                            lockId = 1
                            viewModel.onQueueClicked(if (selectedQueueId == 0L) 0 else selectedQueueId, if (selectedQueueId == 0L) allQueueIds else emptyList(), lockId, pageLimit, offset)
                        }
                    }
                }
                override fun onTabUnselected(tab: TabLayout.Tab?) {}
                override fun onTabReselected(tab: TabLayout.Tab?) {}
            })

            nestedScroll.setOnScrollChangeListener { v, _, scrollY, _, _ ->
                if ((scrollY > (v as NestedScrollView).getChildAt(0).measuredHeight - v.measuredHeight - homeMainLayout.paddingBottom) && viewModel.status.value != ApiStatus.LOADING) {
                    if (ticketsThreshold > prevTicketsThreshold) {
                        if (ticketsThreshold < pageLimit || ticketsThreshold == 0) {
                            moreButton.visibility = View.GONE
                            endOfListView.visibility = View.VISIBLE
                        } else {
                            moreButton.visibility = View.VISIBLE
                            endOfListView.visibility = View.GONE
                        }

                    } else if (ticketsThreshold == prevTicketsThreshold) {
                        moreButton.visibility = View.GONE
                        endOfListView.visibility = View.VISIBLE
                    } else {
                        moreButton.visibility = View.VISIBLE
                        endOfListView.visibility = View.GONE
                    }
                }
            }

            refreshButton.setOnClickListener {
                offset = 0
                viewModel.refresh(isGeneralShoudlBeShown, favoriteMode, selectedQueueIndex, selectedQueueId, allQueueIds, lockId, pageLimit, offset)
                (queueList.layoutManager as LinearLayoutManager).scrollToPositionWithOffset(selectedQueueIndex, queueList.top)
                tabs.selectTab(tabs.getTabAt((lockId - 1)), true)
                queuesListAdapter.notifyDataSetChanged()
            }

            moreButton.setOnClickListener {
                prevTicketsThreshold = ticketsThreshold
                offset  = pageLimit
                viewModel.getTickets(
                    if (selectedQueueId == 0L) 0 else selectedQueueId,
                    if (selectedQueueId == 0L) allQueueIds else emptyList(),
                    lockId,
                    pageLimit,
                    offset
                )
            }
        }
    }

    override fun getFragmentBinding(
        inflater: LayoutInflater,
        container: ViewGroup?
    ) = HomeFragmentBinding.inflate(inflater, container, false)

    private fun setActiveQueueData(queue: Queue) {
        offset = 0
        selectedQueue = queue
        prefs.queueObject = queue
        binding.selectedQueueTitle.text = queue.title
        selectedQueueIndex = queuesListAdapter.currentList.getQueuePosition(selectedQueue as Queue) ?: 0
        queuesListAdapter.currentList.forEach { i -> i.isSelected = false }
        queuesListAdapter.notifyDataSetChanged()
        queuesListAdapter.selectItem(selectedQueueIndex)
        (binding.queueList.layoutManager as LinearLayoutManager).scrollToPositionWithOffset(selectedQueueIndex, binding.queueList.top)
    }

    private fun saveSelectedQueueBeforeNavigating(selectedQueue: Queue) {
        prefs.queueObject = selectedQueue
    }

    override fun onDestroyView() {
        super.onDestroyView()
        Log.i("profileSaveQueue", "i will save queue: $selectedQueue")
        saveSelectedQueueBeforeNavigating(selectedQueue!!)
    }
}

HomeViewModel.kt

class HomeViewModel @Inject constructor(
    private val userRepository: UserRepository,
    private val ticketsRepository: TicketsRepository,
    private val queuesRepository: QueuesRepository,
    private val notificationsRepository: NotificationsRepository,
    private val pushRepository: PushRepository
) : BaseViewModel() {

    private var ticketsList: MutableList<Ticket> = mutableListOf()
    private var summaryTicketsCount: Int? = 0

    private val _status = MutableLiveData<ApiStatus>()
    val status: LiveData<ApiStatus>
        get() = _status

    private val _notificationsCount = MutableLiveData<NoticeCountResponse?>()
    val notificationscount: LiveData<NoticeCountResponse?>
        get() = _notificationsCount

    private val _tickets = MutableLiveData<TicketsResponse?>()
    val tickets: LiveData<TicketsResponse?>
        get() = _tickets

    private val _navigateToTicketDetails = MutableLiveData<Long?>()
    val navigateToTicketDetails
        get() = _navigateToTicketDetails

    private val _queues = MutableLiveData<QueuesResponse?>()
    val queues: LiveData<QueuesResponse?>
        get() = _queues

    private val _queueIds = MutableLiveData<List<Long>?>()
    val queueIds: LiveData<List<Long>?>
        get() = _queueIds

    private val _checkUserSettings = MutableLiveData<User>()
    val checkUserSettings: LiveData<User>
        get() = _checkUserSettings

    fun refresh(showGeneral: Boolean, favoriteOnly: Boolean, selectedQueueIndex: Int, queueId: Long, queueIds: List<Long>?, lockId: Int?, limit: Long?, offset: Long?) {
        ticketsList = mutableListOf()
        getQueues(showGeneral, favoriteOnly, selectedQueueIndex)
    }

    fun getUserSettings() {
        viewModelScope.launch {
            _checkUserSettings.value = retrieveUserSettings()
        }
    }

    private suspend fun retrieveUserSettings(): User? {
        return withContext(Dispatchers.IO) {
            userRepository.getUserInfo()
        }
    }

    fun getUnreadNotificationsCount() {
        _status.value = ApiStatus.LOADING
        viewModelScope.launch {
            kotlin.runCatching { notificationsRepository.getUnreadNotificationsCount("Bearer ${getToken()}") }
                .onSuccess {
                    _notificationsCount.value = it
                    _status.value = ApiStatus.DONE
                }
                .onFailure {
                    _status.value = ApiStatus.DONE
                }
        }
    }

    fun registerPushToken(token: String) {
        viewModelScope.launch {
            pushRepository.registerToken("Bearer ${getToken()}", TokenRegisterBody(token, 1))
        }
    }

    fun getQueues(showGeneral: Boolean, favoriteOnly: Boolean, selectedQueueIndex: Int) {
        _status.value = ApiStatus.LOADING
        viewModelScope.launch {
            kotlin.runCatching { queuesRepository.getQueuesListWithTicketsCount("Bearer ${getToken()}", favoriteOnly) }
                .onSuccess { value ->
                    summaryTicketsCount = value.queues?.mapNotNull { q -> q.ticketsCount }?.sum()
                    val queuesList: List<Queue> = sortQueues(value.queues, selectedQueueIndex, showGeneral)
                    _queueIds.value = value.queues?.map { item -> item.queueId }
                    _queues.value = QueuesResponse(queuesList, value.responseCode, value.responseMessage)
                    _status.value = ApiStatus.DONE
                }
                .onFailure {
                    if (it is HttpException) {
                        _queues.value = QueuesResponse(null, it.code(), getResponseMessage(it))
                        _status.value = ApiStatus.DONE
                    }
                    else {
                        _queues.value = QueuesResponse(null, -1, "Что-то пошло не так")
                        _status.value = ApiStatus.DONE
                    }
                }
        }
    }

    fun getTickets(queueId: Long?, queueIds: List<Long>?, lockId: Int?, limit: Long?, offset: Long?) {
        _status.value = ApiStatus.LOADING
        val body = TicketsListBody(queueId = queueId, queueIds = queueIds, lockId = lockId, limit = limit, offset = offset)
        viewModelScope.launch {
            kotlin.runCatching { ticketsRepository.getTickets("Bearer ${getToken()}", body) }
                .onSuccess {
                    it.tickets?.forEach { ticket -> if (ticket !in ticketsList) { ticketsList.add(ticket) } }
                    _tickets.value = TicketsResponse(ticketsList, it.responseCode, it.responseMessage)
                    _status.value = ApiStatus.DONE
                }
                .onFailure {
                    if (it is HttpException) {
                        _tickets.value = TicketsResponse(null, it.code(), getResponseMessage(it))
                        _status.value = ApiStatus.DONE
                    }
                    else {
                        _tickets.value = TicketsResponse(null, -1, "Что-то пошло не так")
                        _status.value = ApiStatus.DONE
                    }
                }
        }
    }

    private fun sortQueues(queues: List<Queue>?, selectedQueueIndex: Int, showGeneral: Boolean): List<Queue> {
        val favoriteQueuesList: List<Queue>? = queues?.toMutableList()
            ?.filter { a -> a.isInFavoritesList }
            ?.sortedByDescending { b -> b.ticketsCount }

        val restQueuesList: List<Queue>? = queues?.toMutableList()
            ?.filter { a -> !a.isInFavoritesList }
            ?.sortedByDescending { b -> b.ticketsCount }

        val queuesList: List<Queue> = mutableListOf<Queue>()
            .also { items ->
                if (showGeneral) {
                    items.add(0, Queue(0, null, summaryTicketsCount, true,false))
                }
                favoriteQueuesList?.forEach { a -> items.add(a) }
                restQueuesList?.forEach { a -> items.add(a) }
                items[selectedQueueIndex].isSelected = true
            }
        return queuesList
    }

    fun onTicketDetailsClicked(id: Long) { _navigateToTicketDetails.value = id }
    fun onTicketDetailsNavigated() { _navigateToTicketDetails.value = null }

    fun onQueueClicked(id: Long, ids: List<Long>?, lockId: Int?, limit: Long?, offset: Long) {
        ticketsList = mutableListOf()
        getTickets(id, ids, lockId, limit, offset)
    }

    private suspend fun getToken(): String? {
        return withContext(Dispatchers.IO) {
            userRepository.getUserInfo()?.sessionValue
        }
    }

    fun logout() {
        viewModelScope.launch {
            withContext(Dispatchers.IO) {
                userRepository.clean()
            }
        }
    }

    override fun onCleared() {
        super.onCleared()
        ticketsList = mutableListOf()
    }
}

QueuesListAdapter.kt

class QueuesListAdapter (val clickListener : QueuesListListener):
    ListAdapter<Queue, QueuesListAdapter.ViewHolder>(DIFF_CALLBACK) {

    companion object {
        private val DIFF_CALLBACK = object : DiffUtil.ItemCallback<Queue>() {

            override fun areItemsTheSame(oldItem: Queue, newItem: Queue): Boolean {
                return oldItem.queueId == newItem.queueId
            }

            override fun areContentsTheSame(oldItem: Queue, newItem: Queue): Boolean {
                return oldItem == newItem
            }
        }
        private var statesMap = HashMap<Int,Boolean>()
    }

    override fun onBindViewHolder(holder: ViewHolder, position: Int) {
        val item = getItem(position)
        setItemView(item, holder.binding)
        holder.bind(item, clickListener)
        item.isSelected = statesMap[position] != null
    }

    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): ViewHolder {
        return ViewHolder.from(parent)
    }

    fun selectItem(position: Int) {
        val item = getItem(position)
        item.isSelected = true
        statesMap.clear()
        statesMap[position] = item.isSelected
        notifyItemChanged(position)
    }

    private fun setItemView(item: Queue, binding: ItemQueueBinding) {
        when (item.isSelected) {
            true -> {
                item.isSelected = false
                binding.queueContent.setBackgroundResource(R.drawable.item_selected_queue_background)
                binding.queueContent.alpha = 1F
            }
            false -> {
                binding.queueContent.setBackgroundResource(R.drawable.item_queue_background)
                if (item.ticketsCount == 0) {
                    binding.queueContent.isEnabled = false
                    binding.queueContent.isFavoriteIcon.isEnabled = false
                    binding.queueContent.alpha = 0.3F
                } else {
                    binding.queueContent.isEnabled = true
                    binding.queueContent.isFavoriteIcon.isEnabled = true
                    binding.queueContent.alpha = 1F
                }
            }
        }
    }

    class ViewHolder private constructor(val binding: ItemQueueBinding): RecyclerView.ViewHolder(
        binding.root
    ) {

        fun bind(item: Queue,  clickListener: QueuesListListener) {
            binding.queues = item
            binding.clickListener = clickListener
        }

        companion object {
            fun from(parent: ViewGroup): ViewHolder {
                val layoutInflater = LayoutInflater.from(parent.context)
                val binding = ItemQueueBinding.inflate(layoutInflater, parent, false)
                return ViewHolder(binding)
            }
        }
    }
}

class QueuesListListener(val clickListener: (queue: Queue) -> Unit) {
    fun onClick(queue: Queue) {
        clickListener(queue)
    }
}

ProfileFragment.kt

class ProfileFragment : BaseFragment<ProfileFragmentBinding>() {

    @Inject lateinit var factory: ProfileViewModelFactory
    @Inject lateinit var viewModel: ProfileViewModel
    private lateinit var profileQueuesListAdapter: ProfileQueuesListAdapter
    private var initialQueuesList = mutableListOf<Queue>()
    private var favorites = mutableMapOf<Long,Boolean>()

    override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
        ComponentsHolder.getComponent().inject(this)
        super.onViewCreated(view, savedInstanceState)

        (activity as MainActivity).showBottomNavigation()
        (activity as MainActivity).getUnreadNotificationsCount()

        viewModel = ViewModelProvider(this, factory)[ProfileViewModel::class.java]
        binding.model = viewModel
        binding.lifecycleOwner = this

        with(viewModel) {
            getUserSettings()

            checkUserSettings.observe(viewLifecycleOwner) {
                it?.let {
                    favoritesSwitchItem.isChecked = it.isFavoritesChecked!!
                    generalQueueSwitchItem.isChecked = it.isGeneralChecked
                }
            }

            loggedOut.observe(viewLifecycleOwner) {
                it?.let {
                    if (mayNavigate()) {
                        findNavController().navigate(
                            ProfileFragmentDirections
                                .actionProfileFragmentToLoginFragment()
                        )
                    }
                }
            }
        }

        with(binding) {
            profileAppBar.toolbar.title = "Профиль"
            logoutButton.setOnClickListener { viewModel.logout() }
            appVersionDescription.text = requireContext().packageManager.getPackageInfo(requireContext().packageName, 0).versionName

            generalQueueSwitchItem.setOnCheckedChangeListener { _, _ ->
                if (generalQueueSwitchItem.isChecked) {
                    viewModel.updateGeneralQueueState(true)
                } else {
                    viewModel.updateGeneralQueueState(false)
                }
            }

            favoritesSwitchItem.setOnCheckedChangeListener { _, _ ->
                if (favoritesSwitchItem.isChecked) {
                    viewModel.updateFavoritesState(true)
                } else {
                    viewModel.updateFavoritesState(false)
                }
            }
        }
    }

    override fun getFragmentBinding(
        inflater: LayoutInflater,
        container: ViewGroup?
    ) = ProfileFragmentBinding.inflate(inflater, container, false)

}

ProfileViewModel.kt

class ProfileViewModel @Inject constructor(
    private val userRepository: UserRepository,
    private val queuesRepository: QueuesRepository
) : BaseViewModel() {

    private var summaryTicketsCount: Int? = 0
    private var addToFavoritesList = mutableListOf<Long>()
    private var removeFromFavoritesList = mutableListOf<Long>()

    private val _status = MutableLiveData<ApiStatus>()
    val status: LiveData<ApiStatus>
        get() = _status

    private val _queues = MutableLiveData<QueuesResponse?>()
    val queues: LiveData<QueuesResponse?>
        get() = _queues

    private val _loggedOut = MutableLiveData<Boolean>()
    val loggedOut : LiveData<Boolean>
        get() = _loggedOut

    private val _checkUserSettings = MutableLiveData<User>()
    val checkUserSettings: LiveData<User>
        get() = _checkUserSettings

    init { }

    fun logout() {
        coroutineScope.launch {
            clean().also { _loggedOut.value = true }
        }
    }

    fun getUserSettings() {
        coroutineScope.launch {
            _checkUserSettings.postValue(retrieveUserSettings())
        }
    }

    private suspend fun retrieveUserSettings(): User? {
        return withContext(Dispatchers.IO) {
            userRepository.getUserInfo()
        }
    }

    fun updateGeneralQueueState(isShouldBeShown: Boolean) {
        _status.value = ApiStatus.LOADING
        coroutineScope.launch {
            updateGeneralQueue(isShouldBeShown)
            _status.value = ApiStatus.DONE
        }
    }

    fun updateFavoritesState(isFavoritesActive: Boolean) {
        _status.value = ApiStatus.LOADING
        coroutineScope.launch {
            updateFavorites(isFavoritesActive)
            _status.value = ApiStatus.DONE
        }
    }

    private suspend fun updateGeneralQueue(isShouldBeShown: Boolean) {
        withContext(Dispatchers.IO) {
            userRepository.updateGeneralQueueState(isShouldBeShown)
        }
    }

    private suspend fun updateFavorites(isFavoritesActive: Boolean) {
        withContext(Dispatchers.IO) {
            userRepository.updateFavoritesState(isFavoritesActive)
        }
    }

    private suspend fun getToken(): String? {
        return withContext(Dispatchers.IO) {
            userRepository.getUserInfo()?.sessionValue
        }
    }

    private suspend fun clean() {
        withContext(Dispatchers.IO) {
            userRepository.clean()
        }
    }
}

CodePudding user response:

I do not know exactly what's happening since this is a lot of code and quite hard to follow line by line; it's very easy to miss something when reading code on SO, but I do see a few things where you could improve your architecture.

Where I'd start is by looking at parts of your architecture that have "code smells" (A word of caution: Most of the code I'll write will be pseudo-code, and I don't have experience with DataBinding (only ViewBinding), as I'm not a fan of what it does and I've always chosen not to use it, so if Databinding is causing an issue, I wouldn't know for sure.)

Architecture

When leveraging the power of coroutines, you'd want to benefit from the ability to use suspend functions, and the reactive nature of LiveData (or Flow) to observe and react in your UI. I won't go too much detail into every topic, but I'll mention potential testability issues when I see them, since you'll want to Unit Test your business logic and to do that, you ought to keep some things in consideration.

In general, you'd want to follow the Jetpack architecture ideas (unless you work for Square, in which case everything must be different because Google is wrong); with that in mind, I'll just adhere to Google recommended practices where applicable because if you don't like it, you can find your own alternatives ;)

Fragments

I see a lot of state in the Fragments. Lots of booleans, integers, lists, etc. This is normally a red flag. You have a ViewModel, that's where your state should be coming from, the Fragment rarely has reasons to "store" this state locally.

ViewModels

I feel like you're using a lot of LiveData, which is fine, but I believe you'd benefit from a step further by replacing most of that by a combined flow. Each of your internal states is instead a Flow, and you expose to the fragment one (combined) or a couple if you want to split parts of your reactive code. By using the combine(flow1, flow2, etc...) function in your VM, you can then produce a single more cohesive state, and even expose it as a StateFlow for even more efficiency, as you'd then observe the flow from your fragment using something like:

viewLifecycleOwner.lifecycleScope.launchWhenStarted { 
    viewModel.yourFlow.collect {...} //or collectLatest, depending on your usecase
}

This is optional, but it would be an improvement over having so many liveDatas floating around.

Fragment - Adapters

I see you have two or three ListView adapters (good), but they don't really need to be lateinit. You're not really adding much, have them created at init:

private val adapter1 = SomeAdapter()
private val adapter2 = AnotherAdater()

Since they are ListAdapters, once you receive data via (livedata/flow) all you should do is adapter1.submitList(...), since they cannot be null ever. By using lateinit (in something you know you're gonna need anyway) you're not really gaining anything, and are introducing complexity. There are better optimizations you can do than lateinit there.

In the end, your fragment should be as dummy as possible. You "load it" when you display it, abide by its crazy lifecycle, and then wire the things up so it can observe a livedata/flow and update its UI with the incoming state, that's all it should do. And navigation of course, but mainly because it's part of the required plumbing you ought to do in the android framework.

If you add more logic/stuff/state, you're putting yourself in a testing corner and a more complex scenario to manage, as fragments are destroyed, detached, re-added, etc.

ListAdapters

Good job using List Adapters, but your adapters have a few issues.

  • Don't call notifyDataSetChanged, it defeats the purpose. Submit list should do the trick, that's why you have a DiffUtil. If this is not working, well, there are other nuisances with ListAdapter you may need to be aware of, but once you get past those, you should be good to go.

Take for instance this snippet of your code:

    fun selectItem(position: Int) {
        val item = getItem(position)
        item.isSelected = true
        statesMap.clear()
        statesMap[position] = item.isSelected
        notifyItemChanged(position)
    }

Why do you have a static hashMap to indicate selection?

The Adapter already has a lot of work to do behind the scenes, it shouldn't have this responsibility.

When something is selected, you do something about it (set some boolean to true like yourItem.isSelected = true for e.g.) and then produce a new list that will be submitted to the adapter and the diffutil will pick the change.

(this is just an example of an operation that mutates your list, it could be something else, but the principle is, don't keep state where it doesn't belong, instead, react to changes received via the expected channels).

ViewHolders/Adapter

This doesn't look bad, but I feel you're not delegating your responsibilities correctly. Your adapter should not have a lot of if statements there. If an item is selected, the ViewHolder should receive this information and act accordingly, not the Adapter. So I'd pass a boolean to your fun bind alongside all the info the ViewHolder needs to set its correct appearance. This is all read-only info anyway.

Coroutine Scope/Dispatchers

Careful with hardcoding Dispatchers.IO all over the place, this makes it impossible to correctly test, as you cannot override the dispatcher that easily. Instead, inject it, since you're using Dagger already. This way your tests will be able to override them.

In the viewModel always do

viewModelScope.launch(injected_dispatcher) {
   //call your suspend functions and such
   val result = someRepo.getSomething()
   someFlow.emit(result)
}

(just an example).

When you test your VM, you'll supply a test dispatcher.

Conclusion

Overall, good job on the architecture, it's better than a huge activity doing all the work ;)

I feel like you could simplify your code a bit, which, in turn, will greatly help you in finding what part is not behaving as expected.

Remember. Activity/Fragment Observes ViewModel, and deals with Android Framework things (as Google calls them "policy delegates") like navigation, intents, etc. They react to data received and pass it along (to an Adapter for e.g.). A viewModel sits between your source of truth (repos, data layers) and your business logic (split in usecases/interactors or whatever you call them). The ViewModel is there to give your fragile Fragments/Activities, a more stable and longer-living component that will glue your data with your UI.

The Repos/Etc. are all suspend functions that return the data you need from the source, and update it when needed. (e.g. talk to Room DB or an API, or both!) They merely return the data for the VM and higher levels to consume.

UseCase/Interactors are just abstractions to "reuse" the communication between viewmodels and repositories and such (or to some specific logic). They can apply transformations to your data as they see fit, liberating the VM from this resposibility.

E.g. if you have a GetSomethingUseCase, that may, behind the scenes, talk to a repo, wait (suspend), then transform the API/DB Response into something that is needed by the UI, all done without the VM (or the UI) knowing what's going on.

And lastly, make your adapters as small as possible. Remember they already have a lot of responsibilities.

The way I see this working is.

Fragment 1 Starts, VM is init. Fragment observes its state via some livedata/flow when started. Fragment mutates its views to match the state received.

The user goes to Fragment 2 and changes something, this 'something' updates a DB or in-memory data structure.

The user returns to Fragment 1, all is init again or restored, but this time, the liveData/Flow is observed again, and the data comes back (now modified by fragment2). The Fragment updates its UI without thinking much about it, as it's not its responsibility.

I hope this lengthy answer points you in the right direction.

Short of that, I suggest you break down your problem into a smaller one to try to isolate what is not doing what it should. The less "state" you have in random places (Adapters, fragments, etc.) the less the chances of weird problems you're going to have.

Don't fight the framework ;)

Lastly, if you made it this far, if you submit a list to your adapter and it doesn't update, take a look at this SO question/answer as ListAdapter has a "it's not a bug according to google but the documentation doesn't make this clear enough" situation.

  • Related