One of the biggest challenges in building software is eliminating tight coupling between independent components. One way to do this is by employing DRY (don’t repeat yourself), a methodology in which repeated code is consolidated.
Every piece of knowledge must have a single, unambiguous, authoritative representation within a system
The alleged benefit is that by replacing copies of code scattered across a codebase with a single authoritative copy, and having all other components refer to that copy, you can reduce overhead when requirements change and make the code easier to reason.
While I agree with this in principle, in practice, people tend to think of this as de-duplicating code fragments, but what it should be about is de-duplicating responsibility.
Programmers shouldn’t refactor the code by obsessively consolidating all duplications. Instead, they should simply follow the Single-Responsibility Principle (SRP) and allow duplications where they see fit. Yes, that’s right: duplicated code isn’t bad if each copy has different responsibilities.
This is because over time, each module will have its own unique logic that makes changes to the common logic. Attempting to de-duplicate the common code in these scenarios will result in spaghetti code. It’s simply more maintainable to let each module have its own copy of the slightly mutated common logic.
While SRP and DRY are not mutually exclusive, when developers take “de-duplicating code fragments” approach to DRY, it usually breaks SRP.
An example of this I saw was when a JSON serializer and model of a Rails web app were tightly coupled. This was done by having the serializer dynamically expose the fields of a model by iterating over the fields on that model. Conceptually, it was something like this:
forbidden_fields = %w[col7] @record.class.column_names.each do |col| next if forbidden_fields.member? col value = @record.send(col) # ... # Some display logic altering `value` based on its type or some other factor # ... render field_name: col, field_value: value end
While the code seems DRY because it doesn’t force you to explicitly declare which fields to show, the problem here is that the serializer loses control. For example, if you were to add a new column to the model that you didn’t intend on exposing publicily, and forgot to update
forbidden_fields, you’re in trouble. The responsibility of the serializer (at least in this case) is to expose only the necessary information to the app consumer. Going for a more explicit approach would achieve this
%w[col1 col3 col5].each do |col| value = @record.send(col) # ... end