sandorb
sandorb

Reputation: 687

Jetpack compose CircularProgressIndicator lag on api fetch

I want to display a loading indicator when I download data from the API. However, when this happens, the indicator often stops. How can I change this or what could be wrong? Basically, I fetch departure times and process them (E.g. I convert hex colors to Jetpack Compose color, or unix dates to Date type, etc.) and then load them into a list and display them.

enter image description here

@Composable
fun StopScreen(
    unixDate: Long? = null,
    stopId: String,
    viewModel: MainViewModel = hiltViewModel()
) {
    LaunchedEffect(Unit) {
        viewModel.getArrivalsAndDeparturesForStop(
            unixDate,
            stopId,
            false
        )
    }
    val isLoading by remember { viewModel.isLoading }
    if (!isLoading) {
        //showData
    } else {
        LoadingView()
    }
}


@Composable
fun LoadingView() {
    Box(
        contentAlignment = Alignment.Center,
        modifier = Modifier.fillMaxSize()
    ) {
        CircularProgressIndicator(color = MaterialTheme.colors.primary)
    }
}

And the viewmodel where I process the data:

@HiltViewModel
class MainViewModel @Inject constructor(
    private val mainRepository: MainRepository
) : ViewModel() {
    var stopTimesList = mutableStateOf<MutableList<StopTime>>(arrayListOf())
    var alertsList = mutableStateOf<MutableList<Alert>>(arrayListOf())
    var loadError = mutableStateOf("")
    var isLoading = mutableStateOf(false)
    var isRefreshing = mutableStateOf(false)
    fun getArrivalsAndDeparturesForStop(unixDate: Long? = null, stopId: String, refresh: Boolean) {
        viewModelScope.launch {
            if (refresh) {
                isRefreshing.value = true
            } else {
                isLoading.value = true
            }
            val result = mainRepository.getArrivalsAndDeparturesForStop(stopId = stopId, time = unixDate)
            when (result) {
                is Resource.Success -> {
                    //I temporarily store the data here, so that the screen is only refreshed on reload when all the new data has arrived and loaded 
                    var preStopTimes: MutableList<StopTime> = arrayListOf()
                    var preAlertsList: MutableList<Alert> = arrayListOf()
                    if (result.data!!.stopTimes != null && result.data!!.alerts != null) {
                        var count = 0
                        val countAll =
                            result.data!!.stopTimes!!.count() + result.data!!.alertIds!!.count()
                        if (countAll == 0) {
                            loadError.value = ""
                            isLoading.value = false
                            isRefreshing.value = false
                        }
                        //ALERTS
                        for (alert in result.data!!.data.alerts) {
                            preAlertsList.add(alert)
                            count += 1
                            if (count == countAll) {
                                stopTimesList.value = preStopTimes
                                alertsList.value = preAlertsList
                                loadError.value = ""
                                isLoading.value = false
                                isRefreshing.value = false
                            }
                        }
                        for (stopTime in result.data!!.stopTimes!!) {
                            preStopTimes.add(stopTime)
                            count += 1
                            if (count == countAll) {
                                stopTimesList.value = preStopTimes
                                alertsList.value = preAlertsList
                                loadError.value = ""
                                isLoading.value = false
                                isRefreshing.value = false
                            }
                        }
                    } else {
                        loadError.value = "Error"
                        isLoading.value = false
                        isRefreshing.value = false
                    }
                }
                is Resource.Error -> {
                    loadError.value = result.message!!
                    isLoading.value = false
                    isRefreshing.value = false
                }
            }
        }
    }
}

Repository:

@ActivityScoped
class MainRepository @Inject constructor(
    private val api: MainApi
) {
    suspend fun getArrivalsAndDeparturesForStop(stopId: String,time: Long? = null): Resource<ArrivalsAndDeparturesForStop> {
        val response = try {
            api.getArrivalsAndDeparturesForStop(
                stopId,
                time
            )
        } catch (e: Exception) { return Resource.Error(e.message!!)}
        return Resource.Success(response)
    }
}

Upvotes: 2

Views: 2040

Answers (2)

sandorb
sandorb

Reputation: 687

After 6 months I figured out the exact solution. Everything was running on the main thread when I processed the data in the ViewModel. Looking into things further, I should have used Dispatchers.Default and / or Dispatchers.IO within the functions for CPU intensive / list soring / JSON parsing tasks.

https://developer.android.com/kotlin/coroutines/coroutines-adv

suspend fun doSmg() {
            withContext(Dispatchers.IO) {
                //This dispatcher is optimized to perform disk or network I/O outside of the main thread. Examples include using the Room component, reading from or writing to files, and running any network operations.
            }
            withContext(Dispatchers.Default) {
                //This dispatcher is optimized to perform CPU-intensive work outside of the main thread. Example use cases include sorting a list and parsing JSON.
            }
    }

Upvotes: 3

Stephen Vinouze
Stephen Vinouze

Reputation: 2065

My take is that your Composable recomposes way too often. Since you're updating your state within your for loops. Otherwise, it might be because your suspend method in your MainRepository is not dispatched in the right thread.

I feel you didn't yet grasp how Compose works internally (and that's fine, it's a new topic anyway). I'd recommend hoisting a unique state instead of having several mutable states for all your properties. Then build it internally in your VM to then notify the view when the state changes.

Something like this:

data class YourViewState(
    val stopTimesList: List<StopTime> = emptyList(),
    val alertsList: List<Alert> = emptyList(),
    val isLoading: Boolean = false,
    val isRefreshing: Boolean = false,
    val loadError: String? = null,
)

@HiltViewModel
class MainViewModel @Inject constructor(
    private val mainRepository: MainRepository
) : ViewModel() {

    var viewState by mutableStateOf<YourViewState>(YourViewState())

    fun getArrivalsAndDeparturesForStop(unixDate: Long? = null, stopId: String, refresh: Boolean) {
        viewModelScope.launch {
            viewState = if (refresh) {
                viewState.copy(isRefreshing = true)
            } else {
                viewState.copy(isLoading = true)
            }

            when (val result = mainRepository.getArrivalsAndDeparturesForStop(stopId = stopId, time = unixDate)) {
                is Resource.Success -> {
                    //I temporarily store the data here, so that the screen is only refreshed on reload when all the new data has arrived and loaded
                    val preStopTimes: MutableList<StopTime> = arrayListOf()
                    val preAlertsList: MutableList<Alert> = arrayListOf()
                    if (result.data!!.stopTimes != null && result.data!!.alerts != null) {
                        var count = 0
                        val countAll = result.data!!.stopTimes!!.count() + result.data!!.alertIds!!.count()
                        if (countAll == 0) {
                            viewState = viewState.copy(isLoading = false, isRefreshing = false)
                        }
                        //ALERTS
                        for (alert in result.data!!.data.alerts) {
                            preAlertsList.add(alert)
                            count += 1
                            if (count == countAll) {
                                break
                            }
                        }
                        for (stopTime in result.data!!.stopTimes!!) {
                            preStopTimes.add(stopTime)
                            count += 1
                            if (count == countAll) {
                                break
                            }
                        }
                        viewState = viewState.copy(isLoading = false, isRefreshing = false, stopTimesList = preStopTimes, alertsList = preAlertsList)
                    } else {
                        viewState = viewState.copy(isLoading = false, isRefreshing = false, loadError = "Error")
                    }
                }
                is Resource.Error -> {
                    viewState = viewState.copy(isLoading = false, isRefreshing = false, loadError = result.message!!)
                }
            }
        }
    }
}

@Composable
fun StopScreen(
    unixDate: Long? = null,
    stopId: String,
    viewModel: MainViewModel = hiltViewModel()
) {
    LaunchedEffect(Unit) {
        viewModel.getArrivalsAndDeparturesForStop(
            unixDate,
            stopId,
            false
        )
    }
    if (viewModel.viewState.isLoading) {
        LoadingView()
    } else {
        //showData
    }
}

Note that I've made a few improvements while keeping the original structure.

EDIT:

You need to make your suspend method from your MainRepository main-safe. It's likely it runs on the main thread (caller thread) because you don't specify on which dispatcher the coroutine runs.

suspend fun getArrivalsAndDeparturesForStop(stopId: String,time: Long? = null): Resource<ArrivalsAndDeparturesForStop> = withContext(Dispatchers.IO) {
        try {
            api.getArrivalsAndDeparturesForStop(
                stopId,
                time
            )
            Resource.Success(response)
        } catch (e: Exception) {
            Resource.Error(e.message!!)
        }

Upvotes: 1

Related Questions