Search code examples
androidkotlinkotlin-stateflowkotlin-sharedflow

android kotlin Flow emitted values are not collected


I have model class:

data class TweetResponse(
    val data: Data?,
    val matching_rules: List<ApiMatchingRule?>?,

    val lifeSpan: MutableStateFlow<Int>? = MutableStateFlow(30),
    var timer: CountDownTimer?
)

In my repository class I get list of these elements and emit them:

var tweetDataResponse = MutableSharedFlow<MutableList<TweetResponse>>()
var tweetList: MutableList<TweetResponse> = mutableListOf()

....some code

tweetResponse?.let { tweetList.add(it) }
tweetDataResponse.emit(tweetList)

My Viewmodel collects this list and start ticker for every item:

  private fun setCollectors() {
        viewModelScope.launch {
            twitterRepository.tweetDataResponse.collect { response ->
                response.forEach {
                    it.timer = object : CountDownTimer(TWITTER_TIMER_COUNT) {
                        override fun onTick(remainingSeconds: Int) {
                            viewModelScope.launch {
                                it.lifeSpan?.emit(remainingSeconds)
                            }
                        }

                        override fun onFinish() {
                        }

                    }
                }
                tweetResponse.emit(response)
            }
        }
    }

also I emit this list to my fragment (tweetResponse.emit(response)) so it can give it to the corresponding adapter.

In my fragment I collect list and transfer it to adapter:

  private fun setCollectors() {
        lifecycleScope.launch {
            repeatOnLifecycle(Lifecycle.State.CREATED) {
                viewModel.tweetResponse.collect {
                    adapter.addTweets(it)
                    }
                }
            }
        }
    }

And finally in adapter I set this list and bind it in onBIndViewholder:

  override fun onBindViewHolder(holder: ViewHolderHome, position: Int) {
        val item = tweetList[position]

    CoroutineScope(Dispatchers.IO).launch {
        item.lifeSpan?.collect { secondsLeft ->
            withContext(Dispatchers.Main) {
                holder.binding.lifespan.text = secondsLeft.toString()
            }
        }
    }

          item.timer?.start()
}

As you can see, in onBind I set collector for every item and try to collect changes of lifespan flow variable. When I debug, my viewmodel does the ticking all the time and my collector in adapter is set properly. But values never get collected. There is flaw somewhere, can I get some help in the friday night :D


Solution

  • Before looking at why it's not updating, the biggest glaring issue is your onBindViewHolder implementation. It is collecting an infinite flow (StateFlows are infinite) in a new abandoned CoroutineScope, so as you scroll up and down the list, you will have more and more obsolete coroutines running forever and leaking your views for the lifetime of your app. If you're going to start coroutines in onBindViewHolder, you need to run them in the associated Activity's or Fragment.viewLifecycleOwner's lifecycleScope, and furthermore you must store the launched Job in a property and cancel the old one before replacing it.

    Also a general comment--you're using mutability way too much. It doesn't make sense to combine a var with a mutable collection or flow because then it's mutable in two different ways, which is likely to cause bugs.

    It's very error prone to have a Flow of a mutable collection because it's ambiguous whether the emitted items are the same or not. They're possibly the same instance even if the content of the collection is changing, but there's no way to compare an item to the previous item if they're the same instance. If you put these in a StateFlow or use distinctUntilChanged(), it would be completely broken, but even without doing that, it is likely to cause bugs.

    It's also quite convoluted that TweetResponse has a StateFlow for its life and also a Timer that's controlled from outside the class. It's a misuse of a data class to give it data properties that are not data. The Timer class doesn't have a proper equals() function, so your data class's equals() function will be broken.

    There are so many possible avenues for failure here that I don't think it's worth attempting to analyze this code and figure out exactly why it's failing. I think it needs to be redesigned without all the mutability upon mutability everywhere.

    Here's what I'd do. Give TweetResponse a simple startTime property. Then it doesn't have to have any moving parts or mutability. You can give it an extra property that's not in its constructor to return its time remaining. Then you only need to be checking and updating views in one place instead of running coroutines for every single tweet.

    data class TweetResponse(
        val data: Data?,
        val matching_rules: List<ApiMatchingRule?>?,
        val startTime: Long = 0
    ) {
        val currentLifeRemaining: Long
            get() = (TWITTER_TIMER_COUNT - System.currentTimeMillis() + startTime).coerceAtLeast(0L)
    }
    

    Use stateIn where you declare the tweetResponse property for simpler code, and remove your setCollectors() function.

    val tweetResponse = twitterRepository.tweetDataResponse.map { response ->
            val startTime = System.currentTimeMillis()
            response.map { it.copy(startTime = startTime) }
        }.stateIn(viewModelScope, SharingStarted.Eagerly, emptyList())
    

    Then in your adapter, you can run a single coroutine in an infinite loop with a one second delay that it uses to update all its currently visible views using TweetResponse.currentLifeRemaining.