Standardize dependency injection across Collect

[jd-alexander]

Problem description

So we are actively working on making more components within Collect loosely coupled so that we can increase the testability of the app as a whole and also the ability to test individual components. We could say that Dagger is one of the major proponents of this as it provides us with a DI solution that has been tested and proven by our industry to accelerate proper TDD workflows. Therefore it's usage throughout the app has been on an increase as we are constantly building out complex feature requirements that have several dependencies and scopes and this is the reason I think we should begin an official exercise of making all of this standard within the app and contribution guidelines.

• One of the first things we have to do get this initiative going is to do a comprehensive review of SharedPreferences PR by @shobhitagarwal1612 as it's one of the most used components throughout the app and being to mock it would allow us to spin up test environments that require it a lot more easily.

• Next on the list is the utilization of subcomponents and component dependencies. While doing a review of the current refactoring taking place on Collect HttpClient, I realized that the refactor had a HttpComponent in it's midst. While utilizing that approach was a really good decision, there are some constraints associated with sub components that need to be addressed before it's introduction. Nonetheless, they are a good way to organize components and infrastructure along the lines of similar components, scopes and dependencies. Imagine a ApplicationComponent that contains all the general dependencies such as SharedPreferences and the ApplicationContext that all others components can inherit from, and all the SMS related code in aSmsComponent. This approach could lead to easier dependency management, testing and component lifecyle. Before we can introduce this into the codebase we have to take a detailed look at which would work better, whether it be subcomponents or component dependencies, the learning curve and how which layers would be easier to try with with first as a proof of concept.

• The flipside to this, is leaving subcomponents/component dependencies out of the picture and using the general ApplicationComponent approach that hosts all dependencies. This is the easier approach but it doesn't allow proper seperation of dependencies, scope and responsibilities.

• Similar to what we did for permissions, we could make a list of the most important components or areas that need to become injectable and then we could have PRs to do this. While doing this evaluation we have to take into consideration the impact the changes will have on the codebase i.e the risk for regression and also verify that the area needs to be tested more thoroughly.

To begin this, I am asking @shobhitagarwal1612 to do a review of the best approaches we can take to accomplish this and if we would be able to mobilize enough resources to ensure that these changes would be contributor friendly! We have already started using most of these concepts throughout so doing this would be a great way to get everything formalized!

Will update this with reading resources and more of my thoughts/research when I finalize them.

[jd-alexander]

@opendatakit-bot label "needs discussion"

[lognaturel]

In https://github.com/opendatakit/collect/pull/1871#issuecomment-435341476, @seadowg mentioned that he has DI/Dagger experience and is interested in helping clean things up and provide a model and framework for others to keep building in a consistent direction. It sounds like he and @shobhitagarwal1612 have started conversations around that.

I think what is really important is to make sure that there are resources provided to help contributors get a consistent mental model of the goals and structure of the architecture (what scopes to use when, where to inject non-Activity and non-Application classes, etc). There was already some of that (e.g. the comments at the top of AppComponent and AppModule), but it didn't get enforced (mostly by me, sorry!).

@seadowg, @shobhitagarwal1612 it's sounding like you think an illustrative example is a good way to start?

[seadowg]

@lognaturel @jd-alexander sorry I completely forgot to reply in this thread (we had discussed in #1871). I'm finishing up a PR with a reworked configuration for Dagger and some documentation around patterns.