Don't let duplication mislead you

In this article I'm going to tell you about how I started to understand what software design principles mean and hopefully it will give you some insights on how to put them in practice.

Duplication is better than the wrong abstraction

Sandi Metz is one of my favorite authors, every time I read something from her it changes how I think about improving my code. Something that I kept thinking about lately was this phrase: "Duplication is better than the wrong abstraction".

The first language I used professionally was Ruby and the Ruby community is well known for making programming fun and creating readable code, so I got into reading a lot of books/articles on how to write good code.

One of the first things I learned was to name things, but since naming is a quite subjective topic, the second thing I learned really stuck with me: Don't repeat yourself!

The famous DRY principle taught me that duplication is bad for maintenance, so try and reduce it as much as you can. When I heard "Duplication is better than the wrong abstraction" my brain crashed. And I had to reboot it.

What do I not know about this code?

It was actually "easy" to understand why duplication is better than the wrong abstraction, my friend Luciano Ramalho said "it's better to walk than to take the wrong bus" and I think that is a pretty good metaphor. But it was only when I faced my self with a duplication and resisted the urge to extract a new method that I realized why duplication is better than the wrong abstraction.

I was taking a look at a code I had just written following the Shameless Green approach (from 99 Bottle of OOP - amazing book by Sandi Metz and Katrina Owen by the way) and found a easy to strike duplication:

def self.create_successful_event(event)
  create(leave_id: event.leave.id,
          name: event.name,
          data: event.to_json,
          status: SUCCESS)
end

def self.create_failed_event(event)
  create(leave_id: event.leave.id,
          name: event.name,
          data: event.to_json,
          status: FAILED)
end

Instead of just extracting to a method and passing stuff as parameter I thought to myself: "what do I not know about this code?". And then it hit me: duplication is not about two similar pieces of code, it's about an abstraction that is yet to exist.

The real problem is not just the duplication, the event object is completely exposed! It gets passed as a parameter and then completely shattered to make up this hash object that is then passed to the create method.

Other code smells might help

What I didn't know about this code, and in hindsight it looks kinda obvious, was that the event object should be responsible for building that data hash.

When an object cares more about receiving parameters than itself, it usually means that a feature is misplaced - a code smell known as Feature Envy. To remove the original duplication what I ended up doing was creating a new method on event to return the data hash and then just calling it instead of exposing the object.

def self.create_successful_event(event)
  create(event.data.merge(status: SUCCESS))
end

def self.create_failed_event(event)
  create(event.data.merge(status: FAILED))
end

I know data is a bad method name :/

Refactoring with confidence

I got into the habit of reading the code and identifying smells first, instead of just diving into the code to make improvements, and I think it really helped me refactor with confidence.

There are many automated tools that help identify code smells and I try to use them, but nothing beats intuition! Invest some time reading books on refactoring, code smells, design patterns and specially reading code.

Take some time in the morning to just read through your code, resist the urge to make changes though, focus on how the pieces fit together instead. This way you'll build the foundations of your intuition and understanding what all those design principles really mean.

This post is a longer version of this twitter thread

Comments

[Ian Johnson]

Great topic. I've seen this as a source of frustration from many devs trying to push a round peg into a square hole. It really underscores the need to think, rather than just blindly following mantras and anecdotes. I read a similar article not long ago.

From DRY revisited:

Can it be that two classes have identical parts yet still don’t break DRY? Yes. The DRY principle restricts the presence of domain knowledge, it doesn’t put any bounds on the actual code which is required to express that knowledge. The fact the two entities have the same functionality doesn’t mean they violate DRY. In fact, both the Product and the Customer classes hold their own semantics. It just happened that they do it using the identical code in this particular case.