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 Class

A long class is a class that contains too many lines of code. It usually the case because we put too many responsibilities in a single class. Sandi Metz has a rule for this : if a class has more than 100 lines, it almost always means that it has more than one responsibility. By the way, if you have not seen her talk about rules, stop reading and watch it !

Wait… why is it bad ?

I don’t know why but, as developers, we usually think it takes less effort to add a bunch of methods to an existing class instead of creating a brand new one. Where am I going to put this new class ? What name should I choose ? How am I going to use this new class ? It might be a supplementary effort at the moment, but it is definitely worth it.

Let’s take an example of a really long class, with a bunch of methods. Imagine we have a car sharing company, so we might have a Rental class with different methods doing different things :

class Rental
  #...
  def retrieve_rental_data
    JSON.parse(File.read(data_input_path))
  end

  def total_price
    # ...
  end

  def total_fee
    # ...
  end

  def write_rental_data
    File.open(data_output_path, 'w') do |file|
      file.write(JSON.pretty_generate(data))
    end
  end
  # ...
end

The problem here is that in a couple of months (maybe weeks), this class will be way longer than 100 lines because other developers will tend to follow what has been done and they will add more and more methods to this Rental class.

Extract class(es)

In order to manage this issue, one of the solution is to use the « extract class(es) » method. As the name suggests, we are going to extract all the common behavior in a new class.

Going back to our example, we can extract some methods that do not belong to this class. I think we need to be really aggressive when it comes to class (or method) creation :

class Rental
  #...
  def total_price
    # ...
  end

  def total_fee
    # ...
  end
  # ...
end

class DataParser
  def retrieve_data(file_path)
    JSON.parse(File.read(file_path))
  end
end

class DataWriter
  def write_data(file_path, data)
    File.open(file_path, 'w') do |file|
      file.write(JSON.pretty_generate(data))
    end
  end
end

Notice that we change the name of the methods. That way it will be easier later to reuse those classes without knowing of a Rental object. We will have to inject the arguments (like the file_path we will want to use). This is exactly the promise of the Open/Closed Principle : to create classes that are open for extension but close for modification.

Ok, today’s code smell was pretty common but it is always good to remind this kind of common best practices.