Refactory scenario: removing conditional blocks and extracting code to a private method

This is a simple but very useful refactory strategy.

It allows you to easily make the code more readable and clear by having a method name that explains what’s going on and also avoiding blocks of conditionals by turning them into guard clauses.

Let’s use real code from a Rails action as an example, but this can be applied in any programming language:

# users/registrations#new
def new
  email = params[:email].strip.downcase
  @lead = Lead.find_or_initialize_by(email: email)

  unless @lead.persisted?
    valid_email_format = @lead.email.match(URI::MailTo::EMAIL_REGEXP).present?

    if valid_email_format && EmailVerificationService.new.valid?(email)
      @lead.valid = true
    end

    if @lead.valid
      clearbit_data = Clearbit.new.email_lookup(email)
      name_hash = clearbit_data.fetch('person', {}).fetch('name', {})

      if name_hash.present?
        first_name = name_hash.fetch('givenName', nil)
        if first_name.present? && first_name != 'Name'
          @lead.first_name = first_name
        end

        last_name = name_hash.fetch('familyName', nil)
        if last_name.present? && last_name != 'Name'
          @lead.last_name = last_name
        end
      end
    end

    @lead.save! if @lead.changed?
  end
end

Whaaaat?! Why is this action so long? What’s going on here!?

Yup that would also be my reaction seeing that amount of code in an action 😆

Even though it’s on a Users::RegistrationController, it’s doing many things with the Lead model. Yup, that’s how it is on real-life projects. 😉

However, should the programmer have to read every single line of code to grasp what’s going on there? Nope!

Extracting related logic to a private method

So the first thing I like to do is extract all the code that’s related to the same thing to a private method:

def new
  email = params[:email].strip.downcase
  @lead = Lead.find_or_initialize_by(email: email)

  personalize_the_signup_view
end

private

def personalize_the_signup_view
  unless @lead.persisted?
    valid_email_format = @lead.email.match(URI::MailTo::EMAIL_REGEXP).present?

    if valid_email_format && EmailVerificationService.new.valid?(email)
      @lead.valid = true
    end

    if @lead.valid
      clearbit_data = Clearbit.new.email_lookup(email)
      name_hash = clearbit_data.fetch('person', {}).fetch('name', {})

      if name_hash.present?
        first_name = name_hash.fetch('givenName', nil)
        if first_name.present? && first_name != 'Name'
          @lead.first_name = first_name
        end

        last_name = name_hash.fetch('familyName', nil)
        if last_name.present? && last_name != 'Name'
          @lead.last_name = last_name
        end
      end
    end

    @lead.save! if @lead.changed?
  end
end

Ok I already see a significant improvement, if the developer reading this code isn’t interested in code related to personalization of the view for the Lead, he can stop there 👍

Removing conditional blocks

So now we can get rid of some conditional blocks by turning them into guard clauses:

def personalize_the_signup_view
  return if @lead.persisted?

  valid_email_format = @lead.email.match(URI::MailTo::EMAIL_REGEXP).present?

  if valid_email_format && EmailVerificationService.new.valid?(email)
    @lead.valid = true
  end

  return unless @lead.valid

  clearbit_data = Clearbit.new.email_lookup(email)
  name_hash = clearbit_data.fetch('person', {}).fetch('name', {})

  return if name_hash.blank?
  
  first_name = name_hash.fetch('givenName', nil)
  if first_name.present? && first_name != 'Name'
    @lead.first_name = first_name
  end

  last_name = name_hash.fetch('familyName', nil)
  if last_name.present? && last_name != 'Name'
    @lead.last_name = last_name
  end

  @lead.save! if @lead.changed?
end

I find it easier to understand what’s going on. You can see that if the Lead is already in the database, we stop there. If the email has an invalid format, we won’t even bother calling Clearbit API, and we also return early if Clearbit doesn’t return the kind of data we are expecting.

Yeah, I know there are other ways to optimize this code even more, but the goal of this post is to show only “one thing”. One example, so it’s easier to understand.

Must-have gem for rails projects: zero downtime migrations

This is one of the most useful Ruby gems I’ve discovered in a while: zero_downtime_migrations, it catches problematic migrations at development time.

Problems such as:

  • Adding a column with a default in a single migration;
  • Adding a non-concurrent index;
  • Mixing data changes with index or schema migrations;
  • Performing data or schema migrations with the DDL transaction disabled;
  • Using each instead of find_each to loop thru ActiveRecord objects when doing data changes;

And it doesn’t only catch the problem but it also teaches what you should do to avoid it 👏👏

Example from their README:

Solution of problem and solution

Seriously, share this gem with every Rails developer you know and make everybody’s life a bit easier 😁

Someone coding

How to delete local git branches after the merge

From time to time we all get annoyed by the git branch autocomplete, which used to be so useful, but now makes us practically write the whole branch name! 😡

Why it keeps happening? Because we are awesome developers who use feature branch flow and now, after working so hard, we have dozens of branches on our local machine.

Yogo, can you tell me an easy way to clear these branches? I don’t wanna delete each one manually! 😩

I sure can, my friend! Here is an alias that I use to delete all my merged branches:

alias clear_merged_branches="git branch --merged | egrep -v '(^\*|master|production)' | xargs git branch -d"

It will only leave the “master” and “production” branches, which you can easily change on that code. Besides the branches that haven’t been merged into master, of course.

Remember to use this alias while you are on the “master” branch 😉

Update

As lpmusix pointed out, you can also use it as a git alias. Add to the alias section of your ~/.gitconfig:

cb = "!git branch --merged | egrep -v '(^\\*|master|production)' | xargs git branch -d"

Ruby already has its own regular expression to validate emails

I’ve searched and written a lot of regular expressions to validate email on our models and forms but recently I’ve found out that Ruby already has a good one and its easy to access and use.

All you have to do require uri library and use its constant:

require 'uri'
'user@gmail.com'.match(URI::MailTo::EMAIL_REGEXP).present?

It’s also easy to use on an Active Record format validation:

validates :email, format: { with: URI::MailTo::EMAIL_REGEXP, message: "only allows valid emails" }

It even accepts emails with “+” as gmail enables.

 

How to solve rails 4 UndefinedTable Error when creating namespaced models

In a given rails 4 application, I have two namespaced models with a has_many association between them:

# models/review/asset_type.rb
module Review
  class AssetType < ActiveRecord::Base
    belongs_to :review_asset_category, class_name: Review::AssetCategory
  end
end

# models/review/asset_category.rb
module Review
  class AssetCategory < ActiveRecord::Base
    has_many :review_asset_types, class_name: Review::AssetType,
             foreign_key: 'review_asset_category_id'
  end
end

With migrations as:

class CreateReviewAssetCategories < ActiveRecord::Migration
  def change
    create_table :review_asset_categories do |t|
      t.string :name
      t.timestamps null: false
    end
  end
end

The migrations ran well but everytime I ran the tests I received the error:

Failure/Error: it { is_expected.to respond_to(:name) }
     ActiveRecord::StatementInvalid:
       PG::UndefinedTable: ERROR:  relation "asset_categories" does not exist
       LINE 5:                WHERE a.attrelid = '"asset_categories"'::regc...
                                                 ^
       :               SELECT a.attname, format_type(a.atttypid, a.atttypmod),
                            pg_get_expr(d.adbin, d.adrelid), a.attnotnull, a.atttypid, a.atttypmod
                       FROM pg_attribute a LEFT JOIN pg_attrdef d
                         ON a.attrelid = d.adrelid AND a.attnum = d.adnum
                      WHERE a.attrelid = '"asset_categories"'::regclass
                        AND a.attnum > 0 AND NOT a.attisdropped
                      ORDER BY a.attnum

The solution is simple, just add this class method to each model or to the module that is namespacing the models:

module Review
  def self.table_name_prefix
    'review_'
  end
end

I hope this simple advice will save some time for other developers 🙂

Exercism – Practice your dev skills

A week ago I  was trying to find some algorithm exercises and I found exercism.io, a website that helps you practice your developer skills using your terminal, your favorite IDE and TDD 🙂

Captura de Tela 2015-12-28 às 18.12.33

Besides the TDD approach, what I really liked about it is that it encourages you to solve problems iteratively, one test at a time and when all tests are passing, refactor your code for a better solution.

It also lets you discuss each others solutions and I’ve already received some nice advices. Some exercises also made me learn some new methods that I’ve never used before like reducetr and learn that count can receive a block!

I’m practicing Ruby but they have exercises for a lot of other languages like: PHP, Lua, Lisp, F#, Javascript and Elixir.

I suggest you try it and then let me know if you liked it or not and if you did, comment here your profile so we can try to help you improve your coding skills 🙂

ps: you can also see my answers and help me improve mine

Useful RSpec matchers that you may not be using

Just sharing a couple RSpec matchers that I think are very useful but people doesn’t seem to know they even exist 🙂

all matcher

When I wanted to make sure that every element of an Enumerable, for example, should be an instance of Course, I would write:

it 'populates @catalog_courses only with courses' do
  assigns(:catalog_courses).each do |course|
    expect(course).to be_a(Course)
  end
end

But with the all matcher, its just:

it 'populates @catalog_courses only with courses' do
  expect(assigns(:catalog_courses)).to all( be_a(Course) )
end

I used the be_a matcher inside the all but you could use any other like: be_truthy or eq.

contain_exactly matcher

When I wanted to make sure that an Array should include a and b elements but in any order I would write something like:

it 'populates @courses with courses a, b and c' do
  courses = assigns(:courses)

  expect(courses).to include(a)
  expect(courses).to include(b)
  expect(courses).to include(c)
end

Now, using the contain_exactly matcher:

it 'populates @courses with courses a and b' do
  expect(assigns(:courses)).to contain_exactly(a, b, c)
end

Hope you learned something new here. Leave in the comments any other awesome matcher that you think other people may not be using 😀