[smichel17]

I haven't seen bugs due to DRY, but I've caused difficulty of adding new features and poor maintainability because of it.

When I first took over maintaining Red Moon, I was a very new dev and went a little DRY crazy. In particular, there's a state machine for the different filter states (running, paused, stopped, etc), that had a bunch of classes (one per state) that had a lot of overlap. I pulled some common behaviors out into a supertype that the classes with those behaviors now inherited from.

Except, it turns out some of those states ought to act differently (automatic pauses in secure apps should disable the overlay but not restore the backlight; manual pauses should do both), and now it's going to be extra work untangling the various states and pause logic. If I'd left the state machine (amongst other bits) as it was, this feature/bugfix would be implemented already. Also, the current state of things (pun intended) is a bit less readable, in my opinion.

[true_religion]

I have see a dry bug. It happens when you deduplicate code which isn’t an exact duplicate, but only appears to be.

For instance, one piece of code is supposed to do the same thing as another, but in context it caused a side effect that some other part of the code was inadvertently relying on.

[tommica]

>> It happens when you deduplicate code which isn’t an exact duplicate, but only appears to be.

Embarrassing to admit, but until now I didn't think in this perspective - and on hindsight it should be obvious, sometimes code might look duplicate, but is not

[ficklepickle]

Or it starts as duplicate but diverges over time. I've been bitten by the premature deduplication.

[jacobush]

Or worse, the natural progression should be that they diverge, but because they now share some kind of "reused" code in a library or class, an abomination is added to one or both users, just to keep the original "shared" feature.

Great fun.

[fiddlerwoaroof]

I’ve never really understood this, because undoing DRY is relatively easy: you either copy the new function/class and rename it or you inline it in the mistaken case and adjust the code to match.

[smichel17]

Easy, but it's still work. As a thought experiment: if it's so easy, would you be interested in doing it for me? (edit: Obviously not, but compare your internal resistance to if I'd asked you to do something trivial like fix a typo in the readme)

Here's the file I was referencing (only 100 lines!): https://github.com/LibreShift/red-moon/blob/master/app/src/m...

Here's the issue I've been putting off fixing because of it: https://github.com/LibreShift/red-moon/issues/208

The problem is that right now there is one PAUSE state, which is triggered by both manual pauses (brightness should be restored) and automatic pauses (in secure apps; brightness should not be restored). The problem wasn't exactly DRY, it's the misapplication: I merged bits that had identical code, but turned out not to have identical purpose.

And, for posterity, here's what it used to look like way back (DRYing was not the only change, I also split out notification stuff, so it's not a 1:1 length comparison): https://github.com/LibreShift/red-moon/blob/11ae916955ff8c36...

edit: and here's the original file, back mostly before I touched it at all: https://github.com/LibreShift/red-moon/blob/ed2ec4fd1c68611d...

[simonh]

I'm not sure what you don't understand - that doing things unnecessarily and then having to undo them is a waste of time?

Nobody here is saying DRY is bad, just that it's not universally the right thing to do without consideration.

[greenhatman]

It can become much more convoluted than just having a function used in many places. It usually takes the form of multiple levels of inheritance. And it's a pain to deal with, and somewhat of a pain to refactor as well.

[dkneeland]

Thanks for maintaining the app btw! Makes it a lot easier to get to sleep after staring at HN for an hour before bed.