DevOps Zone is brought to you in partnership with:

https://about.me/toch Christophe is a DZone MVB and is not an employee of DZone and has posted 1 posts at DZone. You can read more from them at their website. View Full User Profile

When is your code DRY enough?

08.18.2014
| 7302 views |
  • submit to reddit

You're aware that duplication is a rampant disease: more code, more fragile, less maintainable, less readable. You even use one or another tool and make your code reviewed to find most of them.

When facing some duplicate code, you're not always feeling comfortable to dry it up. You're not even sure you'll keep - as is - the code you've just wrote. By experience, you don't want to spend a whole day to end, maybe, with an abstract solution end to reason about.

Should you avoid similar create actions in different Rails controllers? Should you use metaprogramming and avoid long similar case expressions? What about recurring pattern in your Rails routes? Or multiple repetitions in your specs? When should I stop?

You're right to wonder if it's right or not. Reducing duplication is not only time consuming but it could make your code less readable. Eventually, you want to make your code more maintainable, not less.

DRY is not about identical chunks of bits or characters in different places. DRY aims at not repeating domain concepts. If not, there is a risk that some copy of a domain concept won't be maintained properly. For instance, when implementing invoicing, you want the calculation of VAT implemented in only one place. By doing so, if it changes, there is only one place to change.

When removing duplications that doesn't belong to the same domain concept, it's likely that you'll come with a wrong abstraction. If one of the factored domain changes, it will affect the abstraction and every dependencies that has then to be handled as exceptions. Moreover, the code will be hard to understand and to debug, especially if you use metaprogramming.

In order to know when you should dry your code, you could follow the rule of thumb Three Strikes And You Refactor. I personally apply a more specific approach:

  1. when implementing a feature, code without worrying about duplication (in a dedicated branch)
  2. when you've reached a stable implementation, before merging it, check for duplication (thanks to you, code review, a tool)
  3. for each found duplication
    • don't remove it if
      • you don't - yet - understand them as duplicated concepts
      • explicit code is more important as in tests
      • the solution is complex and hard to understand
    • otherwise remove it

In any case, if you find a recurring pattern in your code more than three times, it should particularly gains your attention.

If you feel uncertain, it's better to let the duplication for the moment. Then, if possible, discuss it during the code review. The reviewer will maybe come with another perspective helping you to come with a good abstraction.

Published at DZone with permission of Christophe Philemotte, author and DZone MVB. (source)

(Note: Opinions expressed in this article and its replies are the opinions of their respective authors and not those of DZone, Inc.)

Comments

Pierre-yves Saumont replied on Wed, 2014/08/20 - 7:53am

Any code containing loops is not DRY. Repeating loops again and again in code is bad practice. Everyone should try to remove all loops (and all if .. then ... else  and switch ... case statements as well) from the code, because this is really repeating oneself.

Of course, as you say, you have to understand them as duplicate concepts, but there is no reason not to do this also in tests. And if the solution seems more complex or hard to understand, it is one more reason to do it. By repeatedly doing so, you will gain a better understanding of these otherwise "complex" solutions, and they will become natural.

The best solution is of course to use a language that does not offer these constructs.

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.