android10 / Android-CleanArchitecture-Kotlin

This is a movies sample app in Kotlin, which is part of a serie of blog posts I have written about architecting android application using different approaches.
https://fernandocejas.com/2018/05/07/architecting-android-reloaded/
4.69k stars 932 forks source link

UseCase.run returning multiple values? #15

Open robertoestivill opened 6 years ago

robertoestivill commented 6 years ago

I like the approach overall. I have implemented a very similar pattern in my app and has been working fine.

However, there is one use case that I never know how to appropriately model. I found that the method UseCase.run returning a single value might sometimes be restricting functionality and can always not be the best option.

Let me clarify with an example:

The problem is that the UseCase.run returns a single value, so in the example the UseCase will return only when all the datasources in the Repository have returned the data.

A simple work-around is to execute the UseCase three different times with different parameters that specify which datasource should be query. However, this does not sounds like a good separation of concern, as the Presenter will have to specify parameters corresponding to the datasources that need to be queried.

Another alternative (hack) is for the UseCase.run to return a Either<Failure, UpdateableType<>> where UpdateableType is similar to an rx.Observable. In this case the UseCase will return right away, and the Presenter will subscribe to data updates as they occur. This also feels ugly, as it will be hard to represent a Failure once the UseCase has returned.

Another way of solving it this problem that I have recently started to look at, is to use coroutines Channels so the UseCase can send multiple values to the Presenters, but i'm not quite there yet to propose a working solution.

What do you think?

Zhuinden commented 6 years ago

You could return an Rx Observable which can portray 0 to N items

robertoestivill commented 6 years ago

@Zhuinden you mean return it as a Either<Failure,Observable> ?

If that's the case, it will be really hard to ever get a Failure, as errors will not start happening after the Observable is subscribed to, and therefore, after UserCase.run has returned to the Presenter.

Also, I would love not to have to include rxjava just for this single scenario.

Zhuinden commented 6 years ago

Either<Failure,Observable>

that looks pretty bad as observable is already a monad and so is either :|

I guess I didn't think this through

android10 commented 6 years ago

I'm not fully understanding your problem.... can you elaborate a bit more?

If I were to use RxJava, I would get rid of Either for the same reason @Zhuinden mentioned in the above comment. Plus I would have to change part of the frameworks of course.

You have to evaluate how much value brings these kind of frameworks (RxJava) bring to your codebase. If it is only for a corner case, maybe that is not the way to go.

robertoestivill commented 6 years ago

Sorry for not being clear.

I do not want to introduce rxjava, actually i'm trying to drop it.

However I don't seem to find a way to model a scenario where the use case needs to return a value more than once to the presenter.

In the example I gave above, the use case could return 3 values to the presenter (Memory, Disk, Network) before finishing it's execution. How do we provide this values to the presenter without stopping the use case execution?

Thanks

Zhuinden commented 6 years ago

By using something that can represent an asynchronous event stream.... Like RxJava Observable :smile:

reinaldomoreira commented 6 years ago

I think you will have some kind of subscriber, Observable looks good to me, maybe LiveData<> fits your need. You could return an wrapper class for error/data. Just a suggestion :), in fact, this is used in some of Google's sample.

robertoestivill commented 6 years ago

Aligned with my first comment, I think something like the following should solve the problem based on coroutines Channel's and producers

~This has not even been compiled, so be aware.~ An JVM example is available here

abstract class UseCase<out Type, in Params> where Type : Any {

    abstract suspend fun run(params: Params): ReceiveChannel<Either<Failure, Type>>

    fun execute(onResult: (Either<Failure, Type>) -> Unit, params: Params) {
        launch(CommonPool) { 
            run(params).consumeEach {
            withContext(UI) {
                   onResult.invoke(it)
            }        
            }
        }
    }

    class None
}

class SomeUseCase : UseCase<String, None> {

    override suspend fun run(params: None) = produce<String> {
         send(Either.Right("Something from memory"))
         delay(500)
         send(Either.Right("Something from disk"))
         delay(5000)
         send(Either.Right("Something from network"))
    }
}

We really need to stop thinking about how problems are solved in rxjava, if we want to get rid of it :)

reinaldomoreira commented 6 years ago

Why get rid of something that is here to help? I Don't think you should use rxJava if you want only some corner cases, but, what you are asking, is basically the purpose of RxJava. I think you should stop trying to avoid a lib. Just my opinion.

robertoestivill commented 6 years ago

@ViperAlpha nobody is getting rid of anything. rxjava is not a dependency of this project and according to the blog post it wasn't a requirement to use it.

There are several reasons why to avoid rxjava but that's worth a completely different discussion.

The goal of architecture design, from my point of view, is to set basis that are generic enough to accommodate present and future functionality in a clean and consistent manner. This is exactly what this issue is about: question how does this architecture facilitates the multiple return use case without having to take it apart.

jaychang0917 commented 6 years ago

IMHO, a UseCase returning multiple values would complicate the whole architecture. Btw, for your example mentioned, designating a data source as the single source of truth would be better than returning multiple values.

NilsLattek commented 6 years ago

The problem is that we cannot observe our database using livedata, because we are not working with LiveData in the UseCase layer. (for good reasons) This contradicts with what Google recommends here https://developer.android.com/jetpack/docs/guide in the UserRepository: they immediatley return the current state of the database and once the webservice is finished you are going to update the database and the liveData object is going to update automatically inside the viewmodel. This is currently not possible with our usecase approach:

So as somebody mentioned before we would need to return some sort of observable for this.