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(this@PairingFragment.requireActivity(),
"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)
}
}
}
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:
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
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
laterpairingInfo()
which is the same as above, you're looking at a stale value before the API call returns with the new resultsThe 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 observe
s 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?