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!
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.
My tech niche is not a Big Shot Software Engineer™, so take my opinion with a grain of salt. Still, from my limited understanding, if in a very DRY project you get “brittle code where simple changes are scary because they could have a huge ripple effect”, what would be the alternative experience you would get in a, uh, humid project? Instead of “ok this one change overhauls an implicit assumption in every single part of the program” you get “ok this one change overhauls an implicit assumption in some parts of the program, and some other parts of the code might be making another implicit assumption incompatible with the first, and hey maybe these parts overlap, who knows”. Again, based on my limited understanding, trading the first scenario for the second does not seem that attractive a deal.
But really the true reason for my opinion is that once somewhere on the internet I saw someone oppose DRY principles using the slogan “Don’t DRY yourself out”, and the lame '90s PSA campaign energy inherent to that phrase infuriated me so much that I’ve supported DRY in software design ever since, out of pure spite. Behold – man, the most rational of all animals