android10 / android-cleanarchitecture-kotlin Goto Github PK

The actual feature package (login or movies) contain all class of the feature.
I think it doesn't reflect the "clean architecture" responsability split.

I know it's a sample project but like you say in the blog post :

Code/package organization is one of the key factors of a good architecture

I think feature package should at least contain the folowing packages :

• presentation
• domain
• data

I want to make a data class in order to send data via post...but internal declaration of Api interface not allows me to archieve this. If I remove internal declaration, it works, but Repository class shouldn't have to fill an ApiParams class.....

internal interface LoginApi {
    companion object {
        private const val LOGIN = "login"
        private const val PARAM_ACCOUNT = "account"
        private const val PARAM_EMAIL = "email"
        private const val PARAM_PASSWORD = "password"
    }

    @POST(LOGIN) fun login(@Body params: LoginParams): Call<LoginEntity>
    data class LoginParams(
            @SerializedName(PARAM_ACCOUNT) private val account: String,
            @SerializedName(PARAM_EMAIL) private val email: String,
            @SerializedName(PARAM_PASSWORD) private val password: String
    )

}

What do you think is the pros and cons of this repository and this one:

https://github.com/bufferapp/android-clean-architecture-boilerplate
https://github.com/bufferapp/clean-architecture-components-boilerplate (forked using arch components)

I am creating a new project and I need to choose one as a reference.

Hello.
The example here covers only fetching immutable data. But for me, the most challenging is creating and updating existing data. How to handle validation of mutable data, when there is high coupling between View and ViewModel.
Should I keep track of each field separately ?

    ...
    val id: MutableLiveData<Int> = MutableLiveData()
    val title: MutableLiveData<String> = MutableLiveData()
    val titleError: MutableLiveData<String> = MutableLiveData()
    val year: MutableLiveData<Int> = MutableLiveData()
    val yearError: MutableLiveData<String> = MutableLiveData()
    ...

and then how to validate ?

    fun createMovie() {
        var valid = true

        val title = title.value
        if (title == null || title.isBlank()) {
            titleError.value = "Title can not be blank"
            valid = false
        }

        val year = year.value
        if (year == null || year < 0) {
            yearError.value = "Year must be positive number"
            valid = false
        }
        ...

        if(valid){
            val newMovie = MovieDetails(
                    id = UUID.randomUUID().toString().hashCode(),
                    title = title!!,
                    year = year!!,
                    ...)

            createMovieDetails.execute({ it.either(::handleFailure, ::handleMovieDetails) }, Params(newMovie))

        }
    }

And then observe on each error ?

    override fun onCreate(savedInstanceState: Bundle?) {
        ...
        with(movieDetailsViewModel) {
            observe(titleError, ::handleTitleError)
            observe(yearError, ::handleYearError)
        }
    }

    private fun handleTitleError(erorrMessage: String?) {
        editTextTitle.error = erorrMessage
    }
    private fun handleYearError(errorMessage: String?) {
        Toast.makeText(context,errorMessage,Toast.LENGTH_SHORT).show()
    }

I'm not sure of any of those lines
And how to update title in ViewModel ? Via TextWatcher ? It becomes quite tricky, when it comes to update TextView from ViewModel's Observable and updating ViewModel from TextView's TextWatcher (recursive updates).

An example of creating new and updating existing movies would be great 👍

Steps to reproduce:

1. Switch off network connection
2. Open the app, show "No network connection" snackbar
3. Switch on network connection
4. Click "Refresh", movies show up
5. Then rotate screen, "No network connection" snackbar is shown.

The cause perhaps is that the MoviesViewModel holds the previous Failure value during configuration change.

What if a UseCase (interactor) is not meant to post (just) to UI?

Assume we have to add a GetMoviesNames interactor that returns only names. It makes sense that it would use the existing GetMovies interactor and then apply more logic to the result to get the list of names only. How would this be achieved with GetMovies only posting to UI? Is there a way to make posting to UI optional so that UseCases could be consumed by other UseCases in different thread than UI?

Are these models both part of the domain?

Reading the code I got this and thought asking about what is the best option or if it's the same for the following code in order to learn a little more. For big gurus of Kotlin, what about using companion object or constructor in data clases or clases, for example, following code:

data class Movie(val id: Int, val poster: String) {

    companion object {
        fun empty() = Movie(0, String.empty())
    }
}

Vs. This Is what I used to use

  data class Movie(val id: Int, val poster: String) {
     constructor():this(0, String.empty())
  }

"Unsafe" cast operator should be nullable
Extension networkInfo on context should return NetworkInfo?, otherwise causes runtime crash.

This derrives from the abstract getSystemService which is annotated as Nullable

Also, as per Kotlin documentation about "Unsafe" cast operator (can be found here) In order to match Java cast semantics we have to have nullable type at cast right hand side

Ways to reproduce, just launch the app with no network connectivity and it crashes with logcat:

    Process: com.fernandocejas.sample, PID: 6453
    java.lang.IllegalStateException: (this.getSystemService(C…anager).activeNetworkInfo must not be null
        at com.fernandocejas.sample.core.extension.ContextKt.getNetworkInfo(Context.kt:23)
        at com.fernandocejas.sample.core.platform.NetworkHandler.isConnected(NetworkHandler.kt:29)
        at com.fernandocejas.sample.features.movies.MoviesRepository$Network.movies(MoviesRepository.kt:37)
        at com.fernandocejas.sample.features.movies.GetMovies.run(GetMovies.kt:25)
        at com.fernandocejas.sample.features.movies.GetMovies.run(GetMovies.kt:22)
        at com.fernandocejas.sample.core.interactor.UseCase$execute$job$1.doResume(UseCase.kt:38)
        at kotlin.coroutines.experimental.jvm.internal.CoroutineImpl.resume(CoroutineImpl.kt:54)
        at kotlinx.coroutines.experimental.DispatchedTask$DefaultImpls.run(Dispatched.kt:161)
        at kotlinx.coroutines.experimental.DispatchedContinuation.run(Dispatched.kt:25)
        at java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1412)
        at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:285)
        at java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1152)
        at java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1990)
        at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1938)
        at java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)

Device used: Android Emulator API 26

Solution 1:
networkInfo extension should return a nullable type for method named networkInfo, Usages of networkHandler.isConnected should handle null case

Solution 2:
Wrap it as an Optional

Any reason why you moved away from data/domain/presentation packages for each feature?

Thanks!

For example, we have a usecase/interactor that needs to be used by other modules also:

VisitModuleUseCase - marks down the count on how many times does the feature/module is visited.

It will be use by all features (eg: movies, news feed, faqs, etc)

What folder should it go? Or should we create specific use case for each modules?

I thought UseCases shouldn't have any framework dependencies but PlayMovie has a Context in it and a Navigator which contains android specific code. I'm a little bit confused, am I missing something?

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:

• A UseCase queries a Repository for data.
• The Repository queries three different datasources (memory, disk, network) to retrieve results, some of them might not be available (memory not warm, disk doesn't exists, network not available)
• The View has to reflect the data as it comes in from the repository.

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?

Not sure if this is supposed to mean anything to you, but it was automatically captured during my build debug and tests yesterday.
No worries if you want to close it in case it is not relevant.

What do you think about returning LiveData instances directly from domain Layer?
I'm asking it because the one of the best features of Room framework is the hability to observe changes in DB directly.

So the LiveData instances should be returned directly from domain (usecase and repositories)?

Hello, i am willing to submit my design for your project.

Hello.
Here is a fork where I did migration from Dagger2 to Koin.
Take a look how much boilerplate code I was able to get rid of.
Don't you think that it's much simpler, easier, faster and cleaner way ?

• No regenerate & recompile required
• No annotations required
• self described Kotlin DSL

I have errors, using Pixel 2XL with Oreo 8.1

05-29 10:44:08.219 21446-21467/com.fernandocejas.sample I/zygote64: Explicit concurrent copying GC freed 14788(793KB) AllocSpace objects, 0(0B) LOS objects, 49% free, 3MB/6MB, paused 168us total 47.372ms 05-29 10:44:13.353 21446-21446/com.fernandocejas.sample E/BufferItemConsumer: [unnamed-21446-10] Failed to release buffer: Unknown error -1 (1) 05-29 10:44:13.403 21446-21763/com.fernandocejas.sample D/OkHttp: --> GET https://raw.githubusercontent.com/android10/Sample-Data/master/Android-CleanArchitecture-Kotlin/movie_038008.json http/1.1 05-29 10:44:13.426 21446-21451/com.fernandocejas.sample I/zygote64: Compiler allocated 7MB to compile void android.widget.TextView.<init>(android.content.Context, android.util.AttributeSet, int, int) 05-29 10:44:13.803 21446-21763/com.fernandocejas.sample D/OkHttp: <-- 200 OK https://raw.githubusercontent.com/android10/Sample-Data/master/Android-CleanArchitecture-Kotlin/movie_038008.json (399ms, unknown-length body) 05-29 10:45:01.262 21446-21446/com.fernandocejas.sample D/ViewRootImpl[MovieDetailsActivity]: changeCanvasOpacity: opaque=false 05-29 10:45:01.289 21446-21446/com.fernandocejas.sample E/BufferItemConsumer: [unnamed-21446-11] Failed to release buffer: Unknown error -1 (1)

Hey all,
I've seen entities and
repositories defined inside the feature, and I thought at first that it is not ideal. Why?
Because it will be used in other modules (features) and failing at the principle of isolation.
So I did what the core package did, I've made a data package for the whole app to use all entities, repositories, etc.

What do you guys think of this? Pros and cons?

Hi @android10, long time no see :)

I really enjoyed reading the new post, this seems pretty lean but also robust approach.

Wanted to ask about the Either type. I had a "Result" class in my mind of the following:

sealed class Result<out T : Any>

sealed class Failure(val throwable: Throwable) : Result<Nothing>()

data class Success<out T : Any>(val value: T) : Result<T>()

fun test() {
    val result: Result<String> = Success("test")

    when (result) {
        is Failure -> print(result.throwable)
        is Success -> print(result.value)
    }
}

For me this has a nicer syntax (also could add the either extension methods with the blocks) and I don't see the benefits of the Either after thinking for several minutes. Of course this is up to personal taste I just wanted to ask for an opinion or check whether I missed some cool aspects between the lines :)

I am not in a so good level in using kotlin, so maybe i am missing something, but are not we losing some capabilities by not using streams of data?

Hi Android10,

Maybe someone help me, because when i use BaseFragment.close() it's not working. And if I watch on backStackEntryCount it's always zero.

Many Thanks.
LEO

Now that you've refactored this into a single project (no longer a multi-project build) why keep dependency declarations in dependencies.gradle.

Moving them into the app/build.gradle would allow their references to integrate better with Android Studio as well as provide notifications when key dependency versions are updated. In its current form, those notifications are lost as the IDE doesn't follow up the gradle graph of build files.

I have seen that you created a MovieView with the same parameters as the Movie model, having in mind that the View shouldn't know anything about the models, following the pattern, but, I have a doubt if that can be an overuse of classes, I mean, that's ok to apply the pattern as it is, isolating each layer, but if the app is so big that can be a mess of classes everywhere, is that ok btw?

I am currently porting my project from the java Clean Architecture structure to the Kotlin structure. I am using the Cloud Firestore database and in my previous project I had a snapshot listener that would emit data every time new data was added to the database. The presenters would be notified by each update (via RXJava observables) and then update the UI.

From what I can see the MoviesRepository.movies() function has to be invoked by the MoviesFragment manually to see any updated data.

Am I correct in assuming this new architecture no longer supports real-time model updates? Has anyone adapted this solution to support reactive streams without RX Java?

Thanks for the help!

Hi,

In my project, I have a dependencies.gradle file. In this one, I have differents arrays of dependencies with corresponding version.

I would like to know if is there a way to have automatic update available version for dependency with this way ?

For example, if i declare com.android.support:appcompat-v7:27.0.2 in build.gradle directly, it'll tell me that a new version is available. Actually, it's declared in my dependencies.gradle like this :

appCompat : "com.android.support:appcompat-v7:$appCompatVersion"

and the version is in the same file, define above like this : appCompatVersion = "27.0.2"

Android Studio doesn't tell me that a new version is available. I'm looking on internet but didn't find anything. Does any one know how I can proceed to achieve this ?

Hi Android10,

I enjoyed reading your article (https://fernandocejas.com/2018/05/07/architecting-android-reloaded/), but had a question about sharing fragments and viewmodels. Where would you recommend is best to store common functionality such as a bottom navigation bar fragment?

Thanks,

Anil

Hi,
Currently I found that You do it this way

val job = async(CommonPool) { run(params) }
launch(UI) { onResult.invoke(job.await()) }

Can we just create one coroutine here and just do context switching to execute the UI block code, maybe something like this

launch(CommonPool + job) {
    val result = async(coroutineContext) { run(params) }
    withContext(UI) { onResult.invoke(result.await()) }
}

And the job on this sample is a private property for cancellation purpose.

By the way, thank you so much for always providing us with such an awesome resources. 👍 🙇

For me this project is interesting because it is based on viewmodel pattern
I see it uses livedata, observe: very cool!
But I would have expected to see the binding like

    android:layout_height="wrap_content"
    android:text="@{user.firstName, default=my_default}"/>

(from https://developer.android.com/topic/libraries/data-binding/expressions)
where are they in such project? Thanks in advance for your hint!

Regarding NavigatorTest:

@Test fun `should forward user to movies screen`() {
  whenever(authenticator.userLoggedIn()).thenReturn(true)

  navigator.showMain(activityContext())

  verify(authenticator).userLoggedIn()
  RouteActivity::class shouldNavigateTo MoviesActivity::class
}

if you switch MoviesActivity with LoginActivity the test still passes

you said

At implementation level, in our example, MVVM is accomplished by the usage of Architecture Components, which its main advantage is to handle configuration changes when the screen rotates, something that has given us many headaches as android developers (I guess you know what I’m talking about).

Disclaimer: that does not mean we have to no longer care about lifecycles, but it is way easier.

ViewModel was created to handle the configuration / lifecycle for you.

viewModel
viewModel Lifecycle

Default error handling was not implemented and I was waiting for your solution.

This is a possible way to manage it. Feel free to close it but I would appreciate your opinion.

I'm not sure if I am breaking SOLID principles but show loading and error message tasks
are very related, and are repetitive tasks that I prefer to move to the base classes.

Now you can pass lambdas to the base disposable observer and add default error handling.

My app uses nullable views so I improvised a solution with lateinit a little strange for me.

Edit: I found this related article for Java:
RxJava2 and Retrofit2 Error Handling on a single place

In a real-world app, there will be many features. Currently the ViewModelModule is responsible to provide all the ViewModels, is it better that each feature gets its ViewModels from its own component instead of the ApplicationComponent?

Do we need to cancel coroutine job in UseCase when ViewModel destroyed (onCleared())?
As was done in previous implementation with Rx observables, which was disposed in Presenter.stop()

Hi @android10

When I compared it to the java version of this project, there are multiple java modules used (data, domain, presentation) but here, in the kotlin project, it only uses one App module.

So is it correct to assume that the better way to architect projects is to use only one module? or is it a limitation currently for Kotlin?

Thank you

Thanks for this great repo. Can you add pagination? It could be perfect.

Assertions made inside the observeForever block are always successful, even when they shouldn't.

For example in MoviesViewModelTest, if i say that the first element poster is equals to "IronMannnn", it will pass, even if the real value is "IronMan".

Hi there,

I found an issue running the example into android M version in Motorola Moto G (3 generation), the issue was related with Youtube Intent when i'm trying to run the movie on the project. The app crash immediately.
The obvious solution I toke was check the version and add Intent Flag for this one. Navigator.kt --> createYoutubeIntent function.

if (android.os.Build.VERSION.SDK_INT <= android.os.Build.VERSION_CODES.M)
intent.flags = Intent.FLAG_ACTIVITY_NEW_TASK

I don't know if this is the best solution... but works, the best part of this is running same project with other phone with Android O works great, so there is a difference in stack related with application context injected.

first of all great job @android10 . I discovered this issue while running the app.
Issue scenario:
apply plane mode -> run app -> No network state is present -> disable plane mode - > click snackbar refresh action - > movie list is present -> rotate screen -> No network state is present.

Hi,

hope find you well with this cold call.

I am an author of mocking framework for Kotlin

I see you are using mockito-kotlin.

I just want you to be aware that there is solution that fully supports Kotlin and ask to try it in your new/current projects.

I can help you if you answer to this issue.

Thanks and please star it

Did you think to update this project and make a cache policy? Because here you always make a data request, nothing is cached? You can use a MediatorLiveData for this one.

Or in general,
what modifications should be implemented to support new CoroutineContext in UseCase and from calling side ?

Hi,

How do you add integration with SDKs that deliver the result to onActivityResult method of fragment/activity? For example if you'd like to implement Authentication via Facebook SDK you would:

1. Create a CallbackManager
2. Register it via registerCallback() of LoginManager and pass a FacebookCallback there,
3. Pass the arguments received in onActivityResult() to the callback manager created in the step 1.

Then if you call LoginManager.getInstance().logInWithReadPermissions(fragment/activity, permissions) you will get your callback called with the result.

Besides of that, you shouldn't forget about unregistering callbacks when you're done. All of this brings quite strong coupling between components. Are there any ideas on how to keep the Dependency rule in place?

Here is some explanation about why we must use ListAdapter.
This project is so good. Thanks for this project and explanations. But if you use diffutil in adapter, you can get some good skills. Recycler.Adapter and diffutil is little bit complex and have some boilerplate code. ListAdapter have some good solutions about this. Details in link. Please read that article.

Hi,
I'm working on the app which has a complex usecases which require multiple actions to be performed in order to usecase to be completed.

Lets, for example, say that I want to implement File deletion feature in my app such as Dropbox. This requires this app to firstly do a deletion of some file metadata from a database table and after it, to delete a file from the storage.

So far, we placed all of this logic to datastore class, but it just does not fill right for me, because I think there is no reason that datastore should know about file storage.

So, my question is what would be a better practice, to move this complex logic to Use Case class and there to sequentially execute calls to the Repository and File storage or to keep it in datastore class?

@android10, I'm looking forward to your opinion.
Best regards.

Hi,kindly implement features with Intent Service,Receiver etc.how we should call Background service ,things like detecting Network presence and starting Intent Service

Like useless, I don't see how it's generated. @android10

I learned a lot by reading this architecture. But in my real life project, viewmodel needs to run 2 or more usecases in prallel and join the result to update the ui. With coroutine in my current code, my code is like this

val job1 = async {usecase A}
val job2 = async {usecase B}
processResults(job1.await(),job2.await()) // This combine two result and do some calculation here

However, For Either class with either() method, I don't know how to achieve this.