Innocent growth of technical debt

I’ve spent the last several days unravelling the mystery of the Each feature in Fuse. It’s a powerhouse feature that has managed to accumulate a fair share of technical debt. Though in this case, the debt seems to arise from unpaid taxes rather than short-cuts, sloppiness, or poor choices. Let’s try to stroll through my memory and take a look at what happened.

The Instantiator

To understand what happened with the code, we need to look briefly at the feature itself. Each applies data to a series of templates in our reactive UX language. To give you an idea, here’s a rough example listing different types of cards:

<StackPanel>
    <Each Items="{deck}" MatchKey="type">
        <NumberedCard ux:Template="number"/>
        <FaceCard ux:Template="face"/>
        <JokerCard ux:Template="joker"/>
    </Each>
</StackPanel>

Each wraps together several orthogonal features. Each feature we added increased the complexity of the code. If memory serves me, this is the order of the primary features we added:

• Iterate over a collection of Items and instantiate the templates. The collection can be a fixed array of items, or a reactive array (the array can be modified without reassigning to Items).
• Instantiating different templates based on a MatchKey. There’s also a hard to explain template origin feature, which helps to build components.
• A Count property to duplicate items without a data source.
• A separate Instance class, which instantiates just one item, basically a shortcut to using Count=1. At this point the base class Instantiator was born.
• Windowing of data: only items within the range of Offset and Limit will be shown.
• Deferred creation of items, preventing overload of the UI thread in a single frame.
• Reuse of items, avoiding the need to recreate instances when the window is shifted, or items are added/removed in one frame
• An identity feature to track logically equivalent items even when the object itself is different

Growing complexity

When we introduced the Instance class, we refactored Each to use a shared Instantiator base class. The base class was becoming a flexible feature with multiple uses. By adding just a few properties, we could make this a veritable workhorse.

The inkling of trouble came shortly after when I added the Offset and Limit feature: an essential element of efficient scrolling regions. Instead of instantiating all items in a list, it became possible to limit it to just those which are currently visible.

A variety of features in ScrollView were also required to make this work. It was still a bit wonky at this point, though could be achieved with some JavaScript. I was working on a parallel feature, which influenced the design, but it wouldn’t become available until much later under the name ScrollViewPager.

I added the deferred creation feature, and the confounding nature of the code really hit me. I was having a hard time understanding how all these bits worked together. It wasn’t because we did anything wrong, or took any short-cuts before. I was still uncertain at this time what a good structure would look like. The code definitely smelled bad, but I didn’t see what I could do about it.

I sensed groupings that logically existed and split the code into multiple files, using partial class. Though it makes some tasks more difficult, it aided in understanding the logical architecture.

By the time I added the reuse and identity features, I realized I’m likely the only person that understands what this Instantiator class is doing. Worse, I was having a hard time making sense of it.

To appreciate the problem better, understand that all of those features are dynamic, our user’s aren’t programming static lists. The list of items can, and does, change while we still waiting for the deferred creation of previous nodes. The window can slide back and forth over the data, including back to items that are still waiting to be removed (removal may be animated). All the while individual items may be updated, possibly requiring new templates.

Breaking Point

I wanted to add a simple feature to Instance. It was unable to set the data context on the items it was creating: essentially it was equivalent to an <Each Count="1"> instead of a <Each Items="{ array-of-one }"/>. The latter is far more flexible and would be helpful with our new data model system.

It should have been an easy addition. All the code was there; I would expose an Item property on Instance and rewrite it to an array of one. I did that, it worked, but not completely. I was also using a template matching feature, MatchKey, and if I changed the source data, it wasn’t selecting the new templates.

I wasn’t worried at first, assuming it was a minor defect in the Instantiator. It took me several hours though to locate the problem. It wasn’t an error, but a limitation. I even found the document I wrote explaining that it can’t work. And it was a fundamental issue of the architecture involved: there was simply no way to get it working!

I recall finding this limitation a long time ago. Nobody intentionally wrote it, but there just happened to be a combination of orthogonal features that didn’t entirely work together. One that apparently doesn’t come up in our users’ code, since nobody ever reported, nor complained about it (to my knowledge). But alas, my quick new Instance.Item feature required it to work.

A questionable decision

To recap, up until now I’m not convinced we built this code incorrectly. I had refactored the structure before, cleaned it up, and added many test cases. It was an unfortunate complexity, but nobody actively created it. The accumulated debt came from an entropy tax instead.

Except for now. Rather than face down this impenetrable goliath, I found a short-cut to implement my new feature. All I needed was to extract a bit of code from Each and reimplement it in Instance. Yes, please queue up the U+1F631 emoji. I essentially copied-and-pasted instead of fixing a problem! 😱

I was displeased with myself.

I resolved to fix the problem. But first, as penance, I refactored and removed some 600 lines of code from the expression system. Then I came back to Instantiator.

I walked into the thick cloud of code with only a general idea of how to fix it. Piece by piece, over some 50 commits, I split it up into three classes with intelligible interfaces and distinct responsibilities. The main class is still about 900 lines of code, but roughly half of that is user API documentation — ~400 lines of functional code is within reason, even if still a bit unclear.

The tax

The common causes of technical debt are usually poor practice: lists of things we failed to do, things we didn’t consider, or sloppiness and laziness. While I do think those are common causes, I don’t feel like it applied to this situation.

You can be doing correct coding, with proper foresight and care, and still end up with technical debt over time. Not even a perfect programmer, nor unicorn, could avoid the constant refactoring required to pay off the technical tax.

Follow me on Twitter or watch my stream for more fun with user interfaces and algorithms.