What should I consider when the DRY and KISS principles are incompatible?

Question

The DRY principle sometimes forces the programmers to write complex, hard-to-maintain functions/classes. Code like this has a tendency to become more complex and harder to maintain over time. Violating the KISS principle.

For example, when multiple functions needs to do something similar. The usual DRY solution is to write a function that takes different parameters to allow for the slight variations in usage.

The upside is obvious, DRY = one place to make changes, etc.

The downside and the reason it's violating KISS is because functions like these have a tendency to become more and more complex with more and more parameters over time. In the end, the programmers will be very afraid to make any changes to such functions or they will cause bugs in other use cases of the function.

Personally I think it makes sense to violate DRY principle to make it follow KISS principle.

I would rather have 10 super simple functions that are similar than to have one super complex function.

I would rather do something tedious, but easy (make the same change or similar change in 10 places), than make a very scary/difficult change in one place.

Obviously the ideal way is to make it as KISS as possible without violating DRY. But sometimes it seems impossible.

One question that comes up is "how often does this code change?" implying that if it changes often, then it's more relevant to make it DRY. I disagree, because changing this one complex DRY function often will make it grow in complexity and become even worse over time.

So basically, I think, in general, KISS > DRY.

What do you think? In which cases do you think DRY should always win over KISS, and vice versa? What things do you consider in making the decision? How do you avoid the situation?

Answer 1

KISS is subjective. DRY is easy to over apply. Both have good ideas behind them but both are easy to abuse. The key is balance.

KISS is really in the eye of your team. You don't know what KISS is. Your team does. Show your work to them and see if they think it's simple. You are a poor judge of this because you already know how it works. Find out how hard your code is for others to read.

DRY is not about how your code looks. You can't spot real DRY issues by searching for identical code. A real DRY issue could be that you're solving the same problem with completely different looking code in a different place. You don't violate DRY when you use identical code to solve a different problem in a different place. Why? Because different problems can change independently. Now one needs to change and the other doesn't.

Make design decisions in one place. Don't spread a decision around. But don't fold every decision that happens to look the same right now into the same place. It's ok to have both x and y even when they both are set to 1.

With this perspective I don't ever put KISS or DRY over the other. I don't see nearly the tension between them. I guard against abuse of either. These are both important principles but neither is a silver bullet.

Answer 2

I wrote about this already in a comment to another answer by candied_orange to a similar question and also somewhat touched on it in a different answer, but it bears repeating:

DRY is a cute three-letter acronym for a mnemonic "Don't Repeat Yourself", which was coined in the book The Pragmatic Programmer, where it is an entire 8.5 page section. It also has a multi-page explanation and discussion on the wiki.

The definition in the book is the following:

Every piece of knowledge must have a single, unambiguous, authoritative representation within a system.

Note that it is emphatically not about removing duplication. It is about identifying which of the duplicates is the canonical one. For example, if you have a cache, then the cache will contain values that are duplicates of something else. However, it must be made very clear that the cache is not the canonical source.

The principle is not the three letters DRY. It is those 20 or so pages in the book and the wiki.

The principle is also closely related to OAOO, which is a not-so cute four-letter acronym for "Once And Only Once", which in turn is a principle in eXtreme Programming that has a multi-page explanation and discussion on the wiki.

The OAOO wiki page has a very interesting quote from Ron Jeffries:

I once saw Beck declare two patches of almost completely different code to be "duplication", change them so that they WERE duplication, and then remove the newly inserted duplication to come up with something obviously better.

Which he elaborates on:

I recall once seeing Beck look at two loops that were quite dissimilar: they had different for structures, and different contents, which is pretty much nothing duplicated except the word "for", and the fact that they were looping - differently - over the same collection.

He changed the second loop to loop the same way the first one did. This required changing the body of the loop to skip over the items toward the end of the collection, since the previous version only did the front of the collection. Now the for statements were the same. "Well, gotta eliminate that duplication, he said, and moved the second body into the first loop and deleted the second loop entirely.

Now he had two kinds of similar processing going on in the one loop. He found some kind of duplication in there, extracted a method, did a couple of other things, and voila! the code was much better.

That first step - creating duplication - was startling.

This shows: you can have duplication without duplicate code!

And the book shows the flip-side of the coin:

As part of your online wine ordering application you’re capturing and validating your user’s age, along with the quantity they’re ordering. According to the site owner, they should both be numbers, and both greater than zero. So you code up the validations:

def validate_age(value):
 validate_type(value, :integer)
 validate_min_integer(value, 0)

def validate_quantity(value):
 validate_type(value, :integer)
 validate_min_integer(value, 0)

During code review, the resident know-all bounces this code, claiming it’s a DRY violation: both function bodies are the same.

They are wrong. The code is the same, but the knowledge they represent is different. The two functions validate two separate things that just happen to have the same rules. That’s a coincidence, not a duplication.

This is duplicated code that is not duplication of knowledge.

There is a great anecdote about duplication that lead to a profound insight into the nature of programming languages: many programmers know the programming language Scheme and that it is a procedural language in the Lisp family with first-class and higher-order procedures, lexical scoping, lexical closures, and a focus on purely functional, referentially transparent code and data structures. What however not many people know, is that it was created in order to study Object-Oriented Programming and Actor Systems (which the authors of Scheme considered to be closely related if not the same thing).

Two of the fundamental procedures in Scheme are lambda, which creates a procedure, and apply, which executes a procedure. The creators of Scheme added two more: alpha, which creates an actor (or object), and send, which sends a message to an actor (or object).

An annoying consequence of having both apply and send was that the elegant syntax for procedure calls no longer worked. In Scheme as we know it today (and in pretty much any Lisp), a simple list is usually interpreted as "interpret the first element of the list as a procedure and apply it to the rest of the list, interpreted as arguments". So, you can write

(+ 2 3)

and that is equivalent to

(apply '+ '(2 3))

(Or something close, my Scheme is pretty rusty.)

However, this does no longer work, since you don't know whether to apply or to send (assuming that you don't want to prioritize one of the two which the creators of Scheme didn't, they wanted both paradigms to be equal). … Or, do you? The creators of Scheme realized that actually, they simply need to check for the type of the object that is referenced by the symbol: if + is a procedure, you apply it, if + is an actor, you send a message to it. You don't actually need separate apply and send, you can have something like apply-or-send.

And that's what they did: they took the code of the two procedures apply and send and put them into the same procedure, as two branches of a conditional.

Shortly after, they also re-wrote the Scheme interpreter, which up to that point was written in a very low-level register-transfer assembly language for a register machine, in high-level Scheme. And they noticed something astonishing: the code in the two branches of the conditional became identical. They hadn't noticed this before: the two procedures were written at different times (they started out with a "minimal Lisp" and then added OO to it), and the verbosity and low-levelness of the assembly meant that they were actually written fairly differently, but after re-writing them in a high-level language, it became clear that they did the same thing.

This lead to a profound understanding of Actors and OO: executing an object-oriented program and executing a program in a procedural language with lexical closures and proper tail calls, are the same thing. The only difference is whether the primitives of your language are objects/actors or procedures. But operationally, it is the same.

This also lead to another important realization that is unfortunately not well understood even today: you cannot maintain object-oriented abstraction without proper tail calls, or put more aggressively: a language that claims to be object-oriented but doesn't have proper tail calls, isn't object-oriented. (Unfortunately, that applies to all my favorite languages, and it is not academic: I have run into this problem, that I had to break encapsulation in order to avoid a stack overflow.)

This is an example where very well hidden duplication actually obscured an important piece of knowledge, and discovering this duplication also revealed knowledge.

Answer 3

When in doubt, always chose the simplest possible solution which solves the problem.

If it turns out the simple solution was too simple, it can easily be changed. An overly complex solution on the other hand is also more difficult and risky to change.

KISS is really the most important of all design principles, but it often gets overlooked, because our developer culture places a lot of value on being clever and using fancy techniques. But sometimes an if really is better than a strategy pattern.

The DRY principle sometimes forces the programmers to write complex, hard to maintain functions/classes.

Stop right there! The purpose of the DRY principle is to get more maintainable code. If applying the principle in a particular case would lead to less maintainable code, then the principle should not be applied.

Keep in mind that none of these principles are goals in themselves. The goal is to make software which fulfills its purpose and which can be modified adapted and extended when necessary. Both KISS, DRY, SOLID, and all the other principles are means to achieve this goal. But all have their limitations and can be applied in a way where they works counter to the ultimate goal, which is to write working and maintainable software.

Answer 4

IMHO: if you stop focusing on the code being KISS/DRY, and start focusing on the requirements driving the code, you will find the better answer you are looking for.

I believe:

1. We need to encourage each other to remain pragmatic (as you're doing)

2. We must never stop promoting the importance of testing

3. Focusing on the requirements more will resolve your questions.

TLDR

If your requirement is to have parts change independently, then keep functions independent by not having a helper functions. If your requirement (and any future changes to it) are the same for all functions, move that logic into a helper function.

I think all of our answers so far make a Venn diagram: we all kind of say the same thing, but we give details to different parts.

Also, no one else mentioned testing, which is partially why I wrote this answer. I think that if someone mentions programmers being afraid of making changes, then it's very unwise to not talk about testing! Even if we "think" the problem is about the code, it might be the real problem is the lack of testing. Objectively superior decisions become more realistic when people have invested into automated testing first.

First, avoiding fear is wisdom -- Good Job!

Here is a sentence you said: the programmers will be very afraid to make any changes to such [helper] functions or they will cause bugs in other use cases of the function

I agree that this fear is the enemy, and you must never cling to principles if they are only causing fear of cascading bugs/work/changes. If copy/pasting between multiple functions is the only way to remove this fear (which I don't believe it is -- see below), then that is what you should do.

The fact that you sense this fear of making changes, and that you are trying to do something about it, makes you a better professional than many others who don't care enough about improving the code -- they just do what they're told and make the bare minimum changes to get their ticket closed.

Also (and I can tell I'm repeating what you already know): people skills trump design skills. If the real life people in your company are outright bad, then it doesn't matter if your "theory" is better. You might have to make decisions that are objectively worse, but you know that the people who will maintain it are capable of understanding and working with. Also, many of us also understand management who (IMO) micromanage us and find ways to always deny needed refactoring.

As someone who is a vendor that writes code for customers, I have to think of this all the time. I might want to use currying and meta-programming because there is an argument that it's objectively better, but in real life, I see people being confused by that code because it's not visually obvious what's happening.

Second, Better Testing Solves Multiple Problems at Once

If (and only if) you have effective, stable, time-proven automated tests (unit and/or integration), then I bet you will see fear fade away. For newcomers to automated tests, it may feel very scary to trust the automated tests; newcomers may see all those green dots and have very little confidence those green dots reflects real life production working. However, if you, personally, have confidence in the automated tests, then you can start emotionally/relationally encouraging others to trust it too.

For you, (if you haven't already) the first step is to research test practices if you haven't. I honestly assume you already know this stuff, but since I didn't see this mentioned in your original post, I have to talk about it. Because automated tests are this important and relevant to your situation you posed.

I am not going to try to single-handedly boil down all testing practices in a single post here, but I would challenge you to focus on the idea of "refactor-proof" tests. Before you commit a unit/integration test to code, ask yourself if there are any valid ways to refactor the CUT (code under test) that would break the test you just wrote. If that's true, then (IMO) delete that test. It's better to have fewer automated tests that don't needlessly break when you refactor, than it is to have a thing tell you you have high test coverage (quality over quantity). After all, making refactoring easier is (IMO) the prime purpose of automated tests.

As I have adopted this "refactor-proof" philosophy across time, I have come to the following conclusions:

1. Automated integration tests are better than unit tests
2. For integration tests, if you need to, write "simulators/fakes" with "contract tests"
3. Never test a private API -- be that private class methods or unexported functions from a file.

References:

• Stop Mocking, Start Testing
• IntegrationTest
• ContractTest

While you are researching test practices, you may have to make extra time to write those tests yourself. Sometimes the only best approach is to not tell anyone you're doing that, because they'll micromanage you. Obviously this is not always possible because the amount of need for testing may be bigger than the need for a good work/life balance. But, sometimes there are things small enough that you can get away with secretly delaying a task by a day or two in order to just write the tests/code needed. This, I know, can be a controversial statement, but I think it's reality.

In addition, you obviously can be as politically prudent as possible to help encourage others to take steps towards understanding/writing tests themselves. Or maybe you're the tech lead that can impose a new rule for code reviews.

As you talk about testing with your colleagues, hopefully point #1 above (be pragmatic) reminds us all to keep listening first and not become pushy.

Third, Focus on the Requirements, Not the Code

Too many times we focus on our code, and not deeply understand the bigger picture our code is supposed to be solving! Sometimes you have got to stop arguing about if the code is clean, and start making sure you have a good understanding of the requirements that are supposed to be driving the code.

It's more important that you do the right thing than it is that you feel that your code is "pretty" according to ideas like KISS/DRY. That's why I'm hesitant to care about those catch phrases, because (in practice) they accidentally make you focus on your code without thinking about the fact that the requirements are what provide a good judgment of good code quality.

---

If the requirements of two functions are interdependent/same, then put that requirement's implementation logic into a helper function. The inputs to that helper function will be the inputs to the business logic for that requirement.

If the requirements of the functions are different, then copy/paste between them. If they both happen to have the same code this moment, but could rightfully change independently, then a helper function is bad because it's affecting another function whose requirement is to change independently.

Example 1: you have a function called "getReportForCustomerX" and "getReportForCustomerY", and they both query the database the same way. Let's also pretend there's a business requirement where each customer can customize their report literally any way they want to. In this case, by design, the customers want different numbers in their report. So if you have a new customer Z who needs a report, it may be best to copy/paste the query from another customer, and then commit the code and move one. Even if the queries are exactly the same, the definitional point of those functions is to separate changes from one customer impacting another. In the cases where you provide a new feature that all customers will want in their report, then yes: you will possibly be typing the same changes between all the functions.

However, let's say that we decide to go ahead and make a helper function called queryData. The reason that's bad is because there will be more cascading changes by introducing a helper function. If there is a "where" clause in your query that is the same for all customers, then as soon as one customer wants a field to be different for them, then instead of 1) changing the query inside function X, you have to 1) change the query to do what customer X wants 2) add conditionals into the query to not do that for others. Adding more conditionals into a query is logically different. I might know how to add a subclause into a query, but that doesn't mean I know how to make that subclause conditional without affecting performance for those not using it.

So you notice that using a helper function requires two changes instead of one. I know this is a contrived example, but the Boolean complexity to maintain grows more than linearly, in my experience. Therefore, the act of adding conditionals counts as "one more thing" people have to care about and "one more thing" to update each time.

This example, it sounds to me, might be like the situation you are running into. Some people emotionally cringe at the idea of copy/pasting between these functions, and such an emotional reaction is OK. But the principle of "minimizing cascading changes" will objectively discern the exceptions for when copy/pasting is OK.

Example 2: You have three different customers, but the only thing you allow to be different between their reports are the titles of the columns. Notice that this situation is very different. Our business requirement is no longer "provide value to the customer by allowing compete flexibility in the report". Instead, the business requirement is "avoid excess work by not allowing the customers to customize the report much". In this situation, the only time you would ever change the query logic is when you will also have to make sure every other customer gets the same change. In this case, you definitely want to create a helper function with one array as input -- what the "titles" are for the columns.

In the future, if product owners decide that they want to allow customers to customize something about the query, then you will add more flags to the helper function.

Conclusion

The more you focus on the requirements instead of the code, the more the code will be isomorphic to the literal requirements. You naturally write better code.

Answer 5

Try to find a reasonable middle ground. Rather than one function with lots of parameters and complex conditionals scattered throughout it, split it into a few simpler functions. There will be some repetition in the callers, but not as much as if you hadn't moved the common code to functions in the first place.

I recently ran into this with some code I'm working on to interface with Google and iTunes app stores. Much of the general flow is the same, but there are enough differences that I couldn't easily write one function to encapsulate everything.

So the code is structured like:

Google::validate_receipt(...)
    f1(...)
    f2(...)
    some google-specific code
    f3(...)

iTunes::validate_receipt(...)
    some itunes-specific code
    f1(...)
    f2(...)
    more itunes-specific code
    f3(...)

I'm not too concerned that calling f1() and f2() in both validation functions violates the DRY principle, because combining them would make it more complicated and not perform a single, well-defined task.

Answer 6

Kent Beck espoused 4 rules of simple design, which relate to this question. As phrased by Martin Fowler, they are:

• Passes the tests
• Reveals intention
• No duplication
• Fewest elements

There is a lot of discussion over the ordering of the middle two, so it may be worth thinking of them as about equally important.

DRY is the third element in the list, and KISS could be considered a combination of the 2nd and 4th, or even the whole list together.

This list provides an alternative view to the DRY/KISS dichotomy. Does your DRY code reveal intent? Does your KISS code? Can you make ether version more revealing or less duplicated?

The goal isn't DRY or KISS, it's good code. DRY, KISS, and these rules are mere tools for getting there.