[gcpwnd]

That sounds like myriads of programmers have done it wrong for decades. Can you elaborate on this and maybe add a few sources?

[kqr]

The ultimate source is David Parnas' 1970s' articles On the Criteria to be Used in Decomposing Systems Into Modules, and Designing for Ease of Extension and Contraction by the same author. The 1960s' NATO conferences on software engineering also touch on this.

These articles essentially introduced modularization, and they argue very well for why their approach is superior to the misunderstanding we frequently see today.

There was a lot of good research into how to build software right back in the '60s and '70s that was mostly ignored/forgotten during the '90s which is when many of the mistaken patterns we see today was invented.

Not much of an elaboration, for which I'm sorry, but I think the original sources are a more efficient use of time than my retelling, for anyone curious.

Edit: I can say one thing in different words than the original papers: one thing you want from your modules is that their interfaces are stable. (This is what's known as "information hiding", i.e. the modules themselves can change as much as you want, but since their interface is stable those changes don't propagate to other parts of the code.)

If you define a module for your routes, then the interface of that module will have to change for literally anything you add or remove to the site. This is the opposite of a stable interface.

Figuring out how to design a stable interface is tricky -- this is why good engineers are worth so much -- but one rule of thumb is to base modules on business domain concepts. The business domain has likely existed for longer than your application, and it's concepts will likely outlive your application. They tend to be stable, compared to whatever implementation details seem reasonable today.

[gcpwnd]

Thanks a lot for response and sources. Will look at them now.

[BurningFrog]

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

[ajuc]

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 :)

[im3w1l]

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.

[BurningFrog]

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.

[smileypete]

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.

[gcpwnd]

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.

[BurningFrog]

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!

[blandflakes]

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...