Hacker News new | past | comments | ask | show | jobs | submit login

If you have to change in 5 places to change one "thing", the code is probably not DRY.



You have 5 places in code where you draw a blue rectangle:

    drawRectangle("blue", x0, y0, x1, y1);
Do you refactor them into drawBlueRectangle(x0, y0, x1, y1)?

It seems you removed the duplication but you didn't. Because if you now have the requirement to draw red rectangles instead you surely won't leave it as

    def drawBlueRectangle(x0, y0, x1, y1):
        drawRectangle("red", x0, y0, x1, y1)
So instead of changing 5 places you now have to change the invocation in 5 places and implementation in 1 place.

You can argue it's because you named it wrongly, it should be "drawWhateverThingTheyHaveInCommonRectangle" instead, for example drawHighlightingRectangle. And you'll be right.

But you don't know if they will have that thing in common forever. And splitting the code is harder than refactoring it, so it often leads to code like this:

     def drawHighlightingRectangle(someDecidingFactor1, someDecidingFactor2, x0, y0, x1, y1):
         if someDecidingFactor == something && someDecidingFactor2 != somethingElse:
            drawRectangle("red", x0, y0, x1, y1)
         elif someDecidingFactor2 == somethingElse:
            drawRectangle("blue", x0, y0, x1, y1)
This code may seems ok, but it tends to grow business logic inside, and you don't immediately know what combinations of deciding factors are actually possible without looking at all the invocations. So either you look at all the invocations before implementing your change (and then the refactor didn't actually save you any work - what does it matter if you look at code and change it vs just look at code and change stuff elsewhere), or you ignore the invocations and add your change in isolation (probably writing code that is redundant and overcomplicated because you handle cases that cannot happen).

This is obviously oversimplified example, but I've done these exact mistakes several times :)


I think you should ask yourselves why you are drawing blue rectangles. What is their purpose? Do they just happen to be blue? Or are they blue because they have they all "do the same job"? Maybe you should have

drawInlineHelpBox(x0, y0, x1, y1)

or

drawEnergyShield(x0, y0, x1, y1)

We can see from the language that unlike your example this is a true abstraction. You went from color parameter to a specific color. It's both phrased in terms of colors.

But here we go from color to ui elements. The terminology is completely different.

     def drawEnergyShield(shield_capacity, incoming_dps, x0, y0, x1, y1):
         hue = incoming_dps / shield_capacity
         alpha = shield_capacity / 200
         drawRectangle(hsva_color(hue, 1, 1, alpha), x0, y0, x1, y1)
And I think you should embrace having business logic in there.


Yeah, the idea of DRY is simple, but it doesn't change the fact that you have to use your judgement, do and learn from mistakes etc. Nothing ever does.

I'd tweak your code example to this:

     def drawHighlightingRectangle(someDecidingFactor1, someDecidingFactor2, x0, y0, x1, y1):
         def highlight_color(factor1, factor2):
             if factor1 == something && factor2 != somethingElse:
                 return "red"
             else:
                 return "blue"

         color = highlight_color(someDecidingFactor1, someDecidingFactor2)
         drawRectangle(color, x0, y0, x1, y1)
This doesn't Repeat the drawRectangle() call.


Maybe

    drawColouredRectangle("red", x0, y0, x1, y1)

    decideHighlightColour(someDecidingFactor1, someDecidingFactor2)
then call separately? Depends on the number of calls to drawHighlightingRectangle I guess.

Something I've learned recently is to try to think in terms of behaviours, then compose them instead of mixing them together. Also it's easier to juggle the pieces by erring on the side of decomposition initially, then recompose a bit once they're organised, but only if it makes more sense.


DRY is not very useful as a concept. It doesn't define any rules how to identify bad repeated code, nor how to change it and doesn't define limitations of the concept or it's application.

I know this sounds wild and harsh but we have to overcome the habit of explaining non trivial or even unsolved problems with truisms.


Yeah, it really only points out where you have a problem.

The solution is still up to your expertise. Including deciding some repetition in OK.

Still, being able to see these design problems has been immensely valuable in my work!


In my last interdisciplinary role, I tried to simplify this as "you drew circles around the wrong parts". If many lines still cross the module boundary...




Consider applying for YC's Summer 2025 batch! Applications are open till May 13

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: