Covariance is not Cohesiveness

One Reason Why Naming Things is Hard

This is another lesson learned from my adventures in refactoring a large JS project. An easy target for any refactor is to pull out duplicated code blocks and turn them into reusable functions. Keep it DRY. Done. Almost…

After reading and discussing Sandi Metz’s article on the wrong abstraction being more expensive than duplication, I started to look at my habit of deduplicating code. I was diligent about pulling out only the code that was called every time, or code that all relied on the same variables; code that co-varied. That way, my functional equivalent could accept the variable, and output the result of all that code that relied on that variable. In my refactoring work, I tried to limit the number of parameters that each new function accepted, since that is a red flag that the function is attempting to account for too many scenarios (and may be the wrong abstraction) but I ran into another issue: naming my shiny new functions.

I had a function that took a dozen lines of code that sorted through a list and updated the DOM based on that list. This block of code was used all over the place and seemed to be prime fodder for a functional deduplication effort. But what do I call it? It took me a day of struggling with naming this function to see that I was staring at a couple of red flags for bad code, namely violating the Single Responsibility Principle.

Red Flags for Violating the Single Responsibility Principle

  • Using the word “and” when describing your function.
  • Having difficulty naming the function, or using a vague name that doesn’t convey to the consumer what the function does.

The adage “code is the documentation” comes to life if you write code that reads like documentation. Naming things helps that. I bet you can guess what the code below does (if not how):

switch (getABTestVariant()) {
case 'a':
// do nothing, control group
break;
case 'b':
removeDollarSigns(document.querySelectorAll('.price'));
break;
case 'c':
removeZeroCents(document.querySelectorAll('.price'));
break;
default:
throw INVALID_VARIANT_CHOSEN;
}

In this code, you don’t know how getABTestVariant, removeDollarSigns or removeZeroCents each work, but you can tell from their names what they should do. It was easy to name these functions because they each do one thing. In my codebase, this pattern of getting the variant and modifying the DOM based on that variant occurs over and over. It would be tempting to abstract this pattern out into a function that intakes an array of functions and a selector for a DOM element, but what would we name it? After all, it would retrieve a variant name, scour the DOM for an element, apply functions conditionally to that element, and possibly throw an error.

Let’s try modifyElement. The consumer would then write modifyElement('.price', [removeDollarSigns, removeZeroCents]); which appears to apply all of those modifications to the elements. Hmm… how about runVariant('.price', [removeDollarSigns, removeZeroCents]); Well, which variant are we running? The problem here is that even though this code covaries, it isn’t cohesive. This is clear when we try to consume the function that runs this code. We’re forced to pass parameters that seem unrelated. The signature makes no sense. Or rather, it makes different sense than what was intended.

Conclusion

It is inevitable that some duplication will exist in your code, so beware of trying to DRY your code to the point that you are writing disjointed functions merely because the code within covaries. When writing functions pay attention to how difficult it is to describe and name the function, how many parameters it requires, and what the signature looks like. These little litmus tests will help you know if your function has a Single Responsibility.

One clap, two clap, three clap, forty?

By clapping more or less, you can signal to us which stories really stand out.