When DRY goes wrong
When DRY goes wrong
DRY has become a mantra throughout the industry. Any time repetitive code shows up, DRY gets applied as a cure all. If you even start to question DRYing up a piece of code, you are viewed as a heretic to the entire industry. Ok, maybe it’s not that bad, but many times DRY gets applied without much t...
DRY is nice and all, but never let your code get so DRY it chafes.
I think it’s one of those things that is generally a good instinct, but easy to go overboard and way over index on.
Usually I try to optimize code for “how easy is it to entirely delete this module” - loose coupling is the bigger concern. Sometimes it makes sense to repeat code with this approach. Usually I’ll abstract a shared concern out once I repeat it 3 or so times, but not always.
It’s all about trade offs.
I often see this problem in the testing world, particularly around frontend tests that utilize UI automation tools.
The pattern I see is often to abstract chunks of common steps into individual functions that often live in places very disconnected from the test. While this might reduce the number of lines of code in a test and arguably make it more maintainable it has its problems.
Main problem number one is that readability has been diminished. It is now harder to understand exactly what this test is doing because steps have been abstracted away. Tests that can be clearly understood, read and describe functionality and behaviors are immensely important to getting others to quickly understand code. I hate to put a barrier there to making that happen.
Second, i don't truly believe it ALWAYS improves maintainability. This decision of abstracting carries a risk. When that abstraction needs to change in one place you are faced with a tough choice...
Does this need to change in ALL places? How do you know? How can you get all places it is used and be certain it has to change in all of them? Changing for all usages is RISKY particularly when there are large numbers of uses and you don't know what they all do.
Do i make a new abstraction? This is safer but now starts to create bloat. It will lead down paths of making future implementations trickier because there are now two things to choose from that are possibly slightly different.
For tests I'm not really convinced that these problems are worth dealing with. Keep it simple and understandable. Repeating yourself for the sake of clarity is okay. I'll say it again... Repeating yourself for the sake of clarity is okay!
Yeah, I've seen this a ton. And it sucks because it's always done with good intentions.
That's why I think that if you need small helpers keep them short and prevent them from going too deep. Nothing is worse than digging through 10 layers of functions just to figure out what's going on.
Does this need to change in ALL places? How do you know? How can you get all places it is used and be certain it has to change in all of them?
These seem like questions that are equally important and hard to answer regardless of whether you're using abstractions, although abstractions have a big advantage in that you can find all the sites where they're used much more easily than you can find all the places where a piece of code is duplicated (or worse yet, duplicated with slight changes).
The example is more of an issue with using the right tool for the job: using classes inheritance and overloading would properly deal with the employee/manager/c-suite distinction.
For sure there are better abstractions that would help. I still think that they only help with over abstraction to a certain point. Digging 5 classes deep just to figure out what's actually happening is just as, if not more, frustrating than digging 5 functions deep.
This just serves as another reminder that I need to finish reading SICP. But that said, I think this brings up some very interesting points. The example provided of DRY is focused on what is happening on a human/company level, while the abstraction barriers provided focus heavily on what is happening on a software level. This is a differentiation that I feel like is extremely important when programming robust, maintainable software. You cannot let non-software related terminology seep into what is fundamentally, well, software.
When you let non-software terminology work its way into software, the software has to start making assumptions. What is a C level employee? What bonuses do they require? Are these things subject to change? The list goes on. But if you approach the problem with software first and foremost, it is clear that all is happening is a variable bonus is added to an employees compensation. It is not this layers problem what that value is, nor is it this layers problem who is being compensated. That is all concerns for a DB layer (of some form) somewhere up the chain. All the financial layer cares about is applying the calculations.
So I don't feel like this is really a case against DRY, as much as it's a case against using non-software terminology and applying non-software assumptions to what is fundamentally, software. The arguments for maintaining independent layers is also important, but if you're thinking fully in terms of software operation, I feel you can more comfortable determine when layers can be interlinked.
DRY is necessary to apply but not solely through abstraction