Naming workarounds in code

Some weeks ago, I gave an advice in a code review to better mark a temporary workaround with a very long and descriptive function name. As I really like that approach, I want to share it with the world in this post.

Why good naming is important

First I want to talk briefly about why I think taking the time to find good names in code is so important.

"Coding" to me usually means more reading or thinking than it means writing. I never measured this, but my feeling is that the ratio is even sometimes up to 95% reading (or thinking) and only 5% actively writing.

This is especially true for bugs: I probably can't count how many times I tried to find the cause for a certain behavior or bug for hours – only then to fix it within a few seconds.

But even in less extreme situations, I usually read way more than I write. And I am going out on a limb here and claim this is true for most coders.

Following this logic we can say: the better readable our code is, the more efficient we will be writing it. And probably also have more fun in doing so. :)

But what is readable code?

Well, the bad news here is: It depends. Readability is really a subjective thing and you'll often find that what one person considers readable completely throws off another.

Yet, I believe that there is a certain base level of practices with which you can achieve a overall better readability for a majority of readers. And that includes good and expressive variable and function names.

Just consider this basic example with non-expressive names:

const convert = (value) => {
   const y = getExchangeRate('Dollar', 'Euro');
   return y * value;
}

Just looking at the function name itself, one might understand that it is converting some value. But to know what it is converting, you have to read on.

Seeing the call to getExchangeRate and the multiplication of the value in the end, you can assume that the function converts money from one currency to another - the currencies being 'Dollar' and 'Euro'.

But in which direction is it converting? Euro to Dollar or Dollar to Euro? Given the order of the arguments passed to getExchangeRate, you might assume it's Dollar to Euros. But if you wanted to be sure about it you'd also have to look inside getExchangeRate. Depdening on it's complexity, that might be a lot of reading.

Now let's make this a bit clearer:

const convertDollarToEuro = (dollar) => {
  const exchangeRate = getExchangeRate('Dollar', 'Euro')
  return dollar * exchangeRate;
}

Like this, all of the assumptions and questions we had before don't even come up. It is already clear that the function is converting dollars to euro just by it's name. Also within the function, the variable names are clearer in what they actually stand for.

Of course this is a basic example - but if you stick to this kind of expressive names throughout your code, you will be able to read and navigate through it way faster.

The case at hand

Now as wirtten in the introduction, I was doing a code review. The code was about mapping some data from an external API into our own object structure.

Note: I simplified the example to focus on the method rather than the project itself So now it is about kittens.

The code I reviewed looked something like this:

const kittenAPIData = await requestKittenFromApi();
const kitten = {
   name: kittenAPIData.name,
   owner: kittenAPIData.owner.fullName,
   furrColor: kittenAPIData.colorOfFurr || '',
   homeTown: kittenAPIData.location.city
   // ... and many more mappings 
}

You might already have stumbled upon Line 5: Why is there a or condition falling back to an empty string for the property furrColor?

My first assumption was that this it is setting a default value for an optional field. But why only for this line and not the others?

As assumptions are evil, I went ahead and asked the developer who wrote it.

As it turned out, this was only a workaround due to a bug in the API: Instead of returning the value for furrColor, it was always returning null. And my assumption of it being optional was wrong as the rest of the code relied on it being set.

The API Developers already knew about this bug and said they were going to fix this soon. So in this case, the workaround was a nice way to let them fix it whenver they want without having to synchronize our deployments. As soon as the API returned the correct values, our code would the right thing automatically.

As much as I like developer communication, it would have been nice to avoid the confusion and extra effort of me asking by being able to see this is a workaround directly in the code.

How to mark a workaround in code

One thing that might come to mind are comments:

const kittenAPIData = await requestKittenFromApi();
const kitten = {
   name: kittenAPIData.name,
   owner: kittenAPIData.owner.fullName,
  // Defaulting to empty string is a workaround due to a bug in the API returning null
   furrColor: kittenAPIData.colorOfFurr || '',
   homeTown: kittenAPIData.location.city
   // ... and many more mappings 
}

This is already better. But well - comments tend to get overlooked. At least I usually read logic first, comments (maybe) later. Also, as this was a bug that would get fixed soon, I wanted the next person stumbling upon this to definitly check and maybe delete the then unecessary workaround.

So why not use an expressive function name to mark it for what it is?

const kittenAPIData = await requestKittenFromApi();
const kitten = {
   name: kittenAPIData.name,
   owner: kittenAPIData.owner.fullName,
   furrColor: defaultToEmptyStringAsTemporaryWorkaroundToBugInAPIReturningNull(kittenAPIData.colorOfFurr),
   homeTown: kittenAPIData.location.city
   // ... and many more mappings 
}

function defaultToEmptyStringAsTemporaryWorkaroundToBugInAPIReturningNull(colorOfFurr) {
  return colorOfFurr || ''
}

Yes, you are seeing this correctly: A 63 character long function name explaining exactly what is going on. Did you get alerted by this? Well good - that was the intention. :)

In my opinion, this approach has several advantages:

1. It most definitly won't get overlooked by anyone
2. If I'd stumble upon this, I'd definitly check if the bug still persists and delete the workaround if not
3. It tells the reader not only what is going, but also why it is there in the first place

Now this approach shouldn't be used too often as it would then defy the purpose of alerting the reader. But I think it's a neat trick sometimes to really strike attention and let others or myself know that the written code here is not supposed to stay in forever.

And as oppsed to just a comment that may even contain a todo, this function name is really painful and gives a lot of motivation to refactor as soon as possible.