Jakub Holý: don’t add unnecessary checks to your code

(written by lawrence krubner, however indented passages are often quotes). You can contact lawrence at: lawrence@krubner.com

This is a very good point:

Imagine you need to call .indexOf on product_code. You might be tempted to add

if (!_.isString(product_code)) return false;

Now you are safe and your code can’t fail. But when I read this code, I immediately think:

Oh, I believed product_code to be always a string but obviously it is not always the case. It must have happened in the past or the author wouldn’t have added this check. I wonder what are the possible non-string values. And what does it mean and how should I handle them?

So by adding this check you have eroded significantly my trust in the data or rather trust in my mental model of the data (domains, possible values) and it has become much more fuzzy. Now I have to always assume that product_code can be something (what?!) else than a string, check for it everywhere and wonder what to do about it.

At the start of a project, when my code tends to be evolving fast, and especially if I’m dealing with an external API that keeps surprising me, I end up in situations where I want to write these guard clauses, simply to see if I really understand the data that I’m dealing with. Rather than put the clauses in the body of the function, I write them as pre-conditions and post-conditions. This is the cheapest and easiest place for me to specify a contract for the function. This can still be messy if I write a bunch of pre-conditions that are not needed — the problem is the same as the one Jakub Holý identifies. I know it is not the best, but I assume these things can be cleaned up over time.