Search code examples
androidkotlinandroid-recyclerviewkotlin-coroutinesmutablelivedata

Problem Implement Update UI with LiveData, Retrofit, Coroutine on Recyclerview : adapter Recyclerview not update


I'm newbie use Kotlin on my dev apps android, and now, I on step learn to implement Update UI with LiveData, Retrofit, Coroutine on Recyclerview. The My Apps:

MainActivity > MainFragment with 3 Tab fragment > HomeFragment, DashboardFragment, and SettingsFragment

I call function to get data from server on onCreateView HomeFragment, and observe this with show shimmer data on my Recylerview when is loading, try update RecyclerView when success, and show view Failed Load -button refresh when error.

The problem is:

  • Adapter Recyclerview not Update when success get Data from Server. Adapter still show shimmer data
  • With case error (no Internet), i show view Failed Load, with button refresh. Tap to refresh, i re-call function to get data server, but fuction not work correct. Recyclerview show last data, not show Failed Load again.

Bellow my code

HomeFragment


private var _binding: FragmentHomeBinding? = null
private val binding get() = _binding!!
private lateinit var adapterNews: NewsAdapter
private var shimmerNews: Boolean = false
private var itemsDataNews = ArrayList<NewsModel>()

override fun onCreateView(
        inflater: LayoutInflater, container: ViewGroup?,
        savedInstanceState: Bundle?
    ): View {
// Inflate the layout for this fragment
_binding = FragmentHomeBinding.inflate(inflater, container, false)
......

newsViewModel = ViewModelProvider(requireActivity()).get(NewsViewModel::class.java)
//news
binding.frameNews.rvNews.setHasFixedSize(true)
binding.frameNews.rvNews.layoutManager = llmh
adapterNews = NewsAdapter(itemsDataNews, shimmerNews)
binding.frameNews.rvNews.adapter = adapterNews
// Observe

//get News
newsViewModel.refresh()

newsViewModel.newsList.observe(
            viewLifecycleOwner,
            androidx.lifecycle.Observer { newsList ->
                newsList?.let {
                    binding.frameNews.rvNews.visibility = View.VISIBLE
                    binding.frameNews.rvNews.isNestedScrollingEnabled = true
                    binding.frameNews.itemNewsLayoutFailed.visibility = View.GONE
                    if (it.size == 0)
                        binding.frameNews.root.visibility = View.GONE
                    else
                        getDataNews(it)
                }
            })
        newsViewModel.loading.observe(viewLifecycleOwner) { isLoading ->
            isLoading?.let {
                binding.frameNews.rvNews.visibility = View.VISIBLE
                binding.frameNews.rvNews.isNestedScrollingEnabled = false
                binding.frameNews.itemNewsLayoutFailed.visibility = View.GONE
                getDataNewsShimmer()
            }
        }

        newsViewModel.loadError.observe(viewLifecycleOwner) { isError ->
            isError?.let {
                binding.frameNews.rvNews.visibility = View.INVISIBLE
                binding.frameNews.itemNewsLayoutFailed.visibility = View.VISIBLE
                binding.frameNews.btnNewsFailed.setOnClickListener {
                    newsViewModel.refresh()
                }

            }
        }
....

return binding.root
}

@SuppressLint("NotifyDataSetChanged")
    private fun getDataNewsShimmer() {
        shimmerNews = true
        itemsDataNews.clear()
        itemsDataNews.addAll(NewsData.itemsShimmer)
        adapterNews.notifyDataSetChanged()
    }

    @SuppressLint("NotifyDataSetChanged")
    private fun getDataNews(list: List<NewsModel>) {
        Toast.makeText(requireContext(), list.size.toString(), Toast.LENGTH_SHORT).show()
        shimmerNews = false
        itemsDataNews.clear()
        itemsDataNews.addAll(list)
        adapterNews.notifyDataSetChanged()
    }

override fun onDestroyView() {
        super.onDestroyView()
        _binding=null
    }

NewsViewModel

class NewsViewModel: ViewModel() {

    val newsService = KopraMobileService().getNewsApi()
    var job: Job? = null
    val exceptionHandler = CoroutineExceptionHandler { _, throwable ->
        onError("Exception handled: ${throwable.localizedMessage}")
    }

    val newsList = MutableLiveData<List<NewsModel>>()
    val loadError = MutableLiveData<String?>()
    val loading = MutableLiveData<Boolean>()

    fun refresh() {
        fetchNews()
    }

    private fun fetchNews() {
        loading.postValue(true)
        job = CoroutineScope(Dispatchers.IO + exceptionHandler).launch {
            val response = newsService.getNewsList()
            withContext(Dispatchers.Main) {
                if (response.isSuccessful) {
                    newsList.postValue(response.body()?.data)
                    loadError.postValue(null)
                    loading.postValue(false)
                } else {
                    onError("Error : ${response.message()} ")
                }
            }
        }
        loadError.postValue("")
        loading.postValue( false)
    }

    private fun onError(message: String) {
        loadError.postValue(message)
        loading.postValue( false)
    }

    override fun onCleared() {
        super.onCleared()
        job?.cancel()
    }

}

NewsAdapter

NewsAdapter(
var itemsCells: List<NewsModel?>  = emptyList(),
    var shimmer: Boolean ,
) :
    RecyclerView.Adapter<ViewHolder>() {

    private lateinit var context: Context

    override fun onCreateViewHolder(parent: ViewGroup, viewType: Int): NewsHomeViewHolder {
        context = parent.context
        return NewsHomeViewHolder(
                NewsItemHomeBinding.inflate(LayoutInflater.from(parent.context), parent, false)
            )
        

    }


    override fun onBindViewHolder(holder: NewsHomeViewHolder, position: Int) {
     
            holder.bind(itemsCells[position]!!)
        
    }

    inner class NewsHomeViewHolder(private val binding: NewsItemHomeBinding) :
        ViewHolder(binding.root) {
        fun bind(newsItem: NewsModel) {
          
            binding.newsItemFlat.newsTitle.text = newsItem.name
            binding.newsItemFlatShimmer.newsTitle.text = newsItem.name

            binding.newsItemFlat.newsSummary.text = newsItem.name
            binding.newsItemFlatShimmer.newsSummary.text = newsItem.name

            if (shimmer) {
                binding.layoutNewsItemFlat.visibility = View.INVISIBLE
                binding.layoutNewsItemFlatShimmer.visibility = View.VISIBLE
                binding.layoutNewsItemFlatShimmer.startShimmer()
            } else {

                binding.layoutNewsItemFlat.visibility = View.VISIBLE
                binding.layoutNewsItemFlatShimmer.visibility = View.INVISIBLE
            }
        }
    }

    override fun getItemCount(): Int {
        return itemsCells.size
    }

I hope someone can help me to solve the problem. thanks, sorry for my English.


Solution

  • You have 3 different LiveDatas, right? One with a list of news data, one with a loading error message, and one with a loading state.

    You set up an Observer for each of these, and that observer function gets called whenever the LiveData's value updates. That's important, because here's what happens when your request succeeds, and you get some new data:

    if (response.isSuccessful) {
        newsList.postValue(response.body()?.data)
        loadError.postValue(null)
        loading.postValue(false)
    }
    

    which means newsList updates, then loadError updates, then loading updates.

    So your observer functions for each of those LiveDatas run, in that order. But you're not testing the new values (except for null checks), so the code in each one always runs when the value updates. So when your response is successful, this happens:

    • newsList observer runs, displays as successful, calls getDataNews
    • loadError observer runs, value is null so nothing happens
    • loading observer runs, value is false but isn't checked, displays as loading, calls getDataNewsShimmer

    So even when the response is successful, the last thing you do is display the loading state


    You need to check the state (like loading) before you try to display it. But if you check that loading is true, you'll have a bug in fetchNews - that sets loading = true, starts a coroutine that finishes later, and then immediately sets loading = false.

    I'd recommend trying to create a class that represents different states, with a single LiveData that represents the current state. Something like this:

    // a class representing the different states, and any data they need
    sealed class State {
        object Loading : State()
        data class Success(val newsList: List<NewsModel>?) : State()
        data class Error(val message: String) : State()
    }
    
    // this is just a way to keep the mutable LiveData private, so it can't be updated
    private val _state = MutableLiveData<State>()
    val state: LiveData<State> get() = _state
    
    private fun fetchNews() {
        // initial state is Loading, until we get a response
        _state.value = State.Loading
        job = CoroutineScope(Dispatchers.IO + exceptionHandler).launch {
            val response = newsService.getNewsList()
    
            // if you're using postValue I don't think you need to switch to Dispatchers.Main?
            _state.postValue(
                // when you get a response, the state is now either Success or Error
                if (response.isSuccessful) State.Success(response.body()?.data)
                else State.Error("Error : ${response.message()} ")
            )
        }
    }
    

    And then you just need to observe that state:

    // you don't need to create an Observer object, you can use a lambda!
    newsViewModel.state.observe(viewLifecycleOwner) { state ->
        // Handle the different possible states, and display the current one
    
        // this lets us avoid repeating 'binding.frameNews' before everything
        with(binding.frameNews) {
            // You could use a when block, and describe each state explicitly,
            // like your current setup:
            when(state) {
                // just checking equality because Loading is a -singleton object instance-
                State.Loading -> {
                    rvNews.visibility = View.VISIBLE
                    rvNews.isNestedScrollingEnabled = false
                    itemNewsLayoutFailed.visibility = View.GONE
                    getDataNewsShimmer()
                }
                // Error and Success are both -classes- so we need to check their type with 'is'
                is State.Error -> {
                    rvNews.visibility = View.INVISIBLE
                    itemNewsLayoutFailed.visibility = View.VISIBLE
                    btnNewsFailed.setOnClickListener {
                        newsViewModel.refresh()
                    }
                }
                is State.Success -> {
                    rvNews.visibility = View.VISIBLE
                    rvNews.isNestedScrollingEnabled = true
                    itemNewsLayoutFailed.visibility = View.GONE
    
                    // Because we know state is a Success, we can access newsList on it
                    // newsList can be null - I don't know how you want to handle that,
                    // I'm just treating it as defaulting to size == 0
                    // (make sure you make this visible when necessary too)
                    if (state.newsList?.size ?: 0 == 0) root.visibility = View.GONE
                    else getDataNews(state.newsList)
                }
            }
    
            // or, if you like, you could do this kind of thing instead:
            itemNewsLayoutFailed.visibility = if (state is Error) VISIBLE else GONE
        }
    }
    

    You also might want to break that display code out into separate functions (like showError(), showList(state.newsList) etc) and call those from the branches of the when, if that makes it more readable

    I hope that makes sense! When you have a single value representing a state, it's a lot easier to work with - set the current state as things change, and make your observer handle each possible UI state by updating the display. When it's Loading, make it look like this. When there's an Error, make it look like this

    That should help avoid bugs where you update multiple times for multiple things, trying to coordinate everything. I'm not sure why you're seeing that problem when you reload after an error, but doing this might help fix it (or make it easier to see what's causing it)