Search code examples
google-mapskotlinandroid-roomkotlin-coroutinesandroid-viewmodel

Is it okay to create clicklisteners (or other listeners) inside a viewmodelscope?


I have a a fragment containing a googleMap where I am creating a bunch of Markers (which also is clickable). They are spiced with different information (colors, shapes and so on) from a room livedata query. In addition I have some MaterialButton buttons (which are styled as pushbuttons) where I toggle the Marker visible status on. At the moment, the "setup" of theese markers takes some time (200ms-2 secs, depends of amount of markers). To get out of that waiting, I was planning to use a viewmodelscope. Since there are some clicklisteners for theese buttons defined in there (they should do some action with the markers), will they still be alive when the viewmodelscope coroutine section ends, and If they are alive, do they still live in the correct coroutine-context, and do I need to do some housekeeping on the listeners when fragment and/or viewmodel ends?

I.E:

class MapsFragment:Fragment(){

   private lateinit var mapsViewModel : MapsViewModel
   private lateinit var googleMap : GoogleMap

//...
   override fun onCreateView(
        inflater: LayoutInflater,
        container: ViewGroup?,
        savedInstanceState: Bundle?
    ): View? {

        mapsViewModel = ViewModelProvider(requireActivity()).get(MapsViewModel::class.java)

        _binding = FragmentMapsBinding.inflate(inflater, container, false)
        val root:View = binding.root
//...
      return root
   }//onCreateView   

//...

   override fun onViewCreated(view: View, savedInstanceState:Bundle?){
      super.onViewCreated(view, savedInstanceState)
//...

      mapFragment?.getMapAsync(_googleMap->
         _googleMap?.let{safeGoogleMap->
            googleMap = safeGoogleMap
         }?:let{
            Log.e(TAG,"googleMap is null!!")
            return@getMapAsync
         }   
//...
      
         mapsViewModel.apply{

            liveDataMapsListFromFiltered?.observe(
               viewLifecycleOwner
            ){mapDetailList->
                
               viewModelScope.launch{ 

                
                  binding.apply{

                     //...
                     siteMarkers.map{
                       siteMarker.remove() //removes existing markes from map on update
                     }
                     siteMarkers.clear() //empty the siteMarker array on update 
                     //...
                   
                     mapDetailList?.map{
                        it.apply{
                           //...
                           coordinateSiteLongitude?.let { lng->
                              coordinateSiteLatitude?.let { lat->
                                 siteMarkerLatLng = LatLng(lat,lng)
                                 siteLatLngBoundsBuilder?.include(siteMarkerLatLng)
                              }
                           }
                           //...
                           siteMarkerLatLng?.let { safeSiteMarkerLatLng ->
                              val siteMarkerOptions =
                               MarkerOptions()
                                    .position(safeSiteMarkerLatLng)
                                    .anchor(0.5f, 0.5f)
                                    .visible(siteMarkerState)
                                    .flat(true)
                                  .title(setTicketNumber(ticketNumber?.toDouble()))
                                    .snippet(appointmentName)//TODO: Consider build siteId instead
                                    .icon(siteIcon[iconType])
                              siteMarkers.add(
                                  googleMap.addMarker(siteMarkerOptions) //Here are the markers added
                              )
                           }//siteMarkerLatLng?.let
                         

                        }//it.apply
                   
                     }//mapDetailList?.map

                     onSiteCheckedChangeListener?.let{
                        fragmentMapsMapTagSelector
                           ?.apTagSelectorMaterialButtonSite
                           ?.removeOnCheckedChangeListener(it) //clearing listener on button before update
                     }
                   
                     onSiteCheckedChangeListener = MaterialButton.OnCheckedChangeListener { siteButton, isChecked ->
                            
                        siteMarkers.map {
                            it.isVisible = isChecked
                        }
                     }.also {
                        fragmentMapsMapTagSelector
                          ?.mapTagSelectorMaterialButtonSite
                          ?.addOnCheckedChangeListener(it)
                     }

                     //Will this onCheckedChangeListener still survive when this viewmodelscope runs to the end ?
                   

                  }//binding.apply

               }//viewModelScope.launch

            }//liveDataMapsListFromFiltered.observe
         
         }//mapsviewModel.apply   


      }//getMapAsync
   
   }//onViewCreated

}//MapsFragment


Solution

  • I think you misunderstand what a CoroutineScope is. It determines the lifecycle of coroutines that it runs, but not of the objects created in the process of running those coroutines.

    viewModelScope is a CoroutineScope that automatically cancels any coroutines it is running when the associated ViewModel is torn down. The coroutine doesn't know what you're doing with it. Cancelling a coroutine merely stops it from running to completion, like returning from a function early. In your code, you set your listeners and haven't stored references to them besides in the views they are set to, so their lives are tied to their respective view's lives.

    If you were going to use a coroutine in your fragment to set up something for your UI, you would use the Fragment's lifecycleScope, not the ViewModel's viewModelScope. Like if you were fetching something to show in your UI, you would want that coroutine to be cancelled when the Fragment is destroyed, not the ViewModel which might be outliving the Fragment.

    Your use of a coroutine in your example code looks pointless, because I don't see any blocking or asynchronous suspend functions being called. You mentioned setting up site markers is taking like 200ms. I'm not familiar with Google Maps since I haven't used it in the past several years, so I'm not sure which part is time-consuming. Usually, UI elements do not allow you to interact with them on background threads, so you might be out of luck. But maybe the time-consuming part is allowed to be done on background threads. You'll have to read the documentation. Using a coroutine for this won't make it take less time, but can prevent the UI from stuttering/freezing.

    If you were going to do some long computation with a coroutine, you would need to switch dispatchers to do the blocking work and interact with the UI elements back on the main dispatcher. Simply putting something in a coroutine doesn't make it take less time, but it provides a convenient way to do something on another thread and then continue on the main thread after the result is ready. For example:

    lifecycleScope.launchWhenStarted { // lifecycle coroutines launch on main thread by default
        val result = withContext(Dispatchers.Default) { // switch to dispatcher for background work
            doTimeConsumingCalculation()
        }
        // back on main thread:
        applyResultsToMyViews(result) 
    }
    

    By using launchWhenStarted instead of launch, a Fragment's lifecycleScope will pause the coroutine when the Fragment is not attached, which will prevent potential crashes from trying to update UI using requireContext() or requireActivity() when there is no Activity.