Effective Coding: Don't Repeat Yourself

I've written about making sound effects, writing music, and even creating pixel art, all from the perspective of a programmer, so I figured at some point I should actually write about programming.

I was going to write the typical article that reads "8 weird habits of effective programmers, number 5 will literally make your brain explode", but then I realized I have really a lot to say on these subjects and a brief overview just wouldn't be sufficient.  

So, instead I will be doing a deep dive into one particular subject, and if enough people find it useful, I'll write more articles like this.  

But first, if you're one of the many people who learns better by listening and watching, I put together a video that goes over all these points.

So what's the subject? 

I'll be discussing a programming principle called Don't Repeat Yourself, or "DRY". It's an extremely useful principle to follow for all programming, not just game code.

The basic premise is to never have duplicate code in your program. It may sound obvious, why would anyone just copy and paste code? But code duplication is actually a surprisingly common programming problem that I've seen from both beginner and senior level programmers.

This typically manifests itself when you want to write two functions that are similar, but have a few differences. Instead of following the DRY principle, the programmer in question will copy and paste the function, only to change the few lines that are needed.  This leaves much of the code in the two functions duplicated.  

Consider this example of Code duplication:

shoot() { 

    ammo--

    spawnProjectile()

}

shootUnlimited() { 

   spawnProjectile()

}

In this example, spawnProjectile() is duplicated. Yes it's just one line, but it's a contrived example and represents potentially many lines of copied code.

Why is this a problem?

Code duplication is very bad for several reasons.

1. It's a maintenance nightmare

Every time you update your code, you have to then find everywhere else you have copied it and update it there also. This is especially problematic if you are working with a team of programmers who may need to edit your code - how will they know that you've copied it, and where?

The maintenance problem leads into problem #2:

2. It introduces bugs

When you update your code, you might accidentally miss one of the spots where it has been copied, causing an error. You might correctly update all the spots, only to realize that the new code wasn't intended to be copied elsewhere.

Sound confusing? It is. In fact, that's problem #3:

3. It's confusing

It's difficult to parse multiple similar functions in your head. Why are there so many? What's the difference? Which one do I call? It's just more total code that your brain needs to read and process. Less code to do the same thing is often better, and avoiding duplicated code is always better.

Common rebuttals

For much of my career in the games industry I've been a lead programmer, which means I am often reviewing other people's code and potentially asking them to change it... for the greater good!  All too often, programmers will become defensive and try to justify why they have violated the DRY principle. These are some of the common rebuttals excuses I have encountered:

1. It's impossible to write it any other way

Beginner programmers will give you this one. "It can't be done!" they will say. Here's an example which will elict the response: "There is no way to write this without copying that line".

if (isEmpty()) {

    if (canReload()) {

        reload()

        spawnProjectile()

    } else {

        playEmptySound()

    }

} else {

    spawnProjectile()

}

There is always a way to avoid code duplication. Sometimes it's as easy as rearranging the code.

if (isEmpty() && !canReload() {
    playEmptySound()
} else {
    if (isEmpty() && canReload()) {
        reload()
    }
    spawnProjectile()
}

The programmer might protest further... "Ah but now you have a condition copied! You wrote isEmpty() && canReload() twice! Sort of! See? It is impossible!"

So you write...

if (isEmpty()) {
    if (canReload()) {
        reload()
    } else {
        playEmptySound()
    }
}

if (!isEmpty()) {
    spawnProjectile()
}

More protests might be coming... "But that's not EXACTLY the same"... etc. but I think the point has been made; there are many ways of writing the same block of code, and you want to choose the one that has no code duplication.

As much protesting as the above excuse will give you, this next one is far more difficult to deal with:

2. It's faster "for me" if I write it this way

I'll go on a quick tangent here to say that I have a large number of programming principles like DRY that I follow and expect other programmers working on my projects to follow. The people who follow them are not just producing fewer bugs, they're also faster. Sometimes orders of magnitude faster than their copy & pasting colleagues.

Here's why it is not faster for you to copy and paste your code:

1. You are likely not considering how much time you spend in the future updating all the spots you have copied your code

2. You are also probably not considering all the time you will have wasted (your time and other people's time) fixing bugs you have introduced

3. It really just isn't faster to copy & paste and find the spots you want to change and re-write them than it is to just write the whole thing without code duplication to begin with.

If you're finding that it takes you a long time to write code without duplication, you just need more experience. Do it more, practice it more. Soon it will be like second nature and much easier. Ironically, my guess is you will probably find that you are faster too.

Experience is an important point here - some people have been coding for a long time, but are used to the way they code and aren't really improving. Programmers in this category are often the culprits for this next excuse:

3. It's okay if I only do it recreationally occasionally

Sometimes programmers are experienced enough to know not to duplicate code, but then do it anyways. The reasons given are always the same: "It was only 5 lines" or "I only do this once in a while" or "ARE YOU JUDGING ME? I CAN QUIT WHENEVER I WANT!" Well, maybe not that last one.

Truthfully, I've rarely heard a reasonable excuse for copying and pasting. I've even seen the trusted "I only did this because Cert is tomorrow!" fail before because, prophetically, the code in question was broken and needed to be patched.

The whole idea is to get into the habit of writing robust code so that you are comfortable with it. So if you find yourself feeling the urge to just "quickly copy and paste" something, that's a great opportunity for you to stop, reconsider, and improve your abilities.

Solutions

So we know it's bad, we've debunked some common rebuttals, so now how do we avoid code duplication? There are different approaches depending on the type of code you are working with.

1. Rearrange

As seen in the above example, sometimes it is possible to just rearrange your code to avoid a copy and paste. In particular, if the code in question has no order dependency, it may be possible to group all the duplicated code into a similar block, then split it into a function.

Consider these two heinous functions:

shootOctopusGun() { 
    ammo--
    if (isEmpty()) { 
        reload()
    }

    playSquishySound()
    playInkFX()
}

shootMonkeyGun() { 
    ammo--
    if (isEmpty()) { 
        reload()
    }

    playHollerSound()
    playBananaFX()
}

Since the common code is all adjacent, you could very easily split it into a separate function.

shootCommon() {
    ammo--
    if (isEmpty()) {
        reload()
    }
}

shootOctopusGun() { 
    shootCommon()
    playSquishySound()
    playInkFX()
}

shootMonkeyGun() { 
    shootCommon()
    playHollerSound()
    playBananaFX()
}

Of course, you still have duplicated code here - shootCommon(). It's not really removing the code duplication, but rather shrinking it.  Also, making too many functions like this can also be confusing. So in the example shown, this isn't even close to the best solution. Don't do this. We'll slowly improve this code until it looks quite nice by the time we're done.

There are of course cases where using functions makes sense. Functions are useful when you have common code that must be strewn about many different areas. For example, a Vector::getDistance() function. I think most programmers understand the typical use case for functions like this.

2. Conditions

Another alternative is to use conditions. This is just an if-else or switch-case block that flips on some condition. It's a slightly better way of writing the above example as the code duplication finally disappears:

shoot() {
    ammo--
    if (isEmpty()) {
        reload()
    }

    switch (gunType) {
        case OCTOPUS: playOctopusFX(); break
        case MONKEY: playMonkeyFX(); break
    }
}

playOctopusFX() { 
    playSquishySound()
    playInkFX()
}

playMonkeyFX() { 
    playHollerSound()
    playBananaFX()
}

Marginally better, but not great. We'll continue to improve this later.

Conditions are useful when you want some change to a (usually small) part of your function, like the following:

drawString(String text, Align alignment, Coord position) {

  int width = calcWidth(text)

  int x = position.x

  switch (alignment) {

  case CENTER: x -= (w / 2); break

  case RIGHT: x -= w; break

  }

  // Drawing code below...

}

In this case, the calling code gets to decide how the function behaves, which is useful. You may want to draw a string from many different alignments, and the onus is on the caller to decide what alignment is appropriate.  The bulk of the drawing code remains the same, and simply uses a new x-coordinate based on the alignment property.

I don't recommend conditions when the content of the condition is drastically changing the behaviour or meaning of the function. In those cases, you may want to use:

3. Inheritance or Composition

By now, OOP (Object-oriented-programming) junkies are chomping at the bit screaming "USE INHERITANCE!111 A FLJSLDKFJ".  There are some situations where inheritance or composition is a great solution.  For now, we'll skip over composition, as deciding when to use composition over inheritance merits its own entire article.  

So how does our still-ugly code example look with inheritance?

Gun::shoot() {

  ammo--

  if (isEmpty()) {

     reload()

  }

}

// OctopusGun and MonkeyGun derive from the Gun class

OctopusGun::shoot() { 

  Gun::shoot()

  playSquishySound()

  playInkFX()

}

MonkeyGun::shoot() { 

  Gun::shoot()

  playHollerSound()

  playBananaFX()

}

Better still. Code duplication is still gone, and we've removed that unnecessary switch-case statement. Furthermore, we've divided the custom FX code into separate classes, which can be a nice way to keep them organized.

But, as I'm sure you can guess, we can do better. More on that next.

Using inheritance (or composition) is often very clean, but I don't recommend it in the above example. Now I need to add new code and a new class for every different type of gun, when the only difference might be the kind of FX it creates. So how can we improve this?

4. Use data

Rather than write new code and new classes to define how each gun might differ, it is often possible to just use data to drive how your code behaves. Let's see how this looks:

// in Gun class

int ammo

// Serialized denotes that these variables are read from data

Serialized  {

  int numAmmoConsumed

  Sound firingSound

  Effect firingEffect

}

shoot() { 

  ammo -= numAmmoConsumed

  if (isEmpty()) {

     reload()

  }

  playSound(firingSound)

  playEffect(firingEffect)

}

// in WeaponData.dat

[Gun]

numAmmoConsumed=1

[OctopusGun]

firingSound=Squishy

firingEffect=Ink

[MonkeyGun]

firingSound=Holler

firingEffect=Banana

[EnergyGun]

numAmmoConsumed=0

firingSound=Zap

firingEffect=MuzzleFlash

Now we're talking. The shoot() function is short, has no inheritance, only 1 condition, is straight-forward and easy to debug.  Perhaps a game designer even came in and added a new weapon - EnergyGun - and they didn't even need source code or any programming knowledge. In fact, they were able to add that weapon in real-time without needing to recompile or even restart the engine. That's awesome!

Of course, you can have too much of a good thing. Using exclusively data to define the behaviour of 100 unique guns isn't going to be clean. You're going to end up with a single enormous Gun class filled with code that might pertain to only 1 of those 100 guns. 

So when do I use what approach?

Some rules of thumb(s):

1. Use Data and Base-class code to drive functionality that is common to most types

Example: playSound(firingEffect)

2. Use conditions to make small functionality changes throughout your code

Example: if(isEmpty()) reload() 

3. Use a derived-class or composition to implement functionality for a group of similar types whose behaviour differs sufficiently from the norm.

Example (this time we'll use composition): 

shoot() { 

  firingType->executeFire()

}

// Two examples of firing code that are different enough in behaviour such that they each warrant a new class

ProjectileFiringType::executeFire() { 

  Projectile p = spawnProjectile()

  p.setVelocity() // .. etc.

}

TraceFiringType::executeFire() { 

  Result result = physics->rayTrace()

  if (result.hitEntity) // .. etc.

}

Conclusion

Hopefully this post helped shed some light to a very useful programming principle - Don't Repeat Yourself. I have a huge list of programming principles like this one, so if enough people find this useful, I will write more articles like this :)

Share your thoughts by commenting below!