I've tried searching stackoverflow, but couldn't find the answer to my issue. When the search button is clicked, I want the app to display the data from the API. The issue I'm having is that it is taking 2 clicks of the search button in order to display the data. The first click displays "null" and the second click displays all data correctly. What am I doing wrong? What do I need to change in order to process correctly on the first click? Thanks in advance!

Pairing Fragment

package com.example.winepairing.view.fragments

import android.os.Bundle
import androidx.fragment.app.Fragment
import android.view.LayoutInflater
import android.view.View
import android.view.ViewGroup
import android.widget.Toast
import androidx.appcompat.app.AppCompatActivity
import androidx.fragment.app.activityViewModels
import com.example.winepairing.databinding.FragmentPairingBinding
import com.example.winepairing.utils.hideKeyboard
import com.example.winepairing.viewmodel.PairingsViewModel



class PairingFragment : Fragment() {
    private var _binding: FragmentPairingBinding? = null
    private val binding get() = _binding!!
    private val viewModel: PairingsViewModel by activityViewModels()


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

        val toolbar = binding.toolbar
        (activity as AppCompatActivity).setSupportActionBar(toolbar)

        binding.searchBtn.setOnClickListener {
            hideKeyboard()
            if (binding.userItem.text.isNullOrEmpty()) {
                Toast.makeText([email protected](),
                    "Please enter a food, entree, or cuisine",
                    Toast.LENGTH_SHORT).show()
            } else {
                val foodItem = binding.userItem.text.toString()
                getWinePairing(foodItem)
                pairedWinesList()
                pairingInfo()
            }
        }
        return view
    }

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

    private fun pairedWinesList() {
        val pairedWines = viewModel.apiResponse.value?.pairedWines
        var content = ""
        if (pairedWines != null) {
            for (i in 0 until pairedWines.size) {
                //Append all the values to a string
                content  = pairedWines.get(i)
                content  = "\n"
            }
        }
        binding.pairingWines.setText(content)
    }

    private fun pairingInfo() {
        val pairingInfo = viewModel.apiResponse.value?.pairingText.toString()
        binding.pairingInfo.setText(pairingInfo)
    }

    private fun getWinePairing(foodItem: String) {
        viewModel.getWinePairings(foodItem.lowercase())

    }
}

So, sorry!!! Here is the viewmodel

package com.example.winepairing.viewmodel

import androidx.lifecycle.LiveData
import androidx.lifecycle.MutableLiveData
import androidx.lifecycle.ViewModel
import androidx.lifecycle.viewModelScope
import com.example.winepairing.BuildConfig
import com.example.winepairing.model.data.Wine
import com.example.winepairing.model.network.WineApi
import kotlinx.coroutines.launch


const val CLIENT_ID = BuildConfig.SPOONACULAR_ACCESS_KEY
class PairingsViewModel: ViewModel() {
    private val _apiResponse = MutableLiveData<Wine>()
    val apiResponse: LiveData<Wine> = _apiResponse

    fun getWinePairings(food: String) {
        viewModelScope.launch {
            _apiResponse.value = WineApi.retrofitService.getWinePairing(food, CLIENT_ID)
        }
    }
}

CodePudding user response：

You haven't posted your actual code for fetching data from the API (probably in viewModel#getWinePairings), but at a guess it's going like this in your button click listener:

• you call getWinePairing - this kicks off an async call that will eventually complete and set data on viewModel.apiResponse, sometime in the future. It's initial value is null
• you call pairedWinesList which references the current value of apiResponse - this is gonna be null until an API call sets a value on it. Since you're basically fetching the most recent completed search result, if you change your search data then you'll end up displaying the results of the previous search, while the API call runs in the background and updates apiResponse later
• you call pairingInfo() which is the same as above, you're looking at a stale value before the API call returns with the new results

The problem here is you're doing async calls that take a while to complete, but you're trying to display the results immediately by reading the current value of your apiResponse LiveData. You shouldn't be doing that, you should be using a more reactive design that observes the LiveData, and updates your UI when something happens (i.e. when you get new results):

// in onCreateView, set everything up

viewModel.apiResponse.observe(viewLifeCycleOwner) { response ->
    // this is called when a new 'response' comes in, so you can
    // react to that by updating your UI as appropriate
    binding.pairingInfo.setText(response.pairingInfo.toString())
    // this is a way you can combine all your wine strings FYI
    val wines = response.pairedWines?.joinToString(separator="\n") ?: ""
    binding.pairingWines.setText(wines)
}
        
binding.searchBtn.setOnClickListener {
    ...
    } else {
        val foodItem = binding.userItem.text.toString()
        // kick off the API request - we don't display anything here, the observing
        // function above handles that when we get the results back
        getWinePairing(foodItem)
    }
}

Hopefully that makes sense - the button click listener just starts the async fetch operation (which could be a network call, or a slow database call, or a fast in-memory fetch that wouldn't block the thread - the fragment doesn't need to know the details!) and the observe function handles displaying the new state in the UI, whenever it arrives.

The advantage is you're separating everything out - the viewmodel handles state, the UI just handles things like clicks (updating the viewmodel) and displaying the new state (reacting to changes in the viewmodel). That way you can also do things like have the VM save its own state, and when it initialises, the UI will just react to that change and display it automatically. It doesn't need to know what caused that change, y'know?

CodePudding user response：

Although you didn't share your ViewModel code, I'm guessing that your ViewModel's getWinePairings() function retrieves data asynchronously from an API and then updates a LiveData called apiResponse with the return value. Since the API response takes some time before it returns, your apiResponse LiveData is going to still be empty by the time you call the Fragment's pairedWinesList() function from the click listener.

Hint, any time you use the .value of a LiveData outside of the ViewModel that manages it, you are probably doing something wrong. The point of LiveData is to react to it's data when it arrives, so you should be calling observe() on it instead of trying to read its .value synchronously.

More information in this question about asynchronous calls.