Reinvigorating a large Android code-base

The problem

We faced a classic problem on a recent project. Stop me if you've heard this one before. We are supporting a well established product with millions of users, running on a large underlying code base, but the code base has been building technical debt for years, slowly turning into a Frankenstein, causing support problems. The types of problems we were facing include:

• Separation of concerns violations making unit test maintenance and creation more challenging, with lower than ideal coverage
• Threading complexity in places leading to defects that are difficult to find and repair
• Numerous special-case customizations distributed throughout the code, each addressing specific customer needs that only activate for that customer
• Reduction in team velocity due to code complexity and the other issues listed above

Challenges of Addressing Technical Debt

In one of my first posts I touched on the challenges of balancing time spent paying back technical debt vs time spent adding new features. It is always difficult to find opportunities to address these types of concerns in a code base because the investment of a large refactoring effort is very high and the direct customer value can be perceived as low (especially outside of engineering) and difficult to quantify when compared to ongoing targeted feature enhancements and defect fixes.

I find the best approach is usually to address refactoring in an iterative approach rather than tackling an entire code base all at once. In this fashion you can balance new feature development with the need to keep your foundation stable.

Solution Overview

On this recent project, however, we were given a task to refresh the entire UI. This was an unusual opportunity to re-evaluate the code base holistically and introduce new concepts and techniques.

We targeted three primary technologies to aid in this effort:

• Dependency injection using Dagger2
• Decomposition of Android activities into Model/View/Controllers
• Usage of RxJava and retrolambda for component communication and thread management

Over the next several posts I will delve into the approach we took, how each contributed to solving the problems identified above, and some of the challenges we faced. 

Delivering Highly Configurable Libraries Using Dependency Injection

In the last post, I described usage of a factory pattern to provide highly configurable functionality. An alternate approach would be the usage of dependency injection(DI). At the time I designed these libraries, dependency injection platforms on Android were not as mature as they are today. Guice was starting to gain some popularity, but uses reflection and can impact runtime performance. With the advent of Dagger2, we have access to a static dependency injection solution that does not rely on run-time binding. 

Using Dagger2, I could replace the previous abstract factory pattern. I decided to explore this solution as an exercise. Note that I did not actually implement and compile this solution and humans are notoriously lousy compilers, so it's possible (likely) there are errors. However, it should illustrate the general approach.  This also assumes that you are generally familiar with usage of Dagger2.

As commented in the code. To override this behavior, the user could provide their own graph and data module and call ApplicationDispatchHandlerDaggerSample.registerHandlers() at runtime.

While some might argue that the DI solution with Dagger2 is cleaner, it would impose an additional constraint on each user of the NAC API to download and learn its usage. This is a relatively minor cost, but you could make the argument that in the interest of keeping NAC as easy to use as possible, the Dagger2 approach would add complexity with no real end-user benefit. A bigger issue with this approach is that, since we don't know which modules the user will want to  inject a-priori, we inject them all. This means that a user would have to specify handlers from within the NAC library, which  the user should not even need to know about, and which we have been keeping private and obfuscated. There are probably ways we could fix this by exposing methods to get the default implementation for a class, but this seems to just add more unintentional complexity. My conclusion is that, while DI is an important and highly useful pattern, for  this  specific use-case, the pattern we chose was an overall better approach.

An Extensible Factory Pattern - Use Case

NuanceAndroidCore (NAC) was designed to be a highly customizable library, facilitating creation of Siri-like voice assistants, without imposing constraints on the look and feel of these assistants. The platform controls the voice dialog and provides callbacks to the UI layer for displaying recognition results and status of the recording and recognition. Consumers of the library render this feedback as they see fit.

The library also performs data lookups such as calendar events and alarms within a range of dates; and actions such as launching an application, adding/deleting/changing alarms, events, and playing music. Much of the benefit of the library comes from its ability to perform all this seamlessly and automatically. However, in many cases, individual customers might have their own implementations for one or more of these applications, with their own API's, and sometimes with added functionality. The challenge was to come up with the best way to support these customizations, control and manage the intricacies of the voice dialog, and supply default behaviors for everything else.

This is where the extensible factory proved to be useful. NAC used this pattern in several places, including creation of its action and lookup handlers. NAC registered its own implementations of each handler (for example a handler for sending an SMS message). Each customer could then customize specific actions by registering a different factory to override the default implementation, should they desire different behavior. In this manner, they could leverage everything else the library provides, including everything related to the voice dialog for that action, and any actions that didn't need customizations, and only provide code for specific overrides of the default behavior.

Each of the action handler factories are chosen from a map keyed off an element of the server response (for example, an action of type "sms" would key an action handler of type SMS). Building off the sample code introduced in the last post about dynamic enums, the factory method looks like:

(Disclaimer: This snippet was hand-edited from the original code and not compiled. The possibility exists for compilation errors.) 

This technique relies on reflection to build up the handlers, but performance is not a concern because the construction of the handlers is a very small portion of the overall time to perform an action, typically dominated by the time in getting recognition results returned from the server.

The voice recognition logic  and dialog management for this system is server-side.

So the opportunity exists for new voice domains to be added and deployed to the field that NAC is not configured for, after NAC has already been released. This is where the dynamic enum class discussed in the last post became useful. With the dynamic enum, NAC could register all its known handlers with immutable keys, while also supporting key types it doesn't know about. For example, the SMS key type would be defined and used by a customer wishing to override default SMS handling, but the customer could create a new key, say "sports" on the fly should the "sports" domain be added to the server domains after the NAC library has already been released.

In the next post, I will explore an alternate approach using Dagger2 for dependency injection.

Balancing Performance with Code Readability : Case Study (Android, Java)

Google has a number of blog posts on how to write code for Android performance. They make recommendations that conflict with normal Java standards such as avoiding enumerated types and using counter-type  loops over their collection types. They make good arguments for comparative performance improvements for each, but I think it's good to step back and evaluate Google's motivations before blindly applying all their recommendations.

• Motivation 1: Bad apps make Android look bad
  
  From Google's perspective, a few misbehaving apps can make the entire ecosystem look bad when compared to the highly controlled ecosystem of Apple. This is especially true for apps that are bad citizens (e.g.- consume more resources than needed for longer than needed, services that run longer than needed, activation of power-hungry resources like GPS hardware for longer than needed). Other recommendations are performance-oriented. There is little motivation for them to recommend well-written code over code that grabs every nanosecond of performance. They aren't the ones supporting the apps, after all.

• Motivation 2: Wide range of hardware capabilities
  
  Often times developers will test only on a high-speed device over WiFi or 4G and not understand the implications of how their app will run on older or less expensive hardware or over slow Edge networks in China or elsewhere. With the advent of smart devices like watches, the limitations can be even more severe, especially for  battery life.

While I strongly endorse writing programs that are good citizens, other recommendations to grab every nanosecond of performance I often find as not always necessary. I also have the luxury of architecting, designing, and developing apps for specific high-end Android devices, which makes motivation 2 less relevant. Even if this were not the case, I still prefer to write first for readability and, only when proven necessary, refactor for performance.

An example of where I recently found this  useful was in creating code whose function is coordinating an external set of contacts with the Android native contact database. There were a number of fields that were to be synchronized and the list of exactly which would be synchronized was subject to change. I found code in another application that someone wrote to do something similar as a reference. The author of this code created a string projection  where each ordinal was referenced by number into that projection. Insertions or deletions of values in this projection would change the meaning of each ordinal, making the code very brittle.

I introduced an enumerated type for storing the elements of the Android database string projection and relied on the ordinal position for later retrieving columns from this projection. The structure was thus "self-aligned" making for easy removal and addition of elements to the projection. The enum looked something like this:

Access to the columns, could then use the ordinal of the enumerated values like so:

instead of using "magic numbers" which rely on the order of the elements in the projection (in this case it would have been "1").

The new self-aligned approach is both much more readable and less susceptible to bugs as the code changes and there was no noticeable performance hit.