Code refactor and reusable code snippets

Hello Team,

Last week I got an opportunity to refactor code for one of our projects. Along with that I got an opportunity to share some code modules that can be used in other projects. It was a good experience. I also found some places in the code where performance of the app could be improved by refactoring the code. Please feel free to correct me if I am wrong at any place. Please do let me know your suggestions.

Refactored code snippets:


 FactoryGirl.define do
factory :attachinary_file do
attachinariable_type { 'Bottle' }
scope { 'images' }
public_id { ('a'..'z').to_a.push(*('0'..'9').to_a).shuffle.first(20).join }
resource_type { 'image' }

In the above code, the public_id field can be refactored and written as:

[*'0'..'9', *'a'..'z', *'A'..'Z'].shuffle.first(20).join

The performance of the second logic is considerably better than the previous one. The benchmarks are as given below:

A] Benchmark.measure{('a'..'z').to_a.push(*('0'..'9').to_a).shuffle.first(16).join}

=> 0.000000 0.000000 0.000000 ( 0.000042)

B] Benchmark.measure{[***'0'..'9', ***'a'..'z', *'A'..'Z'].shuffle.first(16).join}

=> 0.000000 0.000000 0.000000 ( 0.000032)

[***'0'..'9', ***'a'..'z', *'A'..'Z'].shuffle.first(20).join can also be added to a sequence and can be reused in multiple factories.

  1. Giant query loading everything into the memory
bottle_shipment = BottleShipment.all
    bottle_shipment.each do |bs|
     //some code here

Instead of using '.all.each' we should use 'find_each':

BottleShipment.find_each do |bs|
//some code here

'find_each' uses find_in_batches to pull in 1000 records at a time, dramatically lowering the runtime memory requirements.

  1. Old ruby hash syntax should be changed to new hash syntax.

  2. Use pluck() method over map()


column 'Tags', :tags do |m|', ')

The map query runs SQL query as:

SELECT "tags".* FROM "tags"

Benchmark: 0.150000 0.010000 0.160000 ( 0.166678)

This means that we will grab every field on the Tag model and instantiate that object, and then, with .map(&:name) grab the name from each object.

Should be written as:

column 'Tags', :tags do |m|
  m.tags.pluck(:name).join(', ')

The pluck query runs SQL query as:

SELECT "tags"."name" FROM "tags"

Benchmark: 0.010000 0.000000 0.010000 ( 0.006709)

In this query, we only fetch the names, without grabbing all fields.

  1. Limit string to characters where there is no need of having 255 characters.

For eg.

Migration with:

t.string      :phone_number,      null: false, required: true

Can be written as:

t.string :phone_number, null: false, required: true, limit: 10 (As we are definate, phone_number will never be up to 255 characters.)

  1. any? a better way to write .count > 0

For eg.

- if items.count > 0
   = link_to shipment_path(ship_id), method: :delete, remote: true

Can be written as:

- if items.any?
   = link_to shipment_path(ship_id), method: :delete, remote: true
  1. Eg.
    gift_card_code = params[:gift_card][:code]
    gift_card_value = params[:gift_card][:value]
    gift_card = params[:gift_card]

The above can be written as:

gift_card_code, gift_card_value, gift_card = params[:gift_card][:code], params[:gift_card][:value], params[:gift_card]

Some reusable code snippets:

  1. Module for generating 6-digit unique codes using gem 'uuid'.
module GenerateUniqueCode

  def self.included(base)
    base.instance_eval do
      def generate_code
          coupon_code =[0..5].upcase
        end while exists?(code: coupon_code)
  1. Recently, we had a requirement to titleize certain strings in the UI.

The rails in-bulit methods '.capitalize' and '.titleize' had few issues with special characters.

For eg:

A) .titleize()


Will result to:

=> “Whisk(E)Y”

In the above case, 'E' and 'Y' should be of lowecase.

B) .capitalize()

“Wine new/mezcal”

In the above string, 'm' should be capital

To take care of these issues, we wrote a module as follows:

module Titleize
  def self.included(base)
    base.class_eval do
      def name=(s)
        if s.present?
          s = s.titleize
          s = s.gsub(/\((.+?)|\)(.+?)/){ |next_character| next_character.downcase}