Thursday, February 18, 2010

The evils of duplication and DRY...

For those who've taken formal software engineering education or have programmed as a career, this post might something you've already heard of. However, I found myself coming back to this principle again while working on some code, and I figured it deserved a post nevertheless.

DRY: "Don't repeat yourself". Originally formulated in Pragmatic Programmer (a great book I read as an intern), this guideline for software development is simple and to the point. The idea is that you should never have two places that contain duplicate representations of any given fact.

The most obvious implication of this is that you shouldn't copy code. If two places need a given chunk of functionality, abstract it to a base class, static method, helper class, etc. At work, I've heard this copying known as the "editor inheritance" pattern, the "editor" being your text editor.

Even if you don't copy paste your way into this situation, it is easy to run into in other ways. In magecrawl, the two ways to invoke a spell-like affect is via the "Cast Spell" screen and selecting an inventory item and selecting the use verb. I found that keyboard handler for both these screens were 99% identical. In the beginning, spells and spell like affects had no targeting possibilities, they always targeted the player. As I added targeting, I added it to both places not realizing the duplication. This became a problem when I wanted to add a new targeting type later on and nearly missed one of the two copies.

I find that when I find these areas, the best thing to do is address it either "right now" if your in development, or as soon as the next release occurs if your stabilizing for a release. I shelve whatever changes I was working on, and figure out how to remove the duplication. In my case above, I created a base class that contained the common code and had both classes inherit it. I then tested it thoroughly and submitted it before working on new features. Your refactoring hat and your new feature hat shouldn't be on at the same time, you look silly when you try. :)

The more advanced implications of DRY involve not repeating yourself in more than just near identical code. The book mentioned above handles some of those cases, and maybe I'll come back to them in some future post (when it isn't nearly midnight as well).


Ben said...

In a way, LINQ exemplifies DRY on a more subtle level. LINQ in IEnumerable lets you extract patterns that are usually implemented with foreach loops and replace them with function call chains, which are usually more concise. Most of the simple foreach cases--looping until you meet a condition, looping and accumulating a set of transformed values, or looping and filtering a subset of values--have been provided for, but it's easy to add your own extensions for more advanced cases that you find yourself using.

I like to think of DRY as "ruthlessly find patterns in your code that you are implementing in more than one place, then implement each one only once," and then define "pattern" as broadly as possible.

Roberto said...

I agree that DRY is one of the fundamental rules of software engineering. Personally, I think it is the number one design rule; as well as code duplication itself being an example of breaking this (like the copy and pasting you mention), even principles of encapsulation are partly a response to DRY.

And after many years I've come to the same conclusion you seem to be suggesting: fix it now!