Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add Dagger and architecture classes (getodk#1678)
Here's a look at the new dependencies and some reasoning behind them: Android Architecture Components implementation "android.arch.lifecycle:extensions:1.0.0" We should be moving to a MV* architecture for future Activity/Fragment/View development. The current popular options for this are MVP, and MVVM. With either of them, we have to do one of two things: Build our own internal MV* system so that the management of the P/VM component is standard across the project. Use some sort of external MVP/MVVM framework, like Mosby, or Architecture Components. I figured using Android's Architecture Components was the best choice here: its a "standard" implementation, and its unlikely to go anywhere. It also allows us to persist the VM across Activity config and state changes. Dagger implementation 'com.google.dagger:dagger-android:2.11' implementation 'com.google.dagger:dagger-android-support:2.11' annotationProcessor 'com.google.dagger:dagger-android-processor:2.11' annotationProcessor 'com.google.dagger:dagger-compiler:2.11' The biggest pain point with the Geo refactor is that we currently have 6 separate Activities, even though technically the majority of functionality is shared, and the only differences are how the data is stored (point, shape, trace) and presented (Google, OSM). We could go the route of creating a class hierarchy here that contains the shared functionality and then overrides them for each subtype, (perhaps Geo -> GeoPoint -> GeoPointGoogle, or Geo -> GeoGoogle -> GeoGooglePoint), but this means a rather arbitrary hierarchy and a lot of overriding. Ideally, we solve this problem with composition instead of inheritance. We really only need one Activity, with the components that change chosen based on what combination of Point/Shape/Trace and Google/OSM we have. That way instead of having six Activities with unclear division of responsibilities, we have one with clear, discrete modules that can be mocked for testing purposes. While this approach is preferred, it quickly becomes difficult without DI, as our dependency tree gets quite complex very quickly. Sometimes a deeply embedded dependency will need access to something further up the tree—like our LocationClient needing access to a Context for instance—and the only way forward without DI is to either: A) Pass the context down through every dependency layer (i.e. via the Constructor), which quickly becomes impractical. B) Reach out via a static method to pluck a Context from the void like we do with Collect.getInstance. This results in embedded dependencies and makes it more difficult to mock and test. The biggest cost to Dagger is getting the boilerplate setup, and understanding it. I've put things together in a way that there really are only three classes that should ever be modified: AppModule Here we can add @provides methods if we want to inject dependencies into the Collect class itself. These instances will be rare, but could still be useful. For instance we could change: // share all session cookies across all sessions... private CookieStore cookieStore = new BasicCookieStore(); // retain credentials for 7 minutes... private CredentialsProvider credsProvider = new AgingCredentialsProvider(7 * 60 * 1000); To: @Inject private CookieStore cookieStore; @Inject private CredentialsProvider credentialsProvider; And then add to AppModule: @PerApplication // (More on this in a second). @provides public CookieStore provideCookieStore() { return new BasicCookieStore(); } @PerApplication @provides public CredentialsProvider provideCredentialsProvider() { return new AgingCredentialsProvider(7 * 60 * 1000); } Which would allow us in a test setting to provide different values, or mocks, etc. ActivityBuilder Here we can add new injectable Activities, just by adding a new bind method, i.e.: @PerActivity abstract OurNewActivity bindNewActivity(); We can then either have OurNewActivity extend InjectableActivity, or simply call AndroidInjection.inject(this) in the Activity's onCreate. Since Dagger injects at compile-time, it needs to know which specific classes will be injected. If we don't provide specific bind methods here, we have to manually call inject at the furthest subclassed level in order for it to see all of the @inject-able fields. ViewModelBuilder Here we can add new injectable ViewModels. Just like ActivityBuilder, we just need to copy the bind method like so: @BINDS @Intomap @ViewModelKey(OurNewViewModel.class) @PerViewModel abstract ViewModel bindNewViewModel(OurNewViewModel newViewModel); Because the VMs are retrieved from the ViewModelProviders class, there's a bit more boilerplate involved in retrieving one already injected. If you can, overriding the MVVMActivity/MVVMViewModel classes will handle this boilerplate for you. If you can't, you just need to add an @Inject ViewModelProvider.Factory factory; field to your Activity, and call ViewModelProviders.of(this, factory) in your Activity to retrieve the injected VM. Technically, these are the only three classes that should ever be modified moving forward. Everything else was one time set up that should be good to go until a new Dagger release comes out with a feature we want to make use of. Here's some info on the remainder of the Dagger classes: Scopes Any dependency or @provides method annotated with a scope will act as a singleton within that scope. Dagger keeps track of these scopes and doesn't let an app provide/inject dependencies at a scope level above our current one. Injected objects without a Scope will be newly created every time they are injected. Right now we have three scopes: PerApplication Lives the entire time the app does. Our "parent" scope. PerActivity Lives the entire time an Activity does. Good for dependencies that depend on an Activity or Activity Context. PerViewModel Lives the entire time a ViewModel does. Keep Activity Context dependencies out of these as the VM may outlive the Activity. These may sound a bit confusing, but they're basically just helpful annotations to create Singletons that only live at a single level of the application. Basically if its something that's going to live in your VM, add @perviewmodel, and then any time that class is Injected, the single shared instance will be used. If you don't add a scope, there's no issue! It just means you won't be able to share that specific instance across multiple dependencies. We can always add more Scopes as we need them. Some apps use them to define things that should live as long as a User is logged in, or a Session is open, etc. For now though, we're probably good with these three. AppComponent Acts as the very top node of our dependency graph. Every dependency managed by dagger is a part of this Component. Binds our Collect instance to the "Application" in the graph, and then injects any App level dependencies (any fields in Collect marked @Inject). Shouldn't be modified at this point, pure boilerplate. ActivityViewModelProvider, ViewModelFactory, ViewModelFactoryModule, ViewModelKey These classes handle the magic behind making sure the VMs we add to ViewModelBuilder are properly constructed and injected. They really shouldn't be modified at this point. RxJava implementation 'io.reactivex.rxjava2:rxandroid:2.0.1' implementation 'io.reactivex.rxjava2:rxjava:2.1.6' Rx allows us to define the boundaries between our dependencies as unidirectional, composable flows of data. I don't have a great way to describe what this looks like right now without examples, but when the refactor is done I'll comment again with some. Basically without Rx on the Geo Activities, we end with the state-based, callback heavy spaghetti that we have now. Its very hard to know what effects what, as the data is moving in multiple directions. RxRelay implementation 'com.jakewharton.rxrelay2:rxrelay:2.0.0' This just adds a slight improvement on Rx "subjects". We can remove it but I'd prefer not to. Simple library. RxLifecycle implementation 'com.trello.rxlifecycle2:rxlifecycle:2.2.1' implementation 'com.trello.rxlifecycle2:rxlifecycle-android:2.2.1' implementation 'com.trello.rxlifecycle2:rxlifecycle-android-lifecycle:2.2.1' When dealing with Rx, we have to manage which data flows we're observing, otherwise we risk memory leaks. RxLifecycle allows us to make a simple .compose(bindToLifecycle()) call anytime we're observing something within an Activity or ViewModel and the binding will be cleaned up for us. ButterKnife implementation 'com.jakewharton:butterknife:8.8.1' annotationProcessor 'com.jakewharton:butterknife-compiler:8.8.1' Previously I thought we would go with the built-in DataBinding library, but it doesn't look like its being maintained much anymore. Further, whenever there's an error in the binding code, it breaks the entire build and spits out an enormous list of errors. Combing through these to find the actual error is extremely time consuming and demoralizing. Butterknife is a super simple way to achieve the same thing with almost no overhead. We just annotate fields and methods with @BindView(R.id.our_view_id) View view; or @OnClick(R.id.our_View_id) public void onClick() {}, call ButterKnife.bind(this) (this can be handled from a base class) and we don't have to deal with findViewById() calls anymore.
- Loading branch information
1 parent
44699ea
commit 9b7cabc