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. 

Integration Test - Case Study

In the previous post I discussed an integration-test framework that I developed for testing system  interactions with a java  master controller. Recall that this system consists of separate View, Master Controller, and Model/ModelController components.

A basic flow through the system would start with an action in the View, pass through the Master Controller to the Model/Model Controller, pass back into the Master Controller, and then terminate with responses back to the view and the model controller.

For this case I configured Dagger2 to create a mock of the manager that interacts with the view (see mocked view in the diagram). I used the mock to verify that the correct method and method parameters are called at the end of the flow. I also wanted to verify that the correct methods and method parameters are called on the Model Controller, but I could not mock the Model Controller because it is integral to the flow we are testing. Instead, the test spied on the model controller to verify the expected method and method parameter calls.

Test flow (from diagram):

1. Initiate an action into the controller to mimic a user interaction from the GUI (View)
2. The Master Controller interprets the input action as an action needing interpretation and dispatch to the Model Controller and performs these actions. I used Mockito to verify that the correct Model Controller method was called and that the correct parameters were supplied.
3. The Model Controller interacts with the Model, processes the action and sends its own command back to the controller. I verify that the correct Master Controller method is called with the correct parameters. If the correct signature is seen, it means that the following components are working properly for the flow under test:
1. The JNI layer for passing between the Java Master Controller and the C Model Controller for these interactions
2. The Model Controller logic for this action 
4. The Master Controller then interprets the action from the Model Controller, dispatches an action to the View to inform it what to display, and replies to the Model Controller with a confirmation able to successfully interpret and handle the command. Both these terminal responses are also verified.

One added complexity with testing this integration is that there are several locations where the flow crosses thread boundaries. I needed a mechanism for delaying our verifies until a callback is received. The integration test will take much longer to run than a typical unit test (on the order of 15 seconds), but this does not mean that we can get sloppy and use sleep statements. To get reliable results, sleep statements would add more delay than necessary, and could still result in tests which are not repeatable. It’s almost never a good idea to sleep in an automated test...or in any code.

Mockito provides the doAnswer() method which is very helpful for mocking asynchronous responses from an object, but this is not helpful to us because we are testing objects which we can’t mock. What we need is a way of synchronizing with the asynchronous callback, because the code under test relies on that callback completing before it can proceed. Java provides such a mechanism with the CountdownLatch. The CountdownLatch will block until its counter reaches 0. We can setup the CountdownLatch in the test thread and give it a value of 1. In the callback thread, we reduce the count to 0 to unblock the test thread when we perform the callback, allowing it to proceed with its verification. We attach a timeout to the latch in case the callback is never hit (15 seconds in this case). This is much better than a sleep statement because the timeout condition is only hit on a failure condition. For this reason we can choose a large number that we know will pass under all conditions, without actually delaying the test execution time except for the exceptional condition of an actual failing test. 



This solution is still not ideal. We had to instrument the test with a handler that overrides the implementation we are testing to call the implementation’s body and then countdown the latch. I could have used DI to inject this test handler, but in this case the framework already had a mechanism for registering handlers directly, so we were able to register the test handler this way.

Developing an Integration Test Framework - Case Study

In the last two posts I described a test strategy and unit tests for a controller written mostly in java. The Master Controller interfaces with an http server hosting the View and a Model/Model Controller written in C. This Model/Model Controller was developed by a different software group and is like a black box for our purposes. I wanted the capability to test both our C/JNI layer and also test drops of the Model/Model Controller from the other software team. Integration tests provided me with the capability to accomplish both these goals, while also testing interactions between the modules unit-tested in isolation. I could test these interactions from the Master Controller with java test frameworks by observing expected system responses to stimuli, removing the need for an entirely separate C-based test framework. 

The Master Controller code splits its core functionality between "managers" with specific responsibilities, including handling boundary crossings to the View and to the Model/Model Controller. Each manager is housed in a container class. This is an excellent spot to introduce dependency injection for test components, as shown in the ManagerControllerSample.java below: 

I created a test framework that allowed for each integration test to determine for each manager whether it would be implemented as a mock, using the standard implementation, or using a spied version of the standard implementation. With Dagger2, I was able to inject the appropriate component (test or production) at run-time. In Graph.java we see the dagger graph. IntegrationTestBase.java  specifies the list of managers to mock and spy as seen in the call to initGraph(), and ManagerTestDataModule injects the appropriate manager type (standard, mock, or spy):


In the next post I will discuss an integration test that used this framework and a technique for synchronizing callbacks spanning thread boundaries.

Creating an Automated Test Strategy - Use Case

Background

On a recent project I was working on a team developing a software component as part of a larger system. This component can be thought of as a controller in a distributed MVC system, if you think of the model as having its own embedded controller. Our controller (the Master Controller) was written in Java. We had a JNI layer for interfacing with the C-based Model/Model Controller assembly and communicated with a web server hosting the view. I wanted to develop an automated test strategy that would:

• Validate the Master Controller
• Guard against regressions in in the Master Controller
• Validate system interactions
• Allow us to develop functionality independent from schedules/delivery of new functionality in the View and Model/Model Controller components

While most of this could be accomplished with unit tests, I decided we also needed some level of integration test for validating the system interactions. In the following description, bear in mind that the Master Controller is the device under test (DUT) and it interfaces with the View and Model/Model Controller but does not test them directly.

Strategy

It is always a good idea to discuss and document your test strategy. It was even more important for this project, as the other developers on the team were not as familiar with standard automated test tenets, coming from environments where automated test was not a priority. I went about this by performing the following steps:

• Develop and document the proposed strategy on internal project wiki
• Gain team buyin
• Implement integration test framework
• Create tests exemplifying usage for a variety of different types of modules
• Provide training

Our overall strategy was to rely on unit tests for the majority of functionality test, but provide a powerful integration test environment and a small number of tests validating interactions between these components.

Unit Test

There are some widely adopted principles for creating unit tests. For the wiki and the training, I put together a few simple patterns/anti-patterns. If you are at all familiar with unit test, there will be no surprises here:

Do

• Test each module in isolation
• Test edge conditions
• bad or null inputs
• etc
• Keep tests fast
• Ideally milliseconds
• Keep cyclomatic complexity low

Don't

• Allow timing dependencies other than timeouts
• No sleeps/timers
• Span threads in one test
• Depend on other tests or order of execution
• Leave artifacts
• Use @After methods (jUnit) to ensure that artifact cleanup will happen independent of test failure

I chose jUnit, Mockito, and PowerMock as the tools for our unit test. This decision was based on familiarity with the tools, their popularity, and suitability for usage within our system. Mockito performs most of the mocking functionality we needed. Mockito allowed us to:

• Handle external dependencies easily
• Good unit tests only test the module under test, none of its dependencies
• Mock responses from these dependencies
• Verify the method calls on these dependencies, including
• Parameters passed
• With stock matching algorithms or custom validators
• Number of invocations
• Verify that method that shouldn't be called are not called
• Mimic asynchronous callbacks from mocked objects

PowerMock provided us with the key additional capability to mock static method calls.

Integration Test

Although unit testing covers the bulk of the test for the product, it is also useful to validate interactions between the components. In the next post I will describe the integration test strategy I implemented for this project in detail.

System Test

The unit and integration tests created and supported by the development team were only one piece of the overall validation strategy. The QA team tested the overall product manually and with Selenium for creating automated tests driven from the html5/javascript View component.

CI

For tests to be effective, of course, they need to actually be run frequently. Ideally developers would all run the unit test suite before checking in, but this is not enforceable. We had a Jenkins CI environment, where we setup automated tests running on a fixed interval whenever code changes were checked in. Failures were reported and logged and emailed to the team for resolution. We also tied in a code coverage tool for reporting progress against our goals.

TDD

For those who are not familiar with test-driven development, the basic idea is that you create your tests before you implement your code using a recipe like:

1. Create your interfaces
2. Create your tests to these interfaces
3. Implement your code
4. Run your tests
5. Rinse/repeat as necessary until all tests are green (pass)

Some of the benefits of TDD are:

1. Enforces up-front accurate requirements and up-front interface design
2. Improves code readability, interface design, architecture, quality (clearly much less likely to make untestable code :))
3. Ensures that tests don't fall behind implementation

As part of this project I adopted a TDD approach, although found it difficult to adopt whole-heartedly. I found "concurrent" test/development a better fit rather than strict adherance to the recipe. It definitely took longer to take a TDD approach than it would have to simply perform the code, but no longer than it would have to develop the code and then the tests later. I advocated for similar approaches by other team members as part of the team training, but we did not enforce it.

Up Next

As already mentioned, my next post will delve into the integration test strategy and methodology that I adopted for the team on this project.

Advocating for Robust Automated Test

The conundrum

In virtually all software engineering organizations I have worked, management is aware of the benefits and importance of a comprehensive automated software test suite and the pitfalls of scrimping on test. Yet, it's actual application varies widely across projects and organizations. Often I find that automated test is adopted to some extent, but with insufficient rigor, thought, and attention. Occasionally I have seen teams that don't do automated test at all. In almost every team I have worked, when there is a crunch to get something done, testing rigor is relaxed, dropped outright, or at least postponed until after the crunch is over.

Clearly there are pain points in the process which is leading to this situation.

Pain Points

• External schedule constraints
• • Deadlines, deadlines, deadlines
  • Customer demo tomorrow
  • Emergency of the day
• 
  

  Schedule constraints are often be the biggest reason for reduced rigor in testing methodology. It is especially difficult when they are immutable deadlines coming from external sources such as: 

  
  

  • Software delivery to a hardware devices or systems whose release schedules are wholly dictated by hardware availability
  • • For example, when working on pre-installed app for a phone, the apps usually either make the delivery date or are pulled
  • Trade shows
• Challenge of quantifying the cost-benefit trade-off
• How do we know that we aren't spending more than we are getting in return?
• Test maintenance burden
• Difficulty of retrofitting tests into a legacy code base 
• Up-front cost
• • Test Framework identification
  • Developer training
  • Developer mindset shift
  • CI integration
  • Difficulty in getting "valuable" test metrics

Addressing the Pain

• External schedule constraints

One strategy to deal with a situation where there is insufficient remaining time to both deliver the software and a full set of developer automated tests is to:

1. Identify all the required tests and add them to the backlog
2. Deliver as many of the most critical tests as possible
3. Supplement the reduced set of automated tests with a more extensive QA test (both manual and automated) for the immediate deliverable
4. Schedule the remaining unfinished tests for delivery in the following sprint, as soon as the “fire drill” is over

In some especially reactive environments it can be challenging to break out of this “fire drill” mode, as they cascade upon each other. This could be a sign of issues that need to be addressed at the management level,including:

1. Unwillingness to turn away new business
2. Unrealistic expectations
3. Lack of understanding of the impact of these decisions
4. Insufficient development resources

It is a responsibility of the senior members of a developer team to point this out to management and help work out a plan to remediate the issue. Depending on team charter and business conditions this could be a very difficult problem to solve and isn’t necessarily a management failure. Developers should take an active role in sharing the responsibility associated with improving the process and steering the process back to sanity.

Even in the most aggressive environments, there will always be “some” down-time, which could be dedicated to catching up on test. This is a good time to advocate for sprints focusing on automated test get-well.

• Quantifying the cost-benefit trade-off

     Perhaps the best way to get upper-management support for test is to demonstrate that the cost in delivery schedule and engineering resource consumption is outweighed by the benefit. This can be challenging to quantify, to say the least. Metrics such as:

1. Severity and frequency of bug reports
2. Savings in support development time
3. Savings in refactoring time
4. Increased revenue due to release of a higher-quality product
5. Less need for refactoring 

are difficult enough to measure when you have the data. But, of course you can’t have internal data until you have a well-tested code-base to compare against, so it’s a bit of a chicken and the egg. One approach would be to present these benefits qualitatively, not quantitatively and reference cost-benefit tradeoffs published by other companies who have made this investment.

There is a severe penalty for “catching a code-base up” so the benefit is greater when applied at the beginning of a project.

• Test maintenance burden

     There is clearly overhead associated with maintaining tests. The amount of this overhead can vary dramatically on factors such as:

1. Proper test scoping
2. Test repeatability
3. Test complexity and supportability

When the tests are properly scoped, repeatable, and straightforward maintenance cost is not so great. Systemic violation of one or more of these factors can easily push the maintenance cost so high that the cost exceeds the benefit. Sometimes a developer or manager will have had past experience working in environments that have embraced automated test, but improperly applied some of these constraints. This can lead to a belief that automated test “is not worth it”. It can be very difficult to challenge a belief system when it is based on experience! It could help, when encountering someone who doesn’t believe in automated developer test to push into the experiences they had, understand where it may have failed, and explain how it might have worked better if applied differently.

• Difficulty of retrofitting into a legacy code base 

The value of adding good test coverage to a legacy code base is less and the cost greater than if it were applied from the beginning of the project. However, in such environments it may still be valuable when applied judiciously and iteratively in small chunks. For example, before refactoring a bit of buggy, complex, and/or obdurate code, it is helpful to provide strong test coverage for the methods in question and use this to validate the refactored code. Similarly, any time that new functionality is added, good test coverage for that new functionality can be easily justified. Finally, there is always “some” down time in a project, which can be used to bolster tests. I like to target areas of the code that are:

1. Most problematic (highest bug reports)
2. Functionally critical
3. Core (used by many components)
4. Most complex
5. Most likely to change

• Up-front cost

While there are initial costs associated with identifying and implementing a test framework, integrating with CI, and training developers, these costs are manageable and largely scale across multiple projects.

Next up

In upcoming posts I will discuss an automated test strategy that I advocated and adopted for a recent project.

The Importance of Design, Architecture, and Clean Code in a Startup Environment

I've worked for several startups, and practically all suffer from the same basic problem: money. It is a race against time to develop enough customers and/or revenue before investor interest, and thus money, disappears. But to get customers, you need code...usually lots of it. So, naturally the focus is on pumping it out as rapidly as possible. The catch-22 is that practically as soon as the company starts to turn the corner and become profitable that code can turn into a liability. Often companies pass the critical early phases, only to fail at scale because of short-sighted decisions in getting to that first corner.

For emphasis, I will give one extreme example I encountered at a very small startup many years ago where I was managing engineering. I inherited a hastily written code base as a starting point (developed by people who were engineers, but not software engineers). The lack of focus and interest in good code and development practices is perhaps best exemplified by the words of one of the co-founders that I will never forget. In a status meeting attended by the development team I  suggested that a developer at least extract a chunk of code that was "cut-and-paste" repeated into a single method. The Co-founder was at this meeting and interjected that this should not be done "if it might slowdown" the developer. This input, and even the notion that extracting code into a method would slow someone down, was a real eye opener. This was the most extreme case I've seen, and largely influenced by the fact that both the developer and the co-founder were hardware engineers, not software engineers, by background,  but the point is that sometimes even the most basic design and supportability tenets are disregarded in the flawed assumption that this is somehow justified in a startup environment.

In this case (as with most startups) the company did not survive long enough to become profitable, but what if it did? Would the weight of the technical debt sink the company? How could such a product be supported? Would there be the time and resources available to completely rewrite the code or would the company simply get bogged down in never-ending feature enhancements on a foundation that was already crumbling under its own weight?

So while some would argue that good practices is a luxury in startup environments, I would argue that the cost of completely disregarding good practices altogether assures failure. Clearly time is of concern, but "well-written" code need not necessarily take significantly more time than a complete hack. Some basic coding concepts can be applied with little or no additional time, including:

• DRY (Don't Repeat Yourself )
• Self-documenting code
• Using the composed method pattern
• Choosing well named methods
• SOLID principles

Beyond basic coding principles, it is equally important to spend time to design and architect the solution and at least consider the evolution of the system. I'm a firm believer that design can be broken down into iterations, similar to other coding activities. Start with the basic architecture required for the first milestone and map out where you expect it will need to evolve from there, expecting that the future iterations will change as requirements change. I'm not advocating for gold-plated designs, but you need to understand some basics of where the code will evolve to avoid coding into corners. I think you should fully understand the architecture requirements of the first iteration, and perhaps the next. For iterations beyond that, at least devoting some time to think about how well the existing architecture will scale can avoid pitfalls today that will be very costly tomorrow. I think this work should always be done in collaboration with other senior members and stakeholders of the team and should be supplemented with some form of "light" documentation. A day or two of due diligence can pay off dramatically over the lifetime of the project.

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.