Hi everyone, please can anyone help identify why `...

# compose

[Ajayi Olamide]

Hi everyone, please can anyone help identify why tracks state is not collected/displayed in AudioList?

Please note:
1. MediaSelection.Songs is the default mediaUiState.currentMediaSelection, which means AudioList is launched by default when we navigate to LocalMediaScreen. Items are not displayed in AudioList but if we switched ArtistList tab, items are displayed. If I make mediaUiState.currentMediaSelection default to MediaSelection.Artists and ArtistList initial tab, then artist items are not displayed but AudioList items are displayed when tab is switched.
2. Both AudioList and ArtistList have similar codes in LaunchedEffect to retrieve their respective items when launched, but items are not displayed in AudioList although they're successfully displayed in ArtistList.
3. I can confirm their respective localMediaMediaViewModel.loadMedia() is correctly called as I successfully log those items before updating their respective StateFlow values.
4. In AudioList, I can call collect on the tracks StateFlow of localMediaViewModel to listen for items added and push them to local state tracks. This is not a desired implementation (as I want to call loadMedia just once, based on the condition that the respective items are yet to be loaded). However, this approach successfully works on LocalMediaScreen launch. Once I switch to a different tab and switch back, tracks are no longer visible.
This is a strange behaviour but I'm suspecting it's lifecycle/thread related. localMediaRepo.loadMedia is a Sequence<T> function that yields each item after construction. I'm not sure this is an issue as I get the same behaviour even if I returned List<T>.

Kindly help🙏

[Zach Klippenstein (he/him) [MOD]]

There’s things in each of these screenshots that worry me, and I can’t even see all the code in loadMedia so I’m not entirely sure what’s going on there.

1. LocalMediaScreen: Why are you emitting an empty Box in the else case? Just don’t emit anything.
2. Artist/AudioList: The keys to your LaunchedEffect should be the non-snapshot-state dependencies of your effect. In this case, your view models.
3. Artist/AudioList: Your view layer is doing too much. It’s not the responsibility of the view layer to coordinate view models like this. I think some of this is encouraged by the ViewModel architecture, which is partly why it’s so bad. This is terrible for testability as well.
4. loadMedia: Why is this a suspend function if all it does is launch into the viewModelScope?
5. loadMedia: It is a very bad idea to return a lazy Sequence from a suspend function, because if the suspend function is executed in a different context from where the sequence is consumed (as is explicitly the case here), most of the sequence’s work will actually be performed in the consuming context. This probably isn’t what you want, since you’re creating the sequence on a different dispatcher.
6. loadMedia: Why are you returning a Sequence anyway if you’re just consuming the entire thing immediately? Even if you weren’t hopping threads the laziness wouldn’t provide any value here, so you’re just introducing overhead and unnecessary concurrency.
a. A case where laziness might make sense would be if your consumer suspended while consuming, but Sequence still wouldn’t be the right tool for that job because of (5). You’d likely want a Flow instead with a flowOn to keep the producer on the correct dispatcher.
7. loadMedia: Why are you storing `MutableList`s in states? It doesn’t make sense to mutate those lists anyway, so you can just use the read-only type (List).
8. loadMedia: It’s cut off so I can’t see, but what are you doing with the sequence iterator? I hope that full code isn’t (_thing.value as Success).add(iter.next), because that won’t work and would likely explain the behavior you’re seeing. Please read this for an explanation of why.

[Ajayi Olamide]

Hey @Zach Klippenstein (he/him) [MOD], thanks for your reply and all the feedbacks/observations. I've done a lot of refactoring to get these to work, so I might have missed out on some of best practices. Also, I'd say I'm relatively new to kotlin, so I'm still learning multi threading concepts in kotlin. Thanks for pointing out your concerns. I'll try to respond to them all.

1. The Box here is just a temporary placeholder. I have several other arms I need to handle for currentSelection — a number of components I need to display based on the current value of currentSelection. I just want to ensure I got these two working first.
2. Thanks for this. I'll check the link to read more about "non-snapshot-state dependencies." I wasn't just sure if it's okay pass a ViewModel as dependency to an effect.
3. Could you please suggest how to make the ArtistList/AudioList view layer do less? I would appreciate if you're more specific about this. Is it wrong to pass multiple ViewModel s to a single layer? Is it something that has to do with the function calls in LaunchedEffect?
4. The goal is to create a function that runs in the background while updating the UI as data becomes available. localMediaRepo.loadMedia would read all audio files on the device and construct a list of T from the result. As this might be a long process for devices with lots of files, I thought suspending this task is (supposed to be) a good idea. What do you think?
5. I was returning List<T> initially but I think creating a Sequence that yields T, once it's available, for the ViewModel to push to StateFlow would be a good idea. This way, the view layer could collect the data and display in "real-time." What do you think? I'd like to see how I can address the potential issues here.
6. I think I would need to checkout Flow and flowOn.
7. I think I explained this in (5). I had a List but somehow changed to MutableList. I was just trying to emulate a real-time update.
8. Initially, loadMedia was returning a List<T> and I was updating like _thing.value += list. This wasn't working as well. But like I've said, loadMedia is just a function for reading audio files from the device and construct a sequence of T from the result.
Thanks for your help and I hope I'm able to address some of the issues you raised.

[Zach Klippenstein (he/him) [MOD]]

I see what you’re trying to do but sequence is the wrong tool for this job because of the coroutine context stuff I mentioned. You want a Flow.

[Zach Klippenstein (he/him) [MOD]]

Something like this:

fun loadMedia(stuff): Flow<Item> = flow {
	// load files, call emit
}.flowOn(<http://Dispatchers.IO|Dispatchers.IO>)

class YourViewModel {
	private val _items = mutableStateListOf<Item>()
	val items: List<Item> get() = _items
	
	suspend fun run() {
		_items.clear()
		loadMedia(…).collect {
			_items += it
		}
	}
}

[Zach Klippenstein (he/him) [MOD]]

Re: 3, ideally imo you’d not pass in more than one view model, because any interaction with any parent view models would be the responsibility of the screen’s view model. But if you’re using AAC view models, that might not be straightforward, but is where dependency injection libraries might be able to help a bit.

[Zach Klippenstein (he/him) [MOD]]

Libraries like Circuit, Workflow, and a bunch of others make it much easier to do that sort of thing

[Ajayi Olamide]

Thank you so much for your help @Zach Klippenstein (he/him) [MOD], I greatly appreciate your feedback. I've learnt a lot in this conversation. I'll resolve to Flow as you suggested. I think I should emit events from AudioList/ArtistList up the hierarchy, especially for handling states shared globally across the app, instead of passing multiple viewmodels down the tree. I think that should be more reasonable according to your suggestion.