Ryan Silva Jan 31, 2017
Several years ago, I got the call every engineer dreads: there's a big problem in production. The issue manifested itself dramatically but was caused subtly. I had made significant performance improvements by altering a
SQL SELECT * query to exclude a column containing a large blob of text. That text wasn't used in that page... or so we thought.
During the refactor, I identified that text was used in one check, but after conferring with other engineers we decided it was OK to remove the field in this query anyway. It turns out that check was important. At some point years prior, the team decided that the presence of a certain substring in the large text blob would affect the value of a very important boolean field in a non-obvious way. Excluding that text from the SQL query, in certain cases, affected this important boolean value.
More recently, I noticed a different issue. Our API applied a default setting based on create date, because on a certain date the default settings changed. Seven months later, a change to the way we used a 3rd party library affected how we set create dates, which then affected the settings on very specific older data.
These decisions—changing a boolean based on a keyword in a large blob of otherwise unnecessary text, applying a setting based on create date—are land mines. They fix an issue at hand and lie silent for months or years only to explode when an unsuspecting developer makes a routine change.
Nobody likes land mines, but how do you prevent them?
This may sound obvious, but it's so easy to take a shortcut and move on. In fact, that's probably the token signature of a land mine: a questionable decision made in the interest of time that solves the problem... for now. But please, think about future you, future me.
When writing business logic, think about the inputs to your function. Would another developer reasonably expect change of input data to affect the output of your function? Try to imagine scenarios where your input data might change. Would this affect the output in unexpected ways? I don't think most people would expect settings to change based on the create date.
In both of the instances above, a "dirty hack" was introduced to avoid running some kind of migration on the existing data. Migration is scary, I get it. But the outcome of avoiding a migration can be scarier. Migration is a tool in your toolbox that you should not be afraid to use. When weighing options, remember to weigh the obvious risk from migration against the not-so-obvious risk of introducing code complexity and possible land mines for future developers.
I hope these tips will help keep your codebase clear of land mines for all of us.