0

A have a screen where I display 10 users. Each user is represented by a document in Firestore. On user click, I need to get its details. This is what I have tried:

fun getUserDetails(uid: String) {
    LaunchedEffect(uid) {
        viewModel.getUser(uid)
    }
    when(val userResult = viewModel.userResult) {
        is Result.Loading -> CircularProgressIndicator()
        is Result.Success -> Log.d("TAG", "You requested ${userResult.data.name}")
        is Result.Failure -> Log.d("TAG", userResult.e.message)
    }
}

Inside the ViewModel class, I have this code:

var userResult by mutableStateOf<Result<User>>(Result.Loading)
    private set

fun getUser(uid: String) = viewModelScope.launch {
    repo.getUser(uid).collect { result ->
        userResult = result
    }
}

As you see, I use Result.Loading as a default value, because the document is heavy, and it takes time to download it. So I decided to display a progress bar. Inside the repo class I do:

override fun getUser(uid: String) = flow {
    try {
        emit(Result.Loading)
        val user = usersRef.document(uid).get().await().toObject(User::class.java)
        emit(Result.Success(user))
    } catch (e: Exception) {
        emit(Result.Failure(e))
    }
}

I have two questions, if I may.

  1. Is there something wrong with this code? As it works fine when I compile.
  2. I saw some questions here, that recommend using collectAsState() or .collectAsStateWithLifecycle(). I tried changing userResult.collectAsState() but I cannot find that function. Is there any benefit in using collectAsState() or .collectAsStateWithLifecycle() than in my actual code? I'm really confused.
Joan P.
  • 2,368
  • 6
  • 30
  • 63
  • 1
    `collectAsState` is required when you collect the flow inside a Composable function. If you collect the flow inside a ViewModel and update state yourself, it isn't required. – Arpit Shukla Sep 20 '22 at 14:20
  • @ArpitShukla So is there something wrong in my initial approach? – Joan P. Sep 20 '22 at 14:20
  • In this question or the previous one? – Arpit Shukla Sep 20 '22 at 14:21
  • In this question. This is a follow up question. – Joan P. Sep 20 '22 at 14:23
  • Thanks @ArpitShukla So there is nothing wrong in emitting `Result.Loading` from inside the repository class? – Joan P. Sep 20 '22 at 14:25
  • 1
    Yeah you can do that. Although the way I do this stuff is that, I keep the repo function as a `suspend` function and return just success and failure from there and keep the loading logic in the ViewModel. But it's upto you. – Arpit Shukla Sep 20 '22 at 14:29
  • 1
    Returning success or error is part of business logic. It would be better to do it where you process your business logic. This is tradionally with clean architecture done in UseCase, in MVP in Interactor classes – Thracian Sep 20 '22 at 14:35
  • @ArpitShukla So, you say that with your solution, the function in the repository class doesn't return a flow? But what does it return then? Can you please remodel my code in the way you do, to handle the logic of progress bar inside the View Model class? I think that this is what I'm looking for. – Joan P. Sep 20 '22 at 14:35
  • `collectAsStateWithLifecycle` is for when you don't want to emit any result when your app is not in foreground or not in `RESUMED` state. Some apps need to emit updates even while they are not visible to users some are only need to update only when app is visible to user. – Thracian Sep 20 '22 at 14:36
  • @Thracian Thank you, for that. Since I'm not using MVP, not use cases, would it be better to that in the View Model class? Can you please provide me an example? I would like to upvote your answer. – Joan P. Sep 20 '22 at 14:37
  • UseCase is a class that doesn't contain anything related to Android. It can be Unit tested without Roboelectric or any libraries that require mocking Anroid framework – Thracian Sep 20 '22 at 14:39
  • Thank you, @Thracian. I really appreciate your help. All comments helped me a lot. If you want to add an answer with all of your comment, I'll more than happy to upvote it. – Joan P. Sep 20 '22 at 14:47

2 Answers2

2

If you wish to follow Uncle Bob's clean architecture you can split your architecture into Data, Domain and Presentation layers.

For android image below shows how that onion shape can be simplified to

enter image description here

You emit your result from Repository and handle states or change data, if you Domain Driven Model, you store DTOs for data from REST api, if you have db you keep database classes instead of passing classes annotated with REST api annotation or db annotation to UI you pass a UI.

In repository you can pass data as

override fun getUser(uid: String) = flow {
       val user usersRef.document(uid).get().await().toObject(User::class.java)
  emit(user)
}

In UseCase you check if this returns error, or your User and then convert this to a Result or a class that returns error or success here. You can also change User data do Address for instance if your business logic requires you to return an address.

If you apply business logic inside UseCase you can unit test what you should return if you retrieve data successfully or in case error or any data manipulation happens without error without using anything related to Android. You can just take this java/kotlin class and unit test anywhere not only in Android studio.

In ViewModel after getting a Flow< Result<User>> you can pass this to Composable UI.

Since Compose requires a State to trigger recomposition you can convert your Flow with collectAsState to State and trigger recomposition with required data.

CollectAsState is nothing other than Composable function produceState

@Composable
fun <T : R, R> Flow<T>.collectAsState(
    initial: R,
    context: CoroutineContext = EmptyCoroutineContext
): State<R> = produceState(initial, this, context) {
    if (context == EmptyCoroutineContext) {
        collect { value = it }
    } else withContext(context) {
        collect { value = it }
    }
}

And produceState

@Composable
fun <T> produceState(
    initialValue: T,
    key1: Any?,
    key2: Any?,
    @BuilderInference producer: suspend ProduceStateScope<T>.() -> Unit
): State<T> {
    val result = remember { mutableStateOf(initialValue) }
    LaunchedEffect(key1, key2) {
        ProduceStateScopeImpl(result, coroutineContext).producer()
    }
    return result
}
Thracian
  • 43,021
  • 16
  • 133
  • 222
  • 1
    Testing is useless, Doctor Thrace; once you get into it, you'll test if `1 + 1` returns `2`. The rest of your answer is helpful, as always... YEAH! I SAY TESTING IS USELESS BECAUSE I DON'T KNOW HOW TO WRITE TESTS!!! You don't have to roast me about it..! – Richard Onslow Roper Sep 20 '22 at 15:38
  • Maybe tests are useless especially if you manipulate your tests to pass for your code but you can also do test driven development by specifying your inputs and possible outputs then write your code. This way you can minimize errors. And test are i think most useful when someone else, or developer after you needs to do development that might effect your classes. Running tests make sure that code that already works isn't broken after recent development – Thracian Sep 20 '22 at 15:41
  • And i like behavioral development either when there are so many functions calling each other to make sure that only the required functions are called and number of times that they are required. MockK is very good at this. – Thracian Sep 20 '22 at 15:43
  • @RichardOnslowRoper a sample for BDD to check operation are executed in required or based on db is empty or dirty for instance. https://github.com/SmartToolFactory/PropertyFindAR/blob/5d2b510a8d1c489a5520b1fff6a143552e50ca79/libraries/domain/src/test/java/com/smarttoolfactory/domain/usecase/GetPropertiesUseCaseFlowTest.kt#L76 – Thracian Sep 20 '22 at 15:45
  • Wasn't expecting a three-comment reply. Why don't you obey the equal and opposite reaction law? Take two of them back and leave only the third! Doctors should know Physics... – Richard Onslow Roper Sep 20 '22 at 15:52
  • Share the link with me, I'll try to break it. Nothing more fun that trying all ways possible to break someone's code, then roast them on it. – Richard Onslow Roper Sep 20 '22 at 15:56
  • Let yourself wander free, Doctor Thrace. Deletion is weird... – Richard Onslow Roper Sep 20 '22 at 16:22
  • Hey @Thracian can you please help me on this [issue](https://stackoverflow.com/q/73789740/11560810) – Kotlin Learner Sep 20 '22 at 19:45
  • Hey. Maybe you can help me with [that](https://stackoverflow.com/questions/73853421/how-to-stop-collectaslazypagingitems-from-firing-when-itemcount-is-0) too. – Joan P. Sep 28 '22 at 11:59
1

As per discussion in comments, you can try this approach:

// Repository
suspend fun getUser(uid: String): Result<User> {
    return try {
        val user = usersRef.document(uid).get().await().toObject(User::class.java)
        Result.Success(user)
    } catch (e: Exception) {
        Result.Failure(e)
    }
}

// ViewModel
var userResult by mutableStateOf<Result<User>?>(null)
    private set

fun getUser(uid: String) {
    viewModelScope.launch {
        userResult = Result.Loading // set initial Loading state
        userResult = repository.getUser(uid) // update the state again on receiving the response
    }
}
Arpit Shukla
  • 9,612
  • 1
  • 14
  • 40
  • Thank you so much Arpit. This answer was also really helpful. Voted-up. – Joan P. Sep 20 '22 at 14:59
  • 1
    You know what's worse than being unknowledgeable? Being unknowledgeable while pretending to be knowledgeable. The user was using a wildly inefficient/"incorrect" method of retrieving data, and you didn't just ignore that, you added extra redundant code to it while leaving the original problem unresolved, all without even giving a proper explanation for what you suggest. Seriously, member. – Richard Onslow Roper Sep 20 '22 at 15:24
  • @RichardOnslowRoper Could you please explain what was so `inefficient/"incorrect"` in the original method and exactly what `redundancy` did my solution add? – Arpit Shukla Sep 20 '22 at 16:02
  • Sure, kinda busy right now. Read the state codelab till then. I'll post something if I get time. – Richard Onslow Roper Sep 20 '22 at 16:06
  • Hey @ArpitShukla can you please help me on this [issue](https://stackoverflow.com/q/73789740/11560810) – Kotlin Learner Sep 20 '22 at 19:45