I'm very new to Android Development.
I have a Room database of results:
@Entity(tableName = "results")
data class Result(
@PrimaryKey(autoGenerate = true)
val id: Int = 0,
val result : Float,
val date : String
)
@Database(
entities = [Result::class],
version = 1,
exportSchema = false
)
abstract class ResultDatabase : RoomDatabase() {
abstract fun resultDao() : ResultDao
companion object {
@Volatile
private var Instance : ResultDatabase? = null
fun getDatabase(context : Context) : ResultDatabase {
return Instance ?: synchronized(this) {
Room.databaseBuilder(context, ResultDatabase::class.java, "result_database")
.fallbackToDestructiveMigration()
.build()
.also { Instance = it }
}
}
}
}
With a DAO
@Dao
interface ResultDao {
@Insert(onConflict = OnConflictStrategy.REPLACE)
suspend fun insert(result : Result)
@Update
suspend fun update(result : Result)
@Delete
suspend fun delete(result : Result)
@Query("SELECT * FROM results WHERE id = :id")
fun getResult(id : Int) : Flow<Result>
@Query("SELECT * FROM results ORDER BY date ASC")
fun getAllResults() : Flow<List<Result>>
}
and a Repository
class ResultRepository(private val resultDao : ResultDao) {
fun getAllResults(): Flow<List<Result>> = resultDao.getAllResults()
suspend fun getResultsSmoothed(numSteps : Int) : List<Result> {
var allResults : List<Result> = listOf()
getAllResults().collect { result -> allResults = allResults + result }
val minDate = LocalDateTime.parse(allResults.first().date)
val maxDate = LocalDateTime.parse(allResults.last().date)
val period = Duration.between(minDate, maxDate)
var step = period.seconds / numSteps
var hundredResults = listOf<Result>()
var nextDate = minDate
for (i in 0..numSteps) {
val resultPrior = allResults.fold(allResults.first()) { acc: Result, r ->
val rd = LocalDateTime.parse(r.date)
if (rd <= nextDate && (rd > LocalDateTime.parse(acc.date))) r
else acc
}
val resultAfter = allResults.fold(allResults.last()) { acc: Result, r ->
val rd = LocalDateTime.parse(r.date)
if (rd > nextDate && (rd <= LocalDateTime.parse(acc.date))) r
else acc
}
if (resultAfter == resultPrior) {
val newResult = Result(date = nextDate.toString(), result = resultPrior.result)
hundredResults = hundredResults + newResult
} else {
val resultPriorDate = LocalDateTime.parse(resultPrior.date)
val resultAfterDate = LocalDateTime.parse(resultAfter.date)
val periodBetween = Duration.between(resultPriorDate, resultAfterDate)
val periodOver = Duration.between(resultPriorDate, nextDate)
val percentageOver =
periodOver.seconds.toDouble() / periodBetween.seconds.toDouble();
val resultDifference = resultAfter.result - resultPrior.result;
val increaseToPriorAmount = resultDifference.toDouble() * percentageOver;
val stepResult = resultPrior.result + increaseToPriorAmount;
val newResult = Result(date = nextDate.toString(), result = stepResult.toFloat())
hundredResults = hundredResults + newResult
}
nextDate = nextDate.plusSeconds(step)
}
return hundredResults
}
suspend fun getResultLabels(numLabels : Int) : List<String> {
var allResults : List<Result> = listOf()
getAllResults().collect { result -> allResults = allResults + result }
val minDate = LocalDateTime.parse(allResults.first().date)
val maxDate = LocalDateTime.parse(allResults.last().date)
val period = Duration.between(minDate, maxDate)
val step = period.seconds / numLabels
var dateLabels = listOf<String>()
var nextDate = minDate
for (i in 0..numLabels) {
val formatter = DateTimeFormatter.ofPattern("d MMM yy")
dateLabels = dateLabels + nextDate.format(formatter)
nextDate = nextDate.plusSeconds(step)
}
return dateLabels
}
@Suppress("RedundantSuspendModifier")
@WorkerThread
suspend fun insertResult(result : Result) {
resultDao.insert(result)
}
@Suppress("RedundantSuspendModifier")
@WorkerThread
suspend fun updateResult(result : Result) {
resultDao.update(result)
}
@Suppress("RedundantSuspendModifier")
@WorkerThread
suspend fun deleteResult(result : Result) {
resultDao.delete(result)
}
}
The two functions getResultLabels and getResultsSmoothed are processing some values from the database and returning things. The reason I put this in here was to provide it to the view model because I've understood that I shouldn't do this processing in my composables.
Then my ViewModel
class ResultViewModel(private val repository: ResultRepository) : ViewModel() {
val allResults = repository.getAllResults()
fun getSmoothedResults(num : Int) : Flow<List<Result>> {
return flow {
val results = repository.getResultsSmoothed(num)
Log.d("ME vm", results.toString())
emit(results)
}
}
fun getResultLabels(num : Int) : Flow<List<String>> {
return flow { emit(repository.getResultLabels(num)) }
}
fun update(result : Result) = viewModelScope.launch {
repository.updateResult(result)
}
fun delete(result : Result) = viewModelScope.launch {
repository.deleteResult(result)
}
fun insert(result : Result) = viewModelScope.launch {
repository.insertResult(result)
}
}
class ResultViewModelFactory(private val repository: ResultRepository) : ViewModelProvider.Factory {
override fun <T : ViewModel> create(modelClass : Class<T>) : T {
if(modelClass.isAssignableFrom(ResultViewModel::class.java)) {
@Suppress("UNCHECKED_CAST")
return ResultViewModel(repository) as T
}
throw IllegalArgumentException("Unknown ViewModel class")
}
}
What I'm trying to do is put these Lists into Flows so that the composables that use them will automatically update.
For example:
@Composable
fun Metrics(resultViewModel: ResultViewModel,
resultList : List<Result>?) {
if(resultList.isNullOrEmpty())
return
var dateLabels = resultViewModel.getResultLabels(5).collectAsState(initial = listOf())
var hundredResults = resultViewModel.getSmoothedResults(100).collectAsState(
initial = listOf()
)
Log.d("memememe", hundredResults.value.toString())
Column()
{
LineChart(
modifier = Modifier
.fillMaxWidth()
.height(300.dp)
.padding(top = 12.dp, end = 12.dp),
linesChartData = listOf(LineChartData(
lineDrawer = SolidLineDrawer(color = MaterialTheme.colorScheme.onBackground),
points = hundredResults.value.map {
LineChartData.Point(it.result, it.id.toString())
})),
animation = simpleChartAnimation(),
pointDrawer = com.github.tehras.charts.line.renderer.point.NoPointDrawer,
labels = dateLabels.value,
xAxisDrawer = SimpleXAxisDrawer(
axisLineThickness = 1.dp,
axisLineColor = MaterialTheme.colorScheme.onBackground,
labelTextColor = MaterialTheme.colorScheme.onBackground
),
yAxisDrawer = SimpleYAxisDrawer(
axisLineThickness = 1.dp,
axisLineColor = MaterialTheme.colorScheme.onBackground,
labelTextColor = MaterialTheme.colorScheme.onBackground
)
)
DataTable(
modifier = Modifier.fillMaxWidth(),
columns = listOf(
DataColumn(
width = TableColumnWidth.Fixed(60.dp)
) {
Text("")
},
DataColumn {
Text("Date")
},
DataColumn {
Text("Result")
}
)
) {
resultList.forEach { r ->
row {
cell {
IconButton(onClick = { resultViewModel.delete(r) }) {
Icon(Icons.TwoTone.Delete, "Delete result")
}
}
cell {
Text(
text = LocalDateTime.parse(r.date)
.format(DateTimeFormatter.ofPattern("dd MMM yyyy"))
)
}
cell {
Text("${r.result}")
}
}
}
}
}
}
However, my log call has an empty list. I may be approaching this problem completely wrong, but I'm unsure. I'm a little confused about the use of coroutines and state, and how these interact.
You are right, the processing that getResultLabels and getResultsSmoothed do does not belong in the composables. It doesn't belong in the repository either, though.
This kind of processing is called business logic and should be done in between the data layer that your repository belongs to and the UI presentation that is your Compose code. There is even a dedicated layer for that, the Domain layer, although I would only recommend to use that if the business logic was much more extensive than just these two functions. Without a dedicated domain layer the business logic would instead be done in the view model. The view model technically belongs to the UI layer, but since it's the place where the raw data is transformed into UI state, it fits quite well here.
This won't solve your problem, though.
In clean architecture the view model ideally only passes state on to the composables by exposing properties of type StateFlow, while receiving events from the UI by functions without a return value (the latter already looks good in your code). Moreover, the general idea is to have your data sources (in your case the Room database) return a flow that will only be transformed on the way up through the layers until it is collected in the UI. In your code, however, the database flow is already collected by the repository and simple lists are returned. In your view model you then try to wrap the lists in flows again, which won't work, because the view model's flows are now severed from the database.
The goal is that the UI receives a flow that is automatically updated with the current values in the database when anything changes there, without having to poll for new results by calling some functions again. To achieve this, flows should only be collected in compose. Not in the repository, not in the view model.
To refactor your code to comply with this you first need to extract the business logic from the repository. Currently, the functions getResultsSmoothed and getResultLabels work with flows. The business logic should have nothing to do with flows though. It should just transform data, in this case of list of results into a list of something else. Best move the transformations into a separate file as top-level functions on the file level:
fun getResultsSmoothed(
allResults: List<Result>,
numSteps: Int,
): List<Result> {
val minDate = LocalDateTime.parse(allResults.first().date)
val maxDate = LocalDateTime.parse(allResults.last().date)
val period = Duration.between(minDate, maxDate)
val step = period.seconds / numSteps
var hundredResults = listOf<Result>()
var nextDate = minDate
for (i in 0..numSteps) {
val resultPrior = allResults.fold(allResults.first()) { acc: Result, r ->
val rd = LocalDateTime.parse(r.date)
if (rd <= nextDate && (rd > LocalDateTime.parse(acc.date))) r
else acc
}
val resultAfter = allResults.fold(allResults.last()) { acc: Result, r ->
val rd = LocalDateTime.parse(r.date)
if (rd > nextDate && (rd <= LocalDateTime.parse(acc.date))) r
else acc
}
if (resultAfter == resultPrior) {
val newResult = Result(date = nextDate.toString(), result = resultPrior.result)
hundredResults = hundredResults + newResult
} else {
val resultPriorDate = LocalDateTime.parse(resultPrior.date)
val resultAfterDate = LocalDateTime.parse(resultAfter.date)
val periodBetween = Duration.between(resultPriorDate, resultAfterDate)
val periodOver = Duration.between(resultPriorDate, nextDate)
val percentageOver =
periodOver.seconds.toDouble() / periodBetween.seconds.toDouble()
val resultDifference = resultAfter.result - resultPrior.result
val increaseToPriorAmount = resultDifference.toDouble() * percentageOver
val stepResult = resultPrior.result + increaseToPriorAmount
val newResult = Result(date = nextDate.toString(), result = stepResult.toFloat())
hundredResults = hundredResults + newResult
}
nextDate = nextDate.plusSeconds(step)
}
return hundredResults
}
fun getResultLabels(
allResults: List<Result>,
numLabels: Int,
): List<String> {
val minDate = LocalDateTime.parse(allResults.first().date)
val maxDate = LocalDateTime.parse(allResults.last().date)
val period = Duration.between(minDate, maxDate)
val step = period.seconds / numLabels
var dateLabels = listOf<String>()
var nextDate = minDate
for (i in 0..numLabels) {
val formatter = DateTimeFormatter.ofPattern("d MMM yy")
dateLabels = dateLabels + nextDate.format(formatter)
nextDate = nextDate.plusSeconds(step)
}
return dateLabels
}
They are now independent from the rest of your code and can be called whenever a List<Result> needs to be transformed, whatever the current context is.
Both functions in the repository can now be deleted so the repository is now clean of any flow collections, as it should be. The only data it exposes is the flow of all results returned from the getAllResults function.
With this out of the way the view model now just needs to take that flow and transform its content only, by using the two functions we just liberated from the repository:
val smoothedResults: StateFlow<List<Result>> = repository.getAllResults()
.mapLatest { results ->
getResultsSmoothed(results, 100)
}
.stateIn(
scope = viewModelScope,
started = SharingStarted.WhileSubscribed(5_000),
initialValue = emptyList(),
)
val resultLabels = repository.getAllResults()
.mapLatest { results ->
getResultLabels(results, 5)
}
.stateIn(
scope = viewModelScope,
started = SharingStarted.WhileSubscribed(5_000),
initialValue = emptyList(),
)
mapLatest works on flows very similar to how map works on lists: For each new flow emission the content of the flow is replaced by the result of the lambda. The return value is still a Flow, it is just the content that changed, and we didn't need to collect anything.
After that, the flow is converted to a StateFlow by stateIn. A StateFlow is a specially configured flow which doesn't keep a history and only provides the most current value to its collectors. Moreover, it is a hot flow, which means it works on its own and therefore can be shared with multiple collectors. That's also why we need to provide the viewModelScope, so it can internally launch a coroutine to do all this. And since it runs on its own we also need to supply an initial value that will be used until the upstream flow produces its first value. The Android documentation has a nice primer on StateFlows (a bit outdated, they sill collect flows in the view model there), but since flows are a part of Kotlin, for a comprehensive overview you should have a look at the Kotlin documentation.
As you can see from the code above, smoothedResults and resultLabels are now properties. Since they don't have parameters, I moved the 100 and the 5 that were previously used to call the functions getSmoothedResults and getResultLabels into the view model itself. The functions aren't needed any more and can now be removed.
This should now all work as expected. Your Compose code just needs to collect the flows from the new properties instead:
var dateLabels = resultViewModel.resultLabels.collectAsState()
var hundredResults = resultViewModel.smoothedResults.collectAsState()
I removed the initial value here because that is already part of the StateFlow in the view model. Actually, you should replace collectAsState with collectAsStateWithLifecycle from the gradle dependency androidx.lifecycle:lifecycle-runtime-compose. This will be a bit more efficient because the flow collection is stopped when the composable isn't on screen by respecting the activity's lifecycle. Also, the variables can be declared as val instead of var because they contain a flow. The values of the flow may change, but never the flow itself. And to make it perfect, you can use Kotlin's by delegates to unwrap the State's value property. Then it would look like this:
val dateLabels by resultViewModel.resultLabels.collectAsStateWithLifecycle()
val hundredResults by resultViewModel.smoothedResults.collectAsStateWithLifecycle()
Wherever you used hundredResults.value and dateLabels.value before you can now just use hundredResults and dateLabels respectively.
There are just two issues left that need to be addressed:
As you might have already realized you cannot parameterize getResultLabels(5) and getSmoothedResults(100) from the UI anymore. The values are now hardcoded in the view model. To change that the values would need to come from a flow themselves. That sounds complicated at first, but it can be done quite elegantly with Kotlin. With smoothedResults as an example it would look like this:
private val smoothedResultsNum = MutableStateFlow(100)
val smoothedResults: StateFlow<List<Result>> = repository.getAllResults()
.combine(smoothedResultsNum) { results, num ->
getResultsSmoothed(results, num)
}
.stateIn(
scope = viewModelScope,
started = SharingStarted.WhileSubscribed(5_000),
initialValue = emptyList(),
)
fun updateSmoothedResultsNum(num: Int) {
smoothedResultsNum.value = num
}
A MutableStateFlow is just a container for a value that you can access by the value property. It is also a fully fledged flow and can therefore be combined with other flows as seen above. It starts out with a value of 100, but you can change that value anytime by calling updateSmoothedResultsNum from your composables. The smoothedResults StateFlow will always update when any of the underlying flows has a new value. That is, either the database contains new values or the num was changed.
The same can also be applied to the other StateFlow, resultLabels.
This is something that was already a problem in your original code. Your repository's function getAllResults() is called multiple times (twice, to be precise). Each time it is called a new SQL query is executed on the database and a new flow is returned. With the same content, though, so this actually wouldn't be necessary. When you don't have much data in the database that won't matter much, but with increasing data or additional callers of the function this can become a bottleneck quite fast.
The solution is to convert the flow to a SharedFlow in the repository. This is a general version of a StateFlow. It is also a hot flow which is necessary so it can run the underlying flow independently, so it can provide every collector that is subscribed to the SharedFlow with the same values. Hence the name: This flow, in contrast to cold flows as they are returned from Room f.e., can be shared with multiple collectors. This also means, that you need a coroutine scope, as you did with the StateFlow in the view model:
class ResultRepository(
private val resultDao: ResultDao,
scope: CoroutineScope,
) {
val allResults: Flow<List<Result>> = resultDao.getAllResults()
.shareIn(
scope = scope,
started = SharingStarted.WhileSubscribed(),
)
// ...
}
Other than with the view model your repository doesn't have a built-in coroutine scope so it must be supplied as a parameter for the consructor. When you create your repository in an activity, you can use the activity's lifecycleScope for that.