Improving Essential Workflows in a Legacy Code Base

Improving Essential Workflows in a Legacy Code Base

alt

Procore provides a suite of project management tools for teams collaborating to build large-scale construction projects. The tools allow our customers to share access to critical items like documents, scheduling systems, and data. We offer a native iOS app for our customers to access their documents from iPhones and iPads. One of the most important documents they need access to are Drawings (also called blueprints or plans). Over the last few weeks we’ve been focused on enhancing the performance of our Drawings tool to provide a great experience to our customers navigating through thousands of Drawings.

When we first built the Drawings tool, our customers often only needed to navigate through a few hundred drawings. Today, builders use Procore on some of the biggest jobs in the country and these jobs involve tens of thousands of drawings. Our customers told us - and showed us - that the Drawings tool wasn’t meeting their expectations. It just wasn’t a smooth experience on even the most powerful iPads available. So, we decided to fix that.

Drawings as We Know It

Let’s first take a step back to examine the state of Drawings before we began this project.

Like the rest of the Procore app, the Drawings tool uses the Model-View-Controller architecture pattern. In the beginning, when the Drawings tool was simple, this worked fine for us. But as most iOS developers know, as soon as things begin to get even a little complex, MVC turns into Massive-View-Controllers real quickly. Our Drawings tool was no exception: the three main view controllers in Drawings do everything from networking, UI, local storage, business logic, and more. What’s worse, these three view controllers were tightly coupled. They knew the ins and outs of the implementations of each other and relied heavily on those details. Whatever changes were made to one had deep implications on another.

Secondly, there were no tests around Drawings. This made working in the Drawings code base extremely hard and extremely risky. Making changes here was always an "edit and pray" task. No matter the size of the task, big or small, simple or complex, we often shipped regressions in Drawings because of these unknown side effects.

Given the current state of the Drawings tool and the fact that we were going to be making a number of performance related changes to the code, we knew we had two options. We could:

  1. Rewrite Drawings from scratch (it was all Objective-C).
  2. Refactor the current code

Although our knee jerk reaction was to rewrite (a natural tendency in all engineers), we decided not to. Why?

At Procore, we currently ship an update to our iOS app every week. The main driver behind this decision is to deliver value to our customers as fast as possible. Had we chosen to rewrite the entire Drawings code base, we surely would have missed several releases and withheld value for much, much longer. Secondly, when rewriting a feature from scratch where there are no tests, it’s almost guaranteed to introduce regressions and unknown side effects. We’d essentially be replacing old bugs with new bugs. Refactoring the existing code would help us to ship it faster while maintaining a much higher level of confidence in quality.

The Big (and Safe) Refactor

Once we decided on how we were going to tackle this challenge, we needed to figure out what we were going to do. Being familiar with the code, we all had our theories on how to improve the performance around navigating Drawings. With these theories in mind, we first profiled the app to either validate or disprove our theory. This is a basic first step but it's a critical one. Xcode provides some great tooling and can save a significant amount of development time.

In our case, most of our theories turned out to be valid, however there were a few findings that were not obvious. The technical pieces here are mostly uninteresting since it mainly consisted of leveraging built in iOS components. Our workflow here followed this pattern:

  • Profile the app in Time Profiler
  • Find the hotspots in our code where most time was spent
  • Address the top offender
  • Profile the app again and confirm that we fixed it
  • Repeat

After our first iteration of profiling, we decided to rework the data source driving our revision list. Again, we’ll spare the technical details around this change as they are not the interesting pieces of the refactor. However, “simply” swapping out our current data source (managed entirely by the owning ViewController) with a new one was not trivial. It had wide sweeping effects on the entire Drawings tool. This is the interesting piece that we felt was worth sharing. How do we safely rework this legacy, complex and untested code?

This is where we considered one of Martin Fowler’s techniques on working with legacy code: branching by abstraction. Branching by abstraction is a technique that allows large scale changes to be made iteratively and it also gives us the opportunity to improve our test coverage. Convinced by the promises of this technique, we dug right in.

The premise behind this technique is to write a new abstraction layer around the changed functionality and move all call sites onto this new layer. Then, simply call into the old layer (unchanged) from the new abstraction.

alt

This allows us to:

  • Make a small change to create this abstraction layer (this essentially just moves code around)
  • Write tests around our abstraction layer
  • Then, we can swap out the implementation of the abstraction layer with our new (faster) implementation and have the confidence of our tests that we didn’t introduce any side effects.

We decided to implement this abstraction layer with several ViewModels (MVVM). We introduced the view models incrementally; so initially only the code and logic affected by the performance improvements were moved over. For us, that included the following:

  1. Displaying collections of drawing revisions
  2. Keeping track of drawing revision ordering
  3. Displaying a single drawing revision
  4. Swiping up/right/down/left between drawing revisions
  5. Keeping track of viewing history
  6. Hyperlinking between drawing revision callouts

We moved all of the code from each view controller related to the above events into their corresponding ViewModel (unchanged), wrote tests around each ViewModel and then wired up the ViewModel to the view. This change, seemingly big, was in fact pretty small and was mergeable in and of itself. This allowed us to keep continuously integrated without having a giant, long running branch. Put in other terms, this first step separated the refactor from the actual performance change.
alt
The next step was actually making the performance improvement by swapping out the implementation of the ViewModel with more performant code. Now, here’s where we really felt the power of branching by abstraction: we rarely had to build and run our app to validate our changes because our tests told us immediately whether we broke any existing functionality or requirement! There were several times where we made a seemingly trivial and harmless change to improve performance and our tests went red. The short feedback loop and workflow were amazing and allowed us to build faster and with more confidence that we weren’t breaking other pieces of the navigation logic.
alt
Another important note about writing tests around existing business logic (the first step to this process) is that it really forces you to understand the current behavior. There were several nuances to the existing behavior that we did not previously know. Had we re-written the tool, surely those behaviors would have broken.

Conclusion and Results

In summary, we learned a lot in this experience and hope to carry it out in other places in the app, especially when working with legacy code.

By the end of this change, we had:

  • Increased test coverage in Drawings significantly
  • Reduced the size of some of our massive view controllers (and laid the foundation for further reducing later on)
  • Confidence that our change did not introduce unknown side effects
  • Improved the performance in Drawings (the actual task)

We hope you enjoyed the article and find our learnings helpful! If you're interested in this subject and have questions or comments, we’d love to hear from you on Twitter. Just mention @ProcoreEng. If you'd like to work on tough engineering problems like this, we'd love to talk to you about careers at Procore. We're currently hiring for over a dozen different roles in our engineering department, so check out our job listings and reach out to us!