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:
1)Eg:
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' } end end
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.
- Giant query loading everything into the memory
bottle_shipment = BottleShipment.all bottle_shipment.each do |bs| //some code here end
Instead of using '.all.each' we should use 'find_each':
BottleShipment.find_each do |bs| //some code here end
'find_each' uses find_in_batches to pull in 1000 records at a time, dramatically lowering the runtime memory requirements.
-
Old ruby hash syntax should be changed to new hash syntax.
-
Use pluck() method over map()
example:
column 'Tags', :tags do |m| m.tags.map(&:name).join(', ') end
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(', ') end
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.
- 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.)
- 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
- 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:
- Module for generating 6-digit unique codes using gem 'uuid'.
module GenerateUniqueCode def self.included(base) base.instance_eval do def generate_code begin coupon_code = UUID.new.generate(:compact)[0..5].upcase end while exists?(code: coupon_code) coupon_code end end end end
- 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()
“whisk(e)y”.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} end super(s) end end end end
Thanks.