Published on

DRY is the root of all bad code!

Introduction

We've all been there. A pull request review comes in, and someone comments: "This looks repetitive. We should extract this into a shared function."

They might even quote the sacred texts of Clean Code, Uncle Bob or drop a link to some blog post about the DRY principle.

But what if I told you that this reflexive push toward code reuse could be making your codebase worse, not better?

What is DRY?

DRY (Don't Repeat Yourself) is one of those principles that seems so obviously correct that we rarely question it. The idea is simple: every piece of knowledge should have a single, unambiguous representation in your system.

Sounds great in theory. After all, who wants to maintain the same logic in multiple places? But like many principles in software engineering, blind adherence to DRY has lead me down to dangerous paths.

(Alleged) Benefits

The traditional arguments for DRY are compelling:

  • Reduced maintenance burden - change things in one place
  • Consistent behavior across the codebase
  • Less code to test and maintain
  • Easier to enforce standards and patterns

Welcome to the Real World

But the real world isn't as clean as our principles would like it to be. Here's some concrete examples where DRY can actually harm your codebase.

Shared Test Assertions Trap/Crap

You're writing tests and notice similar assertion patterns. Being a "good" developer following DRY, you create a shared assertion helper:

public static class UserAssertions
{
    public static void AssertValidUser(User user)
    {
        Assert.NotNull(user);
        Assert.NotEmpty(user.Name);
        Assert.True(user.Age >= 18);
        Assert.NotEmpty(user.Email);
        // ... more assertions
    }
}

Now your tests look 'cleaner', whatever that means:

[Test]
public void CreateUser_ValidInput_CreatesUser()
{
    var user = userService.CreateUser("John", 25, "john@example.com");
    UserAssertions.AssertValidUser(user);
}

[Test]
public void UpdateUser_ValidInput_UpdatesUser()
{
    var user = userService.UpdateUser(1, "John", 25, "john@example.com");
    UserAssertions.AssertValidUser(user);
}

Looks nice and DRY, right? But what happens when:

  • One test needs slightly different validation rules
  • You need to add a new assertion for a specific test case
  • You want to test edge cases where some fields might be optional

You end up with either:

  1. Bloated assertion methods with flags and parameters
  2. Multiple similar but slightly different assertion methods
  3. Tests that are tightly coupled to shared validation logic

The reality? Your tests become harder to understand and maintain, not easier. When a test fails, you now have to dig through shared assertion code to understand what went wrong.

But wait, it gets worse! The tests become incredibly fragile! Any modification to the shared logic cascades through your test suite in unpredictable ways.

Tests might fail when they should pass, but more dangerously, they might keep passing while hiding serious bugs or unintended behavior changes.

Shared Constants

Another classic DRY trap is sharing constants between production code and tests:

public static class UserConstants
{
    public const int MIN_AGE = 18;
    public const int MAX_NAME_LENGTH = 50;
    public const string EMAIL_REGEX = @"^[^@\s]+@[^@\s]+\.[^@\s]+$";
}

public class UserValidator
{
    public bool IsValid(User user)
    {
        return user.Age >= UserConstants.MIN_AGE
            && user.Name.Length <= UserConstants.MAX_NAME_LENGTH
            && Regex.IsMatch(user.Email, UserConstants.EMAIL_REGEX);
    }
}

[Test]
public void ValidateUser_InvalidAge_ReturnsFalse()
{
    var user = new User { Age = UserConstants.MIN_AGE - 1 };
    Assert.False(validator.IsValid(user));
}

The problem? Your tests are now coupled to your implementation details.

Imagine you change a constant, you'll be effectively modifying both the production code AND its tests simultaneously.

This defeats the whole purpose of testing!

In our example, if someone accidentally changes MIN_AGE from 18 to 16 (which they shouldn't), the tests happily pass along, completely missing this regression or any sense of requirements.

Premature Abstractions

You're working on a simple e-commerce platform. The team has two services that handle orders and refunds. Each service has its own simple flow: validate, save, and notify the user.

public async Task ProcessOrder(Order order)
{
    await ValidateOrder(order);
    await SaveOrder(order);
    await NotifyUser(order);
}

// Similar code in the new RefundService
public async Task ProcessRefund(Order order)
{
    await ValidateRefund(order);
    await SaveRefund(order);
    await NotifyUser(order);
}

Then comes the "DRY enthusiast" with their pull request: "These methods look almost identical! We need an abstraction!"

public abstract class TransactionProcessor
{
    protected abstract Task Validate(Order order);
    protected abstract Task Save(Order order);

    public async Task Process(Order order)
    {
        await Validate(order);
        await Save(order);
        await NotifyUser(order);
    }

    private async Task NotifyUser(Order order)
    {
        // Shared notification logic
    }
}

Now we've created a rigid abstraction that:

  • Forces all transaction types to follow the same flow, making it harder to handle edge cases or variations.
  • Increases cognitive load (developers need to understand the abstraction) and follow through it layer by layer everywhere for no particular reason.

And it gets exponentially worse. Imagine this TransactionProcessor being used not just for 2, but for 5 or 6 different business processes. Each new addition increases the risk of unintended side effects.

Every change becomes a game of whack-a-mole, fix one flow, break two others. Before you know it, nobody dares to touch this "shared" code, and your "clean" abstraction becomes the messiest part of the codebase.

When DRY Actually Helps

Here's a simple example where DRY makes perfect sense. Consider parsing CSV files across different parts of your application:

public static class CsvParser
{
    public static IEnumerable<string[]> ParseCsv(string content)
    {
        using var reader = new StringReader(content);
        string? line;
        while ((line = reader.ReadLine()) != null)
        {
            // Handle quoted fields, escaping, etc.
            yield return line.Split(',')
                .Select(field => field.Trim('"'))
                .ToArray();
        }
    }
}

This is good DRY because:

  • The logic is purely technical with no business rules
  • Changes to CSV parsing SHOULD affect all usages
  • The interface is mature/stable and is not expected to change
  • There's no hidden coupling between business domains or side effects from using this across
  • The abstraction has clear boundaries

When to Keep it WET

Sometimes, it's better to Write Everything Twice (WET) than to create premature abstractions. Consider duplicating code when:

  1. The duplicated code serves different business purposes (e.g., order processing vs. inventory management)
  2. Changes to one instance are unlikely to apply to others (different business rules/flows)
  3. The abstraction would create more complexity than it solves (e.g., forcing unrelated flows into one pattern)
  4. The code is still evolving and needs flexibility for future changes

Its a striking balance between the size of duplication and the maintenance cost of it compared to a potentially complex abstraction

Ending Notes

DRY isn't inherently bad but it's the mindless application of it that causes problems. Instead of asking "How can I avoid repetition?", ask:

  • Will this abstraction make the code easier to understand?
  • Am I solving a real problem or a hypothetical one?
  • Is the coupling I'm introducing worth the reduced duplication?
  • Will this make future changes easier or harder?

Sometimes, embracing a little repetition can save you a ton!