0

I am learning android development and I decided to build a weather app using api that comes from service named open water map. Unfortunately I’ve got the following problem:

In order to get the weather data for wanted city, I first need to perform request to get the geographical coordinates. So what I need to do is to create one request, wait until it is finished, and after that do another request with data that has been received from the first one.

This is how my view model for location looks like:

class LocationViewModel constructor(private val repository: WeatherRepository): ViewModel() {

    val location = MutableLiveData<List<GeocodingModel>>()

    private val API_KEY = „xxxxxxxxxxxxxxxxxxxxxxxxx”


    fun refresh() {
        CoroutineScope(Dispatchers.IO).launch {
        // call fetch location here in coroutine
        }
    }

    private suspend fun fetchLocation(): Response<GeocodingModel> {
        return repository.getCoordinates(
                "Szczecin",
                API_KEY
            )
    }
}

And this is how my view model for weather looks like”

class WeatherSharedViewModel constructor(private val repository: WeatherRepository): ViewModel() {

    private val API_KEY = „xxxxxxxxxxxxxxxxxxxxxxxxx”

    val weather = MutableLiveData<List<SharedWeatherModel>>()

    val weatherLoadError = MutableLiveData<Boolean>()

    val loading = MutableLiveData<Boolean>()


    fun refresh(lat: String, lon: String) {
        loading.value = true

        CoroutineScope(Dispatchers.IO).launch {
        // call fetchWeather here in coroutine
        }

        loading.value = false
    }

    private suspend fun fetchWeather(lat: String, lon: String): Response<SharedWeatherModel> {
                return repository.getWeather(
                    lat,
                    lon,
                    "minutely,hourly,alerts",
                    "metric",
                    API_KEY
                )
    }
}

I am using both view models in a fragment in such way:

override fun onActivityCreated(savedInstanceState: Bundle?) {
        super.onActivityCreated(savedInstanceState)
        val weatherService = WeatherApi.getInstance()
        val repository = WeatherRepository(weatherService)
        locationViewModel = ViewModelProvider(requireActivity(), ViewModelFactory(repository)).get(LocationViewModel::class.java)
        weatherViewModel = ViewModelProvider(requireActivity(), ViewModelFactory(repository)).get(WeatherSharedViewModel::class.java)

        locationViewModel.refresh()
        Log.d(TAG, "lat: ${locationViewModel.location.value?.get(0)?.get(0)?.lat.toString()}, lon: ${locationViewModel.location.value?.get(0)?.get(0)?.lon.toString()}")
        weatherViewModel.refresh(
            locationViewModel.location.value?.get(0)?.get(0)?.lat.toString(),
            locationViewModel.location.value?.get(0)?.get(0)?.lon.toString()
        )

        val weatherList = view?.findViewById<RecyclerView>(R.id.currentWeatherList)
        weatherList?.apply {
            layoutManager = LinearLayoutManager(context)
            adapter = currentWeatherAdapter
        }

        val cityList = view?.findViewById<RecyclerView>(R.id.currentCityList)
        cityList?.apply {
            layoutManager = LinearLayoutManager(context)
            adapter = currentLocationAdapter
        }

        observerLocationViewModel()
        observeWeatherViewModel()
    }

So on a startup both models are refreshed, which means that requests are made. I was trying to somehow synchronize those calls but my last attempt ended that data passed to the refresh method of weather view model was null. So problem is that both coroutine are launched one after another, first one is not waiting for second.

The main question: is there any synchronisation mechanism in coroutines? That I can launch one coroutine and wait with launching second one as long as first is not finished?

gawron103
  • 167
  • 6
  • 21

3 Answers3

0

You are violating the "Single Responsibilty Principle" you need to learn how to write CLEAN code. that is why you are running into such problems. A member of stackoverflow has explained it in depth: single responsibility

king jaja
  • 21
  • 5
0

A few tips:

  • Your general design is somewhat convoluted because you are trying to update LiveData with coroutines, but one LiveData's exposed data is something determined by the other LiveData. This is theoretically OK if you need to be able to access the city even after you already have the weather for that city, but since you've split this behavior between two ViewModels, you end up having to manage that interaction externally with your Fragment, which is very messy. You cannot control it from a single coroutine unless you use the fragment's lifecycle scope, but then the fetch tasks restart if the screen rotates before they're done. So I would use a single ViewModel for this.
  • In a ViewModel, you should use viewModelScope for your coroutines instead of creating an ad hoc CoroutineScope that you never cancel. viewModelScope will automatically cancel your coroutines when the ViewModel goes out of scope.
  • Coroutines make it extremely easy to sequentially do background work. You just need to call suspend functions in sequence within a single coroutine. But to do that, once again, you really need a single ViewModel.
  • It's convoluted to have separate LiveDatas for the loading and error states. If you use a sealed class wrapper, it will be much simpler for the Fragment to treat the three possible states (loading, error, have data).

Putting this together gives the following. I don't really know what your repo is doing and how you convert Response<GeocodingModel> to List<GeocodingModel> (or why), so I am just using a placeholder function for that. Same for the weather.

sealed class WeatherState {
    object Loading: WeatherState()
    object Error: WetaherState()
    data class LoadedData(val data: List<SharedWeatherModel>)
}

class WeatherViewModel constructor(private val repository: WeatherRepository): ViewModel() {

    val location = MutableLiveData<List<GeocodingModel>>()

    private val API_KEY = „xxxxxxxxxxxxxxxxxxxxxxxxx”

    val weather = MutableLiveData<LoadedData>().apply {
            value = WeatherState.Loading
        }

    fun refreshLocation() = viewModelScope.launch {
        weather.value = WeatherState.Loading
        val locationResponse = fetchLocation() //Response<GeocodingModel>
        val locationList = unwrapLocation(location) //List<GeocodingModel>
        location.value = locationList
        val latitude = locationList.get(0).get(0).lat.toString()
        val longitude = locationList.get(0).get(0).lon.toString()
        try {
            val weatherResponse = fetchWeather(latitude, longitude) //Response<SharedWeatherModel>
            val weatherList = unwrapWeather(weatherResponse) //List<SharedWeatherModel>
            weather.value = WeatherState.LoadedData(weatherList)
        } catch (e: Exception) {
            weather.value = WeatherState.Error
        }        
    }

    private suspend fun fetchLocation(): Response<GeocodingModel> {
        return repository.getCoordinates(
                "Szczecin",
                API_KEY
            )
    }

    private suspend fun fetchWeather(lat: String, lon: String): Response<SharedWeatherModel> {
                return repository.getWeather(
                    lat,
                    lon,
                    "minutely,hourly,alerts",
                    "metric",
                    API_KEY
                )
    }
}

And in your Fragment you can observe either LiveData. The weather live data will always have one of the three states, so you have only one place where you can use a when statement to handle the three possible ways your UI should look.

Tenfour04
  • 83,111
  • 11
  • 94
  • 154
0

Without referring to your actual code only to the question itself:

By default code inside coroutines is sequential.

scope.launch(Dispatcher.IO) {
    val coordinates = repository.getCoordinates(place)
    val forecast = repository.getForecast(coordinates)
}

Both getCoordinates(place) and getForecast(coordinates) are suspend functions since they're making network requests and waiting for the result.

getForecast(coordinates) won't execute until getCoordinates(place) is done and returned the coordinates.

J.Grbo
  • 435
  • 5
  • 11