Thoughts about Clean Code: Too DRY


With good reason I want to pick up the topic about how to apply clean code to your code base – or at least how to make the (code) world a little bit better.

As it happened recently a team mate of us started to refactor code I wrote to dry it up. This lead to a deeper discussion inside the pull request where I stated that the code he provided isn’t as readable as the one before.

The Problem in Short

We have a (base) class which is used inside the context of storing information that is retrieved via a REST API. The class itself provides the attributes we need and there’s a mechanism to store and sometimes convert incoming data to those attributes.

One of the attributes we (for whatever reason) decided on to store is called rid_permissions which basically is a hash containing boolean values for certain properties. Those properties are delivered via the API. However there’s some adaption to be done in naming. To be consistent to the ruby style we want our objects to provide ?-methods for those properties.

Readable vs Scanable

You can write the code in different ways and each and every developer might have his or her own preference. There’s nothing to argue about it. Nevertheless during the discussion we had we not only talked about readability – that’s what you will find in the common literature – but I also used the term scannability which simply popped up in my mind. So what’s that?

There are some statements serving as a kind of definition for readability especially given in the book Clean Code. Without citing whole pages here are some statements pointing to the readability of (clean) code.

Clean code is simple and direct. Clean code reads like well-written prose. Clean code never obscures the designer’s intent but rather is full of crisp abstractions and straightforward lines of command. (Grady Booch)

Clean code can be read and enhanced by a developer other than its original author. (Dave Thomas)

So code has to be well readable, especially by other developers and be quickly understandable.

That’s great so far, but I would go one step further. For me (especially after working several days on a different context) as well as for other developers it’s very important to be able to scan files, i. e. quickly review them by scrolling through it and get the feeling for what the methods etc. are for. As soon as you have to stop and think about structuring elements, the code is not scannable anymore – even though it might still be readable after you dig into it and take your time to understand it.

To be scannable means: get a rough idea of what you find in the file without having to really think about it.

Example

Maybe this blog article is not the best choice for performing this experiment, but if you put it on a slide and show it to the audience for 1-2 seconds, the effect is clear.

Consider a model with attributes and (yep, a longer list of) short methods. While scanning from top to bottom the following code (which I am hopfulle able to remember correctly since it’s somehow gone) enters your view port. Give yourself one to two seconds.

×
  permissions = {
    :ignore  => 'ignoreable',
    :share   => 'recommendable',
    :delete  => 'deletable',
    :like    => 'likable',
    :comment => 'commentable'
  }
  permissions.each do |key,val|
    define_method("#{val}?") do
      rid_permissions.include? key
    end
  end

What does it do? Explain in one sentence after having seen it for a second. Yes, it’s readable, but…

Now consider the following code

×
  def ignorable?
    rid_permissions.include? :ignore
  end

  def recommendable?
    rid_permissions.include? :share
  end

  def deletable?
    rid_permissions.include? :delete
  end

  def likeable?
    rid_permissions.include? :like
  end

  def commentable?
    rid_permissions.include? :comment
  end

Sure, it smells a bit like duplication, but for me and even others I bothered with this, it is ways more readbable and especially quickly understandable what the class provides – this well scannable.

Conclusion: Don’t DRY up Too Much

This example lead me to the conclusion: don’t DRY up your code too much.

When adding meta programming to DRY up your code always think twice about the resulting scannability.


Es gibt noch keine Kommentare