https://kotlinlang.org logo
Join Slack
Powered by
is it a good practice to use a `while(true)` in `v...
# compose
p
is it a good practice to use a 
while(true)
in 
viewmodel
init block (inside a 
viewModelScope.launch
corutine) for updating the uiState values each 100ms? I need to do it each 100ms because it's data that is being varied very fast. The issue is that I'm not completly sure if that while(true) is attached to viewmodel lifecicle and will die when the viewmodel dies, for example. The values required on the UiState doesn't come from a flow, so I tryed doing this while true with a delay of 100ms as a simple way of update them.
🚫 2
p
IMHO 
while (true) {...}
is never a good practice, no exception
☝🏼 1
Copy code
While (isActive) {}
Semantically cleaner
p
how whould you define if isActive?
c
p
The coroutine scope has one but I meant to say as a general concept is good to control infinite loops with some mechanism.
But in this case that you have a coroutine you can use isActive. If you use delay() it checks for isActive internally but I still think it is a good practice to check explicitly
p
yes, true, I like that
better to explicitly use isActive
without delay, whould the while(true) not be stopped if the viewmodelscope is cancelled when the viewmodel is closed?
p
You can also have your own 
var isActive = true
And set it to isActive = false in onClear but that's normally not needed
Without delay or anything that checks for isActive the coroutine will never be cancelled
p
I thought that if the scope (viewmodelscope) is cancelled, the coroutines should be canceled
but now I see that not as easy
p
Not always, if the cancellation signal is sent but the coroutine is so busy that it never checks for it, then it will never cancel
p
good to know
thanks
👍 1
m
s
Doing anything on the init{} block of your ViewModel is most likely a bad idea by itself though. Especially launching infinitely running coroutines. Those will run the entire time for all the ViewModels that belong to any screen in the backstack and won't stop running when the screen is no longer resumed
https://x.com/ianhlake/status/1836802135096537482 An example of 3 things you should not do in the beginning of the thread, and what you should do being the answer from Ian.
p
thank you stylianos, but, I don't see how to apply the solution proposed by Ian:
Copy code
class MyViewModel : ViewModel() {
  // Replace with a flow from your repository
  val sourceOfTruth = flow {
    delay(1000)
    emit("loaded data")
  }
  val state = sourceOfTruth.stateIn(
    viewModelScope,
    WhileSubscribed(5000)
    "initial"
  )
}
I don't have a repository, and I don't have a flow, I'm reading some values with reflection from various origins. And I'm reading them on a while like this:
Copy code
class ScreenViewModel(
    private val specificationsUseCase: SpecificationsUseCase
): ViewModel() {
    private val _uiState = MutableStateFlow(SpecificationsScreenUiState(loading = true))
    val uiState: StateFlow<SpecificationsScreenUiState> = _uiState

    init {
        updateUiState()
    }

    private fun updateUiState() {
        viewModelScope.launch {
            while (isActive) {
                _uiState.update { currentState ->
                    currentState.copy(
                        pageContent = specificationsUseCase.getInfo(),
                        loading = false
                    )
                }
                delay(100)
            }
        }
    }
}
as you noticed, I disscovered that if the user press home button, and send app to background, or even if the user changes the screen, the while is still recovering data, so it's not a good behaviour
I need to change it to find how to only become active when the viewmodel is on screen... and I thought isActive did the job, but not
s
Copy code
class ScreenViewModel(
    private val specificationsUseCase: SpecificationsUseCase
): ViewModel() {
  val uiState: StateFlow<SpecificationsScreenUiState> = flow {
    while (currentCoroutineContext().isActive) {
      val info = specificationsUseCase.getInfo()
      emit(SpecificationsScreenUiState(pageContent = info, loading = false))
      delay(100)
    }
  }.stateIn(
    viewModelScope,
    SharingStarted.WhileSubscribed(5.seconds),
    SpecificationsScreenUiState(loading = true),
  )
}
And on your call site you need to make sure to call
Copy code
viewmodel.uiState.collectAsStateWithLifecycle()
instead of just 
collectAsState
p
well, thank you, but it's a little more complex than the sample I passed you
Copy code
data class SpecificationsScreenUiState(
    val loading: Boolean = false,
    val pagerState: PagerState = PagerState( pageCount = { Page.entries.size } ),
    val pageOneContent: Map<String, String> = mapOf(),
    val pageTwoContent: Map<String, String> = mapOf(),
)

class SpecificationsScreenViewModel(
    private val specificationsUseCase: SpecificationsUseCase
): ViewModel() {
    private val _uiState = MutableStateFlow(SpecificationsScreenUiState(loading = true))
    val uiState: StateFlow<SpecificationsScreenUiState> = _uiState

    init {
        updateUiState()
    }

    private fun updateUiState() {
        viewModelScope.launch {
            while (isActive) {
                Log.d("XXXX", "viewmodel while (isActive) loop")
                _uiState.update { currentState ->
                    val currentPage = Page.entries[currentState.pagerState.currentPage]

                    currentState.copy(
                        pageOneContent= if (currentPage == Page.ONE) specificationsUseCase.getInfo1() else currentState.systemPageContent,
                        pageTwoContent= if (currentPage == Page.TWO) specificationsUseCase.getInfo2() else currentState.cpuPageContent,
                        loading = false
                    )
                }
                delay(100)
            }
        }
    }
}
I tryed migrating your sample to my code:
Copy code
val uiState: StateFlow<SpecificationsScreenUiState> = flow {
    while (currentCoroutineContext().isActive) {
        val currentPage = Page.entries[currentState.pagerState.currentPage]

        emit(
            SpecificationsScreenUiState(
                pageOneContent= if (currentPage == Page.ONE) specificationsUseCase.getInfo1() else mapOf(),
                pageTwoContent= if (currentPage == Page.TWO) specificationsUseCase.getInfo2() else mapOf(),
                loading = false
            )
        )
        delay(100)
    }
}.stateIn(
    viewModelScope,
    SharingStarted.WhileSubscribed(5.seconds),
    SpecificationsScreenUiState(loading = true)
)
but I can't get the currentPage, because I don't have 
currentState
to get 
currentState.pagerState.currentPage
So I don't know how to proceed
also, is not necessary to have a _uiState with this implementation? I'm not sure about using this approach
what about returning back to my implementation and simply adding this to notify if the screen is in foreground and the loop should be alive?
Copy code
DisposableEffect(Unit) {
        viewModel.setScreenVisible(true)
        onDispose {
            viewModel.setScreenVisible(false)
        }
    }
m
I would say split the state that is updating via delay into a flow, and the other state into a flow, and then use the correct merge operator to combine them into a single state that the UI consumes.
p
well, things are starting to be very complex
don't know how to do that, flows are a very complex solution for me, too much possibilities and a very hard approach to my understanding
I prefeer to use simpler solutions, if they are not related with complex flows, is preferable
I can try with the solution proposed by Stylianos, but unfortunatelly I don't have the current state in the flow, so I can't know the current page
well, I solved it using this approach: I added this on the vm, and I removed the init{} block, now the updateUiState is called on onResume of the vm:
Copy code
fun onResume() {
    _uiState.update { currentState -> currentState.copy(onScreen = true) }
    updateUiState()
}

fun onPause() {
    _uiState.update { currentState -> currentState.copy(onScreen = false) }
}
the loop on the updateUiState method now checks that onScreen val of the uiState:
Copy code
private fun updateUiState() {
        viewModelScope.launch {
            while (isActive && _uiState.value.onScreen) {
                //JOB
                delay(100)
            }
        }
}
and I set that onScreen value in the screen composable with this:
Copy code
LifecycleResumeEffect(Unit) {
    vm.onResume()
    onPauseOrDispose {
        vm.onPause()
    }
}
what do you think?
6 Views
Open in Slack
PreviousNext
Powered by