Avoid code oversimplification with RoR

Through my years of working with Ruby on Rails, I've seen some common practices that I believe are not required and add complexity to the code base.

This really simple practices look like they do no harm, but can make code less readable, add verbosity and insert unnecessary code.

One expression methods

Methods are supposed to encapsulate a set of processes or expressions, but sometimes they are wrongly used to define a single logic unit. Most of the time, developers create this methods to make the code more readable.

Consider the next example:

class User < ApplicationRecord
  before_save: setup_user

  def setup_user
    update_complexity
  end

  def update_complexity
    self.complex = true
  end
end

After looking at the example above, think about why should we separate the logic in two different methods when we can just move the assignation inside of the setup_user method.

  def setup_user
    self.complex = true
  end

The first example is easier to read for the most of us, but it adds complexity to the model by adding one more method that is just redirecting to another one without doing anything else.

The second example is readable enough to quickly understand the code intentions and keeps the model clean.

Devs should always account for readability but also keep complexity in mind, it is easier to read a one line raw statement instead of dealing with fat models.

Prefixing the model name when naming methods, variables and constants

Naming variables, constants and methods can be such a basic but painful thing to do, the name you choose is not always understandable enough for others. It is not great also when names mean more that they should.

See the next example:

class User < ApplicationRecord
  USER_DEFAULT_ROLE = 'admin'
  ...
end

In the example above we define the user's default role as a constant, we are being very specific in the name so we know that the default role belongs to the user model, but... why?

If we call this constant from outside the scope of the model, we would call it like this User::USER_DEFAULT_ROLE, and as you can see, we are repeating the word "User". It may not look like it can affect the readability of the code, but imagine having several constants in the same model or just having long model and constant names...

class CompanyConciliationFee < ApplicationRecord
  COMPANY_CONCILIATION_FEE_DEVELOPMENT_COST_RATIO = 0.59
  ...
end

Look at how long it would look if we call this invented constant from outside the model scope!
MaintenanceFee::COMPANY_CONCILIATION_FEE_DEVELOPMENT_COST_RATIO

Yeah, the example might look like I wrote the first thing that came to my mind... but that's basically how naming works.

Another reason people do this is because they are thinking to use the constant within the scope of the model, where it wouldn't need to prefix the class name. This may be more straightforward but remember that all constants are always associated to the model they are defined into.

Prefixing the model or class can happen with constants, variables and methods, and even when they feel necessary, keep in mind that they will clutter your code.

Endless class reference

Endless class reference refers to something similar to one expression methods but through classes.

Sometimes we break down the code structure to what we think are the necessary classes to get the job done, I.E. services, presenters, PORO's or whatever type of code encapsulation we can use, but sometimes we lengthen the logic by using more than we need, and therefore increase complexity.

Look at the next example:

class UserGenerator
  def initialize(user)
  ...
  end

  def call
    UserAvatarUploader.new(user, avatar).call
  end

  def avatar
  ...
  end
end

class UserAvatarUploader
  ...

  def call
    AvatarUploader.new(:user, user, avatar)
  end
end

class AvatarUploader
  ...

  def call
    user.update(avatar: avatar)
  end
end

If you follow the process to create a user and upload an avatar, you can observe that we are actually using 3 different services to do one simple thing, upload an avatar.

This example is a very basic and simplified version of what happens in production code bases that are used by thousands of people every day. User's don't notice, but devs suffer when debugging only to find out that they have to follow an endless class reference to figure out where in the code the avatar is uploaded.

We don't need all those classes. Keep it simple... if you only job is to upload an avatar, follow the KISS principle and keep it in only one service, you don't need the others! It may feel to you that you are decoupling service responsibilities, but what you are actually doing is making your jr. teammates life very unpleasant one.

And there it was... a few cases that I often see in Ruby on Rails codebases that can be avoided. What do you think? Do you know about another common simple practice that can be avoided to simplify our code?