Correct way of handling Coroutine inside Fragment

Issue

The following suspend function is updating a progress bar and 2 TextViews in 1 second delays. The progressbar is indicating the progress of an MP3 and the TextViews respectively the elapsed and remaining time.

The user can leave the fragment and come back to it again, meaning the fragment(view) gets destroyed and created again.

I was wondering if this implementation is correct and/or if there are better implementations and/or alternatives (first time ever implementing a coroutine). Here is some code:

class BookViewFragment : Fragment(), CoroutineScope {
    private var _binding: FragmentBookViewerBinding? = null
    private val bookViewFragmentBinding get() = _binding!!

    private lateinit var job: Job
    override val coroutineContext: CoroutineContext
        get() = Dispatchers.Main + job

    override fun onCreateView(
        inflater: LayoutInflater,
        container: ViewGroup?,
        savedInstanceState: Bundle?
    ): View? {
        _binding = FragmentBookViewerBinding.inflate(layoutInflater)
        val view = bookViewFragmentBinding.root
        job = Job()
        initMediaPlayer()
        return view
    }

    override fun onDestroyView() {
        super.onDestroyView()
        job.cancel()
        _binding = null
        mp.stop()
        mp.release()
    }

    private fun initMediaPlayer() {
        mp = MediaPlayer()

        mp.run {
            setDataSource(...)
            setVolume(0.5f, 0.5f)
            prepare()
        }
        totalTime = mp.duration
        initPositionBar()
    }

    private fun initPositionBar() {
        bookViewFragmentBinding.mediaPosition.max = totalTime

        launch {
            setTimeOnProgressBar()
        }
    }

    private suspend fun setTimeOnProgressBar() {
        coroutineScope {
            launch {
                var progress = mp.currentPosition
                while (progress < mp.duration) {
                    progress = mp.currentPosition
                    bookViewFragmentBinding.mediaPosition.progress = progress
                    val timePlayed = progress
                    val timeLeft = mp.duration - timePlayed
                    bookViewFragmentBinding.timePlayed.text = formatIntToTime(timePlayed)
                    bookViewFragmentBinding.timeLeft.text =
                        getString(R.string.time_left, formatIntToTime(timeLeft))
                    delay(1000)
                }
            }
        }
    }
}

Solution

This looks correct, but you have created two unnecessary layers of coroutine around your loop. In setTimeOnProgressBar(), you have wrapped your coroutine in a new coroutineScope that you don't use for anything. That can be removed, and then this doesn't have to be a suspend function at all. And so you can also remove the coroutine you've wrapped the call to setTimeOnProgressBar() with in initPositionBar().

Also, you've recreated a bunch of boilerplate that's already provided by the Android ktx library. There is already a lifecycleScope extension property you can use to launch coroutines, and it is automatically cancelled in onDestroyView(). So you don't need to create a parent Job or override coroutineContext, or cancel the parent job.

You can use lifecycleScope.launch when launching coroutines.

class BookViewFragment : Fragment() {
    private var _binding: FragmentBookViewerBinding? = null
    private val bookViewFragmentBinding get() = _binding!!

    override fun onCreateView(
        inflater: LayoutInflater,
        container: ViewGroup?,
        savedInstanceState: Bundle?
    ): View? {
        _binding = FragmentBookViewerBinding.inflate(layoutInflater)
        val view = bookViewFragmentBinding.root
        initMediaPlayer()
        return view
    }

    override fun onDestroyView() {
        super.onDestroyView()
        _binding = null
        mp.stop()
        mp.release()
    }

    private fun initMediaPlayer() {
        mp = MediaPlayer()

        mp.run {
            setDataSource(...)
            setVolume(0.5f, 0.5f)
            prepare()
        }
        totalTime = mp.duration
        initPositionBar()
    }

    private fun initPositionBar() {
        bookViewFragmentBinding.mediaPosition.max = totalTime

        setTimeOnProgressBar()
    }

    private fun setTimeOnProgressBar() {
        lifecycleScope.launch {
            var progress = mp.currentPosition
            while (progress < mp.duration) {
                progress = mp.currentPosition
                bookViewFragmentBinding.mediaPosition.progress = progress
                val timePlayed = progress
                val timeLeft = mp.duration - timePlayed
                bookViewFragmentBinding.timePlayed.text = formatIntToTime(timePlayed)
                bookViewFragmentBinding.timeLeft.text =
                        getString(R.string.time_left, formatIntToTime(timeLeft))
                delay(1000)
            }
        }
    }
}

Answered By - Tenfour04