Skip to content

Instantly share code, notes, and snippets.

@romtsn
Last active June 9, 2020 08:55
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save romtsn/d4221976b88bf9ccf5c9f101484883a1 to your computer and use it in GitHub Desktop.
Save romtsn/d4221976b88bf9ccf5c9f101484883a1 to your computer and use it in GitHub Desktop.

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:

  1. 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.

  2. 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.

  3. 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): Arch_draft

  4. 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment