Every developer knows what code smell is. I also thought I knew. But it is not just looking at code and saying “this code seems bad”. Code smells are a really documented topic with solutions to each problem, since 1999 and this book from Martin Fowler and Kent Beck. This article is part of a 23-Day Challenge on learning code smells and how to avoid them.

Long Method

Today we will learn more about a pretty common code smell : long method. The name is self explanatory. It happens when a method contains too many lines of code. I think that more than five lines is already a long method.

Wait… why is it bad ?

Because a long method is almost always a violation of the Single Responsibility Principle. It is not the case all the time, but a long method should make you start asking questions about what the method is really doing.

Another reason is that a long method is almost always hard to test. This problem is just a consequence of the fact that we are coupling too many objects. A long method almost always have too many dependencies, so testing it becomes hard.

Let’s take an example of a long method:

def write_shop_data(shop_id, file_path)
  shop = Shop.find(shop_id)
  data = {
    name: shop.name,
    size: shop.size,
    category: shop.category
  }
  File.open(file_path, 'w') do |file|
    file.write(JSON.pretty_generate(data))
  end
end

This method finds a shop, generates an Hash with this shop data and writes this data on a file. You can already see the multiple responsibilities, right ?

Exctract method(s)

The most common and easy solution to a long method is the extract method. The idea is simple : we create new methods by extracting code from the previous one. There is multiple benefits in this solution :

  • It reduces code duplication. By extracting method(s) from a long method, we will find code that can be reused elsewhere.

  • It increases developers happiness by improving readability. We achieve this by naming correctly the new method(s). And because we read code more often than we write it, it seems to be an important benefit :clap:

Let’s rearrange our previous code example using the extract method:

def write_shop_data(shop_id, file_path)
  File.open(file_path, 'w') do |file|
    file.write(pretty_data_for(define_shop(shop_id)))
  end
end

def pretty_data_for(shop)
  JSON.pretty_generate(data_for(shop))
end

def data_for(shop)
  {
    name: shop.name,
    size: shop.size,
    category: shop.category
  }
end

def define_shop(id)
  Shop.find(id)
end

Ok, so we have seen what are the benefits of tracking our long methods and trying to reduce them by extracting responsibilities out of them. I hope this helped!