Is it an anti-pattern to keep a StateFlow inside a Repository?

Question

I have a singleton LocationRepository class and inside that I have a callbackFlow method which gets location updates from FusedLocationProvider. And a Service class that collects that flow. The thing is I want to use that location info in an Activity too.

class LocationRepositoryImpl @Inject constructor(
    private val api: Api,
    private val fusedClient: FusedLocationProviderClient,
    @ApplicationContext private val context: Context
) : LocationRepository {

    private val _lastLocation = MutableStateFlow<Location?>(null)
    val lastLocation = _lastLocation.asStateFlow()

    override fun getLocationUpdates(hasLocationPermissions: Boolean): Flow<Location> =
        callbackFlow {
            val locationRequestTimeInterval =
                ConfigManager.getInstance().locationRequestFromDeviceTimeLimitMs

            val locationManager =
                context.getSystemService(Context.LOCATION_SERVICE) as LocationManager

            val isGpsEnabled = locationManager.isProviderEnabled(LocationManager.GPS_PROVIDER)
            val isNetworkEnabled =
                locationManager.isProviderEnabled(LocationManager.NETWORK_PROVIDER)

            if (!isGpsEnabled && !isNetworkEnabled) {
                throw LocationRepository.LocationException()
            }

            val locationCallback: LocationCallback = object : LocationCallback() {
                override fun onLocationResult(locationResult: LocationResult) {
                    locationResult.lastLocation?.let { location ->
                        lastFuseLocationTime = System.currentTimeMillis()
                        _lastLocation.update { location }
                        trySend(location)
                    }
                }
            }

            val locationRequest = LocationRequest.Builder(locationRequestTimeInterval)
                .setMinUpdateIntervalMillis(locationRequestTimeInterval)
                .setWaitForAccurateLocation(false)
                .setPriority(Priority.PRIORITY_HIGH_ACCURACY)
                .build()

            if (hasLocationPermissions) {
                fusedClient.requestLocationUpdates(
                    locationRequest,
                    locationCallback,
                    Looper.getMainLooper()
                )
            }

            awaitClose {
                fusedClient.removeLocationUpdates(locationCallback)
            }
        }
}

There are several ways I have in mind to achieve that,

1. Using something like an event bus (LocalBroadcastManager maybe) to send Location info from Service to the Activity.
2. Collecting that callbackFlow both in the Activity's viewModel and in the Service class.
3. Defining a StateFlow in LocationRepository that holds the location info, update it when the location callback is triggered and collect it in Activity's videModel. Also collect the callBackFlow directly in the Service.

I don't want to use the first 2 because I am not sure about collecting a cold flow from multiple places and the LocalBroadcastManager is deprecated.

Is keeping aStateFlow inside a singleton repository a violation of clean architecture and MVVM?

Answer 1

In clean architecture, the repository layer abstracts the data sources and provides data to higher layers. If the repository's data rely on location information, it is crucial to implement a StateFlow that accurately reflects the latest known location. This is non-negotiable to ensure the integrity of the data.

From an MVVM perspective, the repository is typically accessed by the ViewModel to provide data to the UI layer. If the location information is relevant to the UI and you need to observe it in the activity, it can make sense to expose the StateFlow from the repository and collect it in the activity's ViewModel OR you can also expose a Flow in the repository abstract interface, and expose a StateFlow in the viewModel so the UI can use it!

Something to note though, is that StateFlow is a hot flow, so it will emit values even when there are no collectors. Just ensure this is the right behaviour your are expecting in your case!

Answer 2

Keeping a StateFlow inside a singleton repository itself is not a violation of clean architecture or MVVM principles. However, the way it is accessed and used in different parts of your application might need some consideration.

In your case, the LocationRepository is responsible for handling location-related operations, and it exposes a Flow to provide location updates. This is a reasonable approach.

To use this location information in multiple places, such as an Activity and a Service, you have a few options:

1. Collect the Flow in both the Activity's ViewModel and the Service: This approach allows both the Activity and the Service to independently consume the location updates. However, it's important to note that the callbackFlow created by the repository is hot, meaning that it will continue to emit values as long as there are active collectors. If both the Activity and the Service are active and collecting the same Flow, they will both receive the updates simultaneously.
2. Define a separate StateFlow in the repository for location updates: Instead of exposing the callbackFlow directly, you could define a separate StateFlow in the LocationRepository that holds the latest location information. Update this StateFlow within the callbackFlow, and have both the Activity's ViewModel and the Service collect this StateFlow. This way, both components can access the latest location information independently without having to collect the callbackFlow directly.

Here's an example of how you could modify your code to implement option 2:

    class LocationRepositoryImpl @Inject constructor(
        private val api: Api,
        private val fusedClient: FusedLocationProviderClient,
        @ApplicationContext private val context: Context
    ) : LocationRepository {
    
        private val _lastLocation = MutableStateFlow(null)
        val lastLocation: StateFlow = _lastLocation
    
        private val locationCallback: LocationCallback = object : LocationCallback() {
            override fun onLocationResult(locationResult: LocationResult) {
                locationResult.lastLocation?.let { location ->
                    lastFuseLocationTime = System.currentTimeMillis()
                    _lastLocation.value = location
                }
            }
        }
    
        override fun getLocationUpdates(hasLocationPermissions: Boolean): Flow =
            callbackFlow {
                // ...
                if (hasLocationPermissions) {
                    fusedClient.requestLocationUpdates(
                        locationRequest,
                        locationCallback,
                        Looper.getMainLooper()
                    )
                }
    
                awaitClose {
                    fusedClient.removeLocationUpdates(locationCallback)
                }
            }
    }

With this modification, you can collect lastLocation in both the Activity's ViewModel and the Service to get the latest location updates independently.

Answer 3

Why not follow the Clean Architecture principles and utilize UseCases? By incorporating UseCases, we can further enhance the code structure and separation of concerns. Additionally, we can leverage StateFlow in the ViewModel to handle data updates efficiently.

In the repository layer, we can define a method that returns a Flow

fun getDataStream(): Flow<List<MyData>>

Then in Usecase

class GetAllData(
    private val repository: MyRepository
) {
    operator fun invoke() = repository.getDataStream()
}

and finally in viewModel

class MyViewModel @Inject constructor(
    private val getAllDataFromRepo: GetAllData
) : ViewModel() {

    fun GetAllData(): Flow<List<MyData>> = getAllDataFromRepo().stateIn(
        viewModelScope, SharingStarted.WhileSubscribed(5000), emptyList()
    )

}

Usage in view/composable

val dataList: List<MyData> by viewModel.GetAllData()
    .collectAsStateWithLifecycle(initialValue = emptyList())

By using the stateIn operator, we can maintain the state until at least one subscriber is active. Also, we introduce a 5-second delay before destroying the state, ensuring better data availability.