First off, sorry if something is heavily contradicting what you've done already, but I still wanna leave my 2 cents here, if you find time for that in the future to address that. If not, then this will be a todo list for my future self :) Regardless, everything here is purely subjective and discussable, as always.
A follow-up from our e-mail discussion:
-
Flows instead of LiveData: you could take a look at this article as an example of how to use StateFlow with ViewModels. And here's an example on how to bring Flow to native. Not a must of course, just if you find time.
-
About interactors: I'm not sure what did you mean by "sources are uncontrollable black boxes", when you are actually the one who's defining and implementing them :) IMHO it's just a yet another unnecessary layer (because essentially interactors = usecases, you can also check internet) that bloats your code. I think it's completely fine to let repository combine data sources directly, and I'm not sure what kind of boilerplate can be there, would love to get an example of that.
-
Repositories as data layer - makes sense in the current structure, yep, but I'd still advocate to have them as data providers. I'll just drop here how I see that (bear with me, I spent ~5 mins on that :D):
-
Invoke in usecases: check this as an example, as a bonus you can invoke your usecases as plain functions (e.g.
val createProfile: CreateProfileUseCase; createProfile()
)
Re. the threading - gotcha, was confused by the network
, but actually background
is also not the best approach I believe. Ideally you should inject some kind of DispatcherProvider
and use it inside withContext
, so you can easily replace it in the tests, but not enforce everyone to use the default dispatcher :)
Now to the other things:
- Not sure what's the intention behind having an interface of interfaces (e.g. Daos, UseCases and so on)? If it's about implementations, I usually prefer to have an interface and a default implementation (so, as opposed to in tests, you can have fake/mock impl), like
CreateProfileUseCase.Default
- I think there's too much abstraction (too many base classes), but that's fine can be easily cleaned up later
- I saw that you added databinding and already removed it - that's cool, I wouldn't recommend adding it again - we had huge issues with build times at Runtastic with that, and still have, because kapt is very slow, and I try to avoid it at all costs usually (probably Dagger is the only kapt-lib which is worth it)
- In the spirit of no-kapt, I wouldn't also use Epoxy, but rather user Groupie - as it's more lightweight and easier to use and is kapt-free
- I usually try to avoid Fragments and their funny backstack, and also didn't really use the navigation library much - but is there a reason to have an Activity, which does nothing else, but only hosts a single Fragment? If there's no strong reasoning - I'd rather go for just an activity, w/o this single Fragment. I know it's probably hard to change your usual approach, but I really felt myself how fragments can be painful at scale :D
- ViewModels should be also injected into the activities/fragments, otherwise you can't really mock/fake their dependencies in the instrumentation tests (but this is later I guess)