12. Common Legacy App Problems

Written by Lance Gleason

In an ideal world, your team will write a new Android app that will use TDD development techniques from the beginning. In this world, your app will have great test coverage, an architecture that is set up for TDD, and code with a high level of quality. Because of that, the team will be very confident in its ability to fearlessly refactor the code, and they will ship code regularly.

In the real world, many, if not most, apps have technical debt that you will need to work around. In this chapter, you will learn about some of the more common issues you will run into with legacy applications. Then, in subsequent chapters, you will learn how to address these when working on legacy projects.

A brief history of TDD in Android

While there are many technical aspects that affect TDD on a platform, the culture surrounding the development community has a big impact on its adoption.

Android was first released on the G1 in 2008. In those days, the primary development language was Java and, as a result, many early developers came from other Java domains, such as server-side development. At that point, TDD as we know it today had only been around for nine years and was just beginning to see adoption in some software development communities and organizations. During that time, the Rails framework, which was four years old, arguably had the largest percentage of projects being developed using TDD. This was due, in part, because many signatories of the Agile Manifesto were evangelizing it.

Java was 12 years old at that time and pre-dated TDD. It had become a mature technology that large conservative enterprises were using to run mission-critical software. As a result, most Java developers, with the exception of those who were working at cutting edge Agile development shops, were not practicing TDD.

This enterprise Java development culture affected the early days of Android development. Unlike Rails, the initial versions of Android supported testing as more of an afterthought, or not at all. As a result, most new Android developers had not come from an environment where testing was important or even known; it was not a primary concern of the framework and most developers focused on learning how to write apps, not on testing. During those early days, many of the concepts we take for granted today were just beginning to be developed. Over time the platform evolved, devices became more powerful, and apps became more complex.

Eventually, tools like Robolectric and Espresso were introduced and refined. TDD became more of an accepted practice among Android developers. But even today, it is not uncommon to be at an Android developer meetup where fewer than half of the developers in the audience are actively writing tests or practicing TDD on a daily basis.

Lean/XP technical practice co-dependency

TDD is one of the key development practices of Lean/XP.

Before Lean/XP became popular, most development organizations used a Waterfall development approach with steps that often provided marginal value for projects.

Some traits of a Waterfall project include:

1. All known requirements for the entire project (i.e., a project that will take months of development effort) are gathered and written in a requirements document before coding begins.
2. Certain documents, such as software architecture design documents, are often created after the requirements have been created, but before development begins.
3. Test plans need to be created before testing begins.

While all of this sounds logical in theory, in reality:

1. At some point, usually mid-project, new requirements are discovered that require a change.
2. If the project is really strict with its process (usually the exception), it must modify all artifacts upstream and downstream. This leads to a project timeline that is extended.
3. If the business cannot afford to have the timeline slip because of the process (again, what usually happens), phases are skipped, the documents end up not reflecting the reality of the project, and the project gradually becomes chaotic.

Lean/XP fun historical facts

In software, the techniques we use were built on the shoulders of the giants, often from other industries. TDD/XP has roots in manufacturing through a subset of Six Sigma techniques, which are called Lean. To learn more about Lean, XP and its relationships, this Wikipedia article on Lean software development is a great place to start: https://en.wikipedia.org/wiki/Lean_software_development.

Lean, as the word implies, eliminated many of the unnecessary aspects of the development process and only kept the things that were deemed necessary. The practices that are kept are often highly interdependent. Because of this, if one practice is not followed, another one becomes more difficult or may not be done as well. For example, TDD is a critical component for doing continuous deployment. When an app is developed with TDD principles at the unit and integration layer, the practice guides the project towards better architecture.

Let’s explore some of the co-dependent legacy code issues.

No unit or integration tests

This is the biggest issue you will likely run into when working on a legacy project. It happens for a variety of reasons. The project may be several years old and the development team may have chosen not to write unit tests. For example, it is not uncommon to find teams with technically strong Android developers who do not know how to write tests. Of course, if you are on a team that does not know how to practice TDD, this book would make a great gift, especially for holidays, birthdays or “just because.” :]

Difficult to test architecture

One of the reasons why MVVM, MVP and MVI are popular is because they tend to drive a structure that is easier to test. But, if the app was initially developed without any unit tests, it is likely there is no coherent architecture. While you may get lucky with a design that is testable, it is more common to find an untested app with an architecture that is, in fact, difficult to test.

Components that are highly coupled

When an app’s components are highly coupled, they are highly interdependent.

For example, let’s say that you want to be able to search for any pets that share traits with a specific pet — in this case you will use the name of the pet. One implementation might look like this:

class Cat(
  val queenName: String, 
  val felineFood: String, 
  val scratchesFurniture: Boolean, 
  val isLitterTrained: Boolean)

class Dog(
  val bestFriendName: String, 
  val food: String, 
  val isHouseTrained: Boolean, 
  val barks: Boolean)

fun findPetsWithSameName(petToFind: Any): List<Any> {
  lateinit var petName: String  
  if (petToFind is Cat){
    petName = petToFind.queenName
  } else if (petToFind is Dog) {
    petName = petToFind.food  
  }
  return yourDatabaseOrWebservice.findByName(petName)  
}

The functionality that should be truly unique to the findPetsWithSameName method is the call to the yourDatabaseOrWebservice.findByName() call. But, this method also has code to get data from these objects before doing a find on them. To test this, you could write a test that only creates an instance of a Cat and passes it into the method. For example, let’s say that you have a cat named Garfield and your test data has two other animals with the same name. Your test would look something like this:

@Test
fun `find pets by cats name`() {
  val catNamedGarfield = Cat("Garfield", "Lasagne", false, false)  
  assertEquals(2, findPetsWithSameName(catNamedGarfield).size)    
}

This test will pass. The problem is that there is a bug in your implementation, because the code for the dog is retrieving the search name from the wrong field. To get full code coverage, and find this issue, you need to write an additional test that also passes in a Dog. There is also a boundary condition that has not been addressed if someone passes in different class, like a Lion:

@Test
fun `find pets by dogs name`() {
  val dogNamedStay = Dog("Stay", "Blue Buffalo", false, false)  
  assertEquals(5, findPetsWithSameName(dogNamedStay).size)    
}

@Test
fun `find pets by lions name`() {
  val lionNamedButterCup = Lion("Buttercup", "Steak", false, false)  
  assertEquals(2, findPetsWithSameName(lionNamedButterCup).size)    
}

In total, you needed three unit tests for this method. A less coupled implementation might look like this:

open class Pet(var name: String, var food: String)

class Cat(
  name: String, 
  food: String, 
  var scratchesFurniture: Boolean, 
  var isLitterTrained: Boolean): Pet(name, food)

class Dog(
  name: String, 
  food: String, 
  var isHouseTrained: Boolean, 
  var barks: Boolean): Pet(name, food)

fun findPetsWithSameName(petToFind: Pet): List<Pet> {
  return yourDatabaseOrWebservice.findByName(petToFind.name)  
}

With the new version of this code, you only need to write one test to test the functionality of findPetsWithSameName(petToFind: Pet):

@Test
fun `find pets by cats name`() {
  val catNamedGarfield = Cat("Garfield", "Lazagne", false, false)  
  assertEquals(2, findPetsWithSameName(catNamedGarfield).size)    
}

You could refine this test further to mock a pet, taking away any dependency on implementations of Cat. Highly coupled code can also lead to situations where one component changes or you do a small refactoring in one area that leads to large changes throughout the app (or even the tests). While an app’s architecture doesn’t guarantee that this won’t happen, the less consistent the architecture, the more likely you are to see this.

Components with low cohesion

One important tenant for Object-Oriented Design is to have components that focus on doing one thing well. This is also referred to as cohesion. For example, let’s say you have a class called Car:

class Car {
  val starter = Starter()
  val ignition = Ignition()
  val throttle = Throttle()
  val engineTemperature = Temperature()
  var engineRPM = 0
  var oilPressure = 0
  var steeringAngle = 0L
  var leftDoorStatus = "closed"
  var rightDoorStatus = "closed"

  fun startCar() {
    ignition.position = "on"
    starter.crank()
    engineRPM = 1000
    oilPressure = 10
    engineTemperature = 60
  }

  fun startDriving() {
    if(leftDoorStatus.equals("closed") &&
        rightDoorStatus.equals("closed")) {
      steeringAngle = 0L  
      setThrottle(5)    
    }
  }

  private fun setThrottle(newPosition: Int) {
    if (ignition.position.equals("on") && engineRPM > 0 &&
        oilPressure > 0) {
      throttle.position = newPosition
      engineRPM = newPosition * 1000
      oilPressure = newPosition * 10
    }
  }

}

For this implementation, you have two publicly callable methods, startCar() and startDriving():

• startCar() method: Turns on the ignition, cranks the starter and updates the status of the engine RPM, oil pressure and engine temperature.
• startDriving() method: Checks that the doors are closed, sets the steering wheel to an angle of 0 and calls a private method called setThrottle() that begins to move the car.

setThrottle() has to check that the ignition position is on and that the engineRPM and oilPressure are above 0. If they are, it sets the throttle position to the value passed in, and sets the engine RPM and oil pressure to a multiplier of the throttle position.

With a car, you could have multiple engine choices. For example, your current car runs on fuel, but what if you wanted to switch the current car out for an electric one? Things such as the engineRPM and oilPressure would not be needed — these are really details of the engine. As a result of this, your class currently has low cohesion.

Since this is an incomplete car, before it’ll be usable, you will need to add things such as brakes and tires, which will make Car a very big (and complex) class.

Now, take a look at the same example with high cohesion:

class Engine {
  val starter = Starter()
  val ignition = Ignition()
  val throttle = Throttle()
  val engineTemperature = Temperature()
  var engineRPM = 0
  var oilPressure = 0    

  fun startEngine() {
    ignition.position = "on"
    starter.crank()
    engineRPM = 1000
    oilPressure = 10
  }

  fun isEngineRunning(): Boolean {
    return ignition.position.equals("on") && engineRPM > 0 &&
        oilPressure > 0
  }

  fun setThrottle(newPosition: Int) {
    if (isEngineRunning()) {
      throttle.position = newPosition
    }
  }  
}

class Car {
  val engine = Engine()  
  var steeringAngle = 0L
  var leftDoorStatus = "closed"
  var rightDoorStatus = "closed"

  fun startCar() {
  	engine.startEngine()    
  }

  fun startDriving() {
    if (leftDoorStatus.equals("closed") &&
        rightDoorStatus.equals("closed")) {
      steeringAngle = 0L  
      engine.setThrottle(5)    
    }
  }
}

Here, you have the same functionality, but classes have more of a single purpose.

If you’ve been in enough legacy code-bases, you will run across components like the first example in which you have large classes that are doing a lot of different things. The more lines of code that a class has, the more likely it is to have low cohesion.

Reliance on Internal Constructors

Imagine that you were writing a unit test for the Car class above and wanted to test that class in isolation. The current implementation is written in a way where it would be very difficult to pass in a mock or spy for an Engine. The good news is that there is an easy fix for this by refactoring Car with an optional constructor parameter:

class Car(val engine = Engine()) {
  var steeringAngle = 0L
  var leftDoorStatus = "closed"
  var rightDoorStatus = "closed"

  fun startCar() {
  	engine.startEngine()    
  }

  fun startDriving() {
    if (leftDoorStatus.equals("closed") &&
        rightDoorStatus.equals("closed")) {
      steeringAngle = 0L  
      engine.setThrottle(5)    
    }
  }
}

Use of Singletons

Imagine that you used a singleton to create your Engine class with the following implementation.

class Engine private constructor() {

  private object HOLDER {
    val INSTANCE = Engine()
  }
  
  companion object {
    val instance: Engine by lazy { HOLDER.INSTANCE }
  }

  val starter = Starter()
  val ignition = Ignition()
  val throttle = Throttle()
  val engineTemperature = Temperature()
  var engineRPM = 0
  var oilPressure = 0    

  fun startEngine() {
    ignition.position = "on"
    starter.crank()
    engineRPM = 1000
    oilPressure = 10
  }

  fun isEngineRunning(): Boolean {
    return ignition.position.equals("on") && engineRPM > 0 &&
        oilPressure > 0
  }

  fun setThrottle(newPosition: Int) {
    if (isEngineRunning()) {
      throttle.position = newPosition
    }
  }  
}

class Car {
  val engine = Engine.instance 
  var steeringAngle = 0L
  var leftDoorStatus = "closed"
  var rightDoorStatus = "closed"

  fun startCar() {
  	engine.startEngine()    
  }

  fun startDriving() {
    if (leftDoorStatus.equals("closed") &&
        rightDoorStatus.equals("closed")) {
      steeringAngle = 0L  
      engine.setThrottle(5)    
    }
  }
}

Since Engine is only created once this can result in flaky unit tests. The solution is the same as the one for internal constructors.

class Car(val engine = Engine.instance) {
  var steeringAngle = 0L
  var leftDoorStatus = "closed"
  var rightDoorStatus = "closed"

  fun startCar() {
  	engine.startEngine()    
  }

  fun startDriving() {
    if (leftDoorStatus.equals("closed") &&
        rightDoorStatus.equals("closed")) {
      steeringAngle = 0L  
      engine.setThrottle(5)    
    }
  }
}

Now when you are testing this class you can pass in a mock or spy for your Engine.

Other legacy issues

A large codebase with many moving parts

Many legacy systems, over time, become large apps that do a lot of things. They may have hundreds of different classes, several dependencies, and may have had different developers — with different development philosophies — working on the project. Unless you were the original developer on the project, there will be sections of the app code that you don’t fully understand. If you are new to the project, you may be trying to figure out how everything works.

One common anti-pattern is have a developer that is new to a project get familiar with it by adding tests to an untested component of the application. That is a bad idea because, in order to write a meaningful test, you need to understand what the expected behavior of the component is that you are testing. A better approach is to add tests to features, modifications and bug fixes along with related components as you are working on the system.

Your sample projects for this book will not be large, but you will be using the same approach that you will want to use for large projects — namely, focusing on one section of the app and working through the others over time.

Complex/large dependent data

In some domains, you may have an app that creates and consumes a large amount of data. Those projects may have a large number of models with several unique data fields. Taming this beast as you test can easily look like a insurmountable task, so stay tuned for tricks on how to address this.

Libraries/components that are difficult to test

A lot of libraries and components have very important functionality that are easy to use, but did not consider automated unit tests as part of the design. One example is using Google Maps in a project with custom map markers. If you had to create this functionality, you would have to write a lot of code. But, integration tests can be very challenging. In some instances, the best solution may be not to test these components because the value added by the tests are lower than the effort to create tests.

Google Location Services Components are another example of this. Stay tuned for an example where we look at ways to work around these kinds of libraries.

Old libraries

This happens a lot: A developer needs to add functionality to an app. Instead of reinventing the wheel, they use a library from the Internet. Time passes and the library version included with the app is not updated. If you are lucky, the library is still being supported and there are no breaking changes when you update it.

The library has a new version with breaking changes

If a library version in a project has not been updated in a few years, and it is being actively maintained, there is a good chance that a new version with breaking changes has been introduced.

Issues that may introduce include:

1. Features you currently use may have been removed.
2. You may have a significant number of touch-points in the app that require a significant amount of refactoring.
3. Core functionality may have changed.

Even if you used TDD with the initial version of the library, there is not much you can practically do to prevent this, outside of timing when you do your upgrade.

The library is no longer being maintained

This happens for a variety of reasons, including:

1. It was an open-source side project for a developer who ended up getting busy with other endeavors.
2. A new library or approach has been developed, which leads to projects migrating from the old library.
3. If a company created the library, they may have gone out of business or stopped supporting a product.

If the library is open source, you could decide to take over maintenance of it. Alternatively you will need to migrate to a new library. If your app already has a lot of unit tests, this will break them as you add support for the new library.

Wrangling your project

The app is working well with no tests

After seeing all of the issues you can run into with a legacy project, you may be asking if you should start to add tests to the project. If you plan on continuing to maintain a project for a while, the short answer is yes, but with a few caveats.

1. If your project has more than one developer, TDD will not add as much value to the project unless the entire development team is dedicated to practicing it.
2. Unless your project is small, the first passes at TDD will take a non-trivial amount of effort to set up.
3. Rome wasn’t built in a day; neither will your test suite.
4. You probably will not have the luxury of stopping new feature development for several months to add test coverage to your entire project.

You consider rewriting the entire app

This can be a very tempting option when working with a legacy app, especially if you were not the original author. If your app is small or truly a mess, that may be the best solution. But, if your project is large, and you are planning on keeping most of these features, however tempting a rewrite may be, it could be a job-killing move.

Most legacy apps have a significant number of undocumented features and edge cases. For these projects, a rewrite will often take several months. In addition to that, the business will likely want to maintain the existing app and add new features to it. While a rewrite may still be the best solution, before heading down that path, a better option would be to break the app up into components and refactor things a component at a time. This is called the Strangler pattern.

Lore has it that the Strangler pattern got its name from a vine called the strangler vine. These vines seed themselves in fig trees. Over time, they grow into their own plants surrounding and killing the tree. Likewise, you components will surround the initial implementation, eventually killing off the original one.

Key points

• Lean and Extreme Programming have a lot of interdependencies.
• Many legacy applications have little or no automated tests.
• Lack of automated tests can make it difficult to get started with TDD and may require getting started with end-to-end Espresso tests.
• Rewriting a legacy application should generally be considered as a last resort option.

Where to go from here?

Beyond the techniques you will be learning in this book, the book Working Effectively With Legacy Code by Michael Feathers does a great job of talking about legacy code problems. You can check out this book at https://www.oreilly.com/library/view/working-effectively-with/0131177052/.