Introduction to DRY

Take a look at this code for obtaining a list of packages that the current user of a logistics company has to deliver:

const myDeliverablePackages = packages
	// packages have been inspected and are safe to deliver
	.filter(package => package.inspected && package.safe)
	// packages are ready for delivery
	.filter(package => package.status === Statuses.READY_FOR_DELIVERY)
	// packages are assigned to the current logged in user
	.filter(package => package.assignedOperator === currentUser.id)

As it turns out, we may need to get this specific list of packages in several places throughout our codebase (maybe once to display this list in the clientside, twice to send it as the payload of a confirmation email, thrice if we also want to take the same list and use it as a payload to some requests that change the status of these packages, etc).

If we just copy and paste the same chunk of code in all those places, then eventually things start to get really repetitive:

// ...  
// ... We add it to the React component that displays this list

const myDeliverablePackages = packages
	// packages have been inspected and are safe to deliver
	.filter(package => package.inspected && package.safe)
	// packages are ready for delivery
	.filter(package => package.status === Statuses.READY_FOR_DELIVERY)
	// packages are assigned to the current logged in user
	.filter(package => package.assignedOperator === currentUser.id)

// ...  
// ... and for the code that prepares the payload for a confirmation email
const myDeliverablePackages = packages
	.filter(package => package.inspected && package.safe)
	.filter(package => package.status === Statuses.READY_FOR_DELIVERY)
	.filter(package => package.assignedOperator === currentUser.id)

// ...  
// ... and for the code that prepares the payload to change the state
const myDeliverablePackages = packages
	.filter(package => package.inspected && package.safe)
	.filter(package => package.status === Statuses.READY_FOR_DELIVERY)
	.filter(package => package.assignedOperator === currentUser.id)

   
// ...
// ... and in several other places
const myDeliverablePackages = packages
	.filter(package => package.inspected && package.safe)
	.filter(package => package.status === Statuses.READY_FOR_DELIVERY)
	.filter(package => package.assignedOperator === currentUser.id)

// ...

const myDeliverablePackages = packages
	.filter(package => package.inspected && package.safe)
	.filter(package => package.status === Statuses.READY_FOR_DELIVERY)
	.filter(package => package.assignedOperator === currentUser.id)


// ...
// ... you get the idea
// ...

If you don't know better (or don't really care?) you might just roll with it and keep on adding this same piece of code in any other new places where this same logic is needed.

However! There comes a time on every coder's career where they open their eyes to an interesting realization: Writing the same code ten times in ten different places feels kind of dumb.

Not only because sometimes the code for the functionality you're including is quite lengthy so lengthy x 10 = a lot of repeated code; but also because whenever you want to improve or enhance what that code does you need to make sure you do it in all ten places!

If you don't keep all ten code blocks updated in the same way and at the same time, you've now created ten different pieces of code that basically do the same thing in slightly different ways and thus made it harder for yourself or your peers to maintain it.

And that may be alright if you're a vibe coder building a 10MM SaaS every day while sitting at the pool leaving it all to c*rsor or w*ndsurf! but for people that are *actually* paying attention to the code that they write this can get incredibly tedious, and bugs can very easily be introduced.

This is where a simple concept may either come instinctively, or be passed down by more senior coworkers: Don't. Repeat. Yourself. (DRY!)

You might already grasp the idea behind how "Don't repeat yourself" works after reading the phrase "Don't Repeat Yourself"

Let's see how the previous scenario would look if we apply this principle to the code snippet we saw before by thinking about how we could put the logic that we're repeating into a function:

// Newly created function to encapsulate the logic 
// to get the list of packages that we care about

function getPackagesToDeliver(packages, userId){
  return packages
      // packages have been inspected and are safe to deliver
      .filter(package => package.inspected && package.safe)
      // packages are ready for delivery
      .filter(package => package.status === Statuses.READY_FOR_DELIVERY)
      // packages are assigned to the chosen userId 
      .filter(package => package.assignedOperator === userId)
}

Great! we now put the code we had repeated in a lot of places inside a single function, and our job is 90% done.

✨Coding✨

We can now call this function everywhere we need to, replacing the code block that we were repeatedly and separatedly copying and pasting everywhere:

// ... We add it to the React component that displays this list
const myDeliverablePackages = getPackagesToDeliver(packages, currentUser.id);


// ... and for the code that prepares the payload for a confirmation email
const myDeliverablePackages = getPackagesToDeliver(packages, currentUser.id);
    
    
// ... and for the code that prepares the payload to change the state
const myDeliverablePackages = getPackagesToDeliver(packages, currentUser.id);
    
// ...
// ... and in several other places, you get the idea
// ...

This might not look like a big change or that it gives us that many benefits at first glance; I mean, it's only like 12 less lines that we have in or program so why go through the hassle of changing it?

We're already behind schedule and have other things to do, so why bother?

Keep in mind that even though we might have good intentions, it is possible to take this principle too far:

If we try to avoid repeating every minuscule amount of code as soon as we think we may need to use it in more than one place, or worse- if we try to preemptively apply DRY without a good reason to every code that appears more than once, then we may be unintentionally making things just as complicated as if we always repeated every code block.

For example, someone may look at the code we saw before and feel tempted to do this:

// We surely need the entire list of packages in several places, 
// so a function to return the entire list of packages makes sense right?
function getPackages(packages){

	return packages;
}

// We surely need to know which packages have been inspected in several places,
// so a function to return the packages that have been inspected makes sense right?
function getInspectedPackages(packages){

	return packages.filter(package => package.inspected);
}

// We surely need to know which packages have been inspected in several places,
// so a separate function to return the packages that are safe makes sense right?
function getSafePackages(packages){

	return packages.filter(package => package.safe);
}

// We surely need to know which packages are ready for delivery in several places,
// so a separate function to get packages ready for delivery makes sense right?
function getReadyForDeliveryPackages(packages){

	return packages.filter(package => package.status === Statuses.READY_FOR_DELIVERY);
}

// We surely need to know which packages are assigned to a specific user,
// so a separate function to get the packages assigned to a specific user sense right?
function getUserPackages(packages, userId){

	return packages.filter(package => package.assignedOperator === userId);
}


// Same function as before that groups the logic we need,
// but now it's harder to read, debug and reason about
function getPackagesToDeliver(packages, userId){

	const packageList = getPackages(packages);
    const inspectedPackageList = getInspectedPackages(packageList);
    const getInspectedSafePackageList = getSafePackages(inspectedPackageList);
    //ISRFD = (I)nspected (S)afe (R)eady (F)or (D)elivery
    const getISRFDPackageList = getReadyForDeliveryPackages(getInspectedSafePackageList);
   	const getUserISRFDPackageList = getISRFDPackageList(getISRFDPackageList, userId);
    
	return getUserISRFDPackageList;
}

After this, we now need to go through 5 other functions if we want to understand (and possibly modify!) the logic needed for us to get this information, or to debug the entire thing if we detect an issue and need to fix it. In this example we're unnecessarily putting every line of code into its own function because we think we're saving ourselves time in the long run.

But there really aren't any benefits to doing this. On the contrary, it gives us more work for the future!

There's also the possibility that we think that every two instances of a very tiny and minuscule piece of code is worth its own function because repeating code is bad since a random person at work told me so and we don't want to repeat code because everyone will think we don't know what we're doing.

Take this for example:

// In one file we have this ternary to get which hand
// a character of our video game will use
const handToUse = playerIsRightHanded ? player.rightHand : player.leftHand;

// ...
// ... and we use it again in a different file
const handToUse = playerIsRightHanded ? player.rightHand : player.leftHand;

Welp, we have TWO instances of the same code. The HORROR. That means we MUST create a function instead of repeating the code twice!

// function that encapsulates the logic (?)
function getHandToUse(playerIsRightHanded, rightHand, leftHand){
  return playerIsRightHanded ? rightHand : leftHand;
}

// We now can use it on the first file
const handToUse = getHandToUse(playerIsRightHanded, player.rightHand, player.leftHand);

// ...
// ... and we use it again in the other
const handToUse = getHandToUse(playerIsRightHanded, player.rightHand, player.leftHand);

Great, we have used DRY and we now feel better because the code is much more legible, clean, and maintainable!

Right?

Well no, because there was absolutely no benefit to doing this. It's a trivial example, but think about the amount of effort it took to write the function and whether it actually makes the code more maintainable or easier to read compared against the original:

// This function accepts three parameters, just like a the ternary
// There's no aggregated value or logic,
// we're actually just returning the ternary.

function getHandToUse(playerIsRightHanded, rightHand, leftHand){
  return playerIsRightHanded ? player.rightHand : player.leftHand; // Bruh.
}

// Calling it takes even more code than just using the ternary!
// And worse, it's less clear what this does...
const handToUse = getHandToUse(playerIsRightHanded, player.rightHand, player.leftHand);

// ... while if we used the ternary operator,
// we immediately know what is going on
const handToUse = playerIsRightHanded ? player.rightHand : player.leftHand;

Just use the ternary bud. A function that wraps a ternary is a waste of time, and also now if you wanted to make sure the logic of that function is tested you'd be adding a test that checks that the left operand is returned when one condition is truthy and the right operand when it's not.

That's straight up what the ternary operator does, why would you test that a ternary operator works as it should?

So yeah, try to adequately think about the tradeoffs regarding effort, overhead, maintainability and convenience for your codebase when assessing whether you should "un-repeat" a code block into a separate reusable piece just because it could be used more than once, or twice, or thrice because as we saw on those examples DRY can definitely go too far!

It was precisely that there were some instances of production code that I saw trying to create unnecessary abstractions very similar to the previous example what inspired me to write this blog post, and even to publish a small ESLint plugin that would flag that kind of code in order to spread the word about those unnecessary abstractions. You can check it out here if you want 😀:

eslint-plugin-unnecessary-abstractions