atrk.gif
    • Quick Contact: 1 212.714.6135

Common mistakes in RoR

Database column Indexing
At the beginning of programming , mostly during writing migrations, we forget to add indexes , which may or may not have any effect in initial stages of application, but will have drastic delay in fetching records from the database as the table size increases.

Using delete_all instead of destroy_all
ActiveRecord::Base::delete_all or ActiveRecord::Base#delete method deletes the record(s) from the database without firing any callback. So, This method must be used sensibly.
Where as ActiveRecord::Base::destroy_all or ActiveRecord::Base#destroy executes before_destroy and after_destroy callbacks.

ActiveRecord transaction for saving multiple object
Whenever saving multiple objects in a single request, always use ActiveRecord::Base::transaction method to support atomicity. So, if any record has not saved the entire transaction, will get rolled back and nothing will be saved into the database.
Always add indexes to the column being used for searching records.
Example:

ActiveRecord::Base.transaction do
  @tasks.each do |task|
    task.save!
  end
end


Set the primary key explicitly for database views
Sometimes we need to make use of database views for complex data joined from different tables, to reduce the overheads of fetching data in a single query and also to support the searching and sorting based on the columns of joined tables.

Always set the primary key explicitly in the respective model, this will provide the flexibility to define association with other models.

Example:

Class UserDetailsView < ActiveRecord::Base
  self.table_name = :user_details # user_details is a view in database
  self.primary_key = :id # view_contains id from the joind user table and can be used as primary key for this model
end


Using IN sub-queries
Relational Database supports different types of sub-queries. Unfortunately, one of them is a performance disaster.

IN and NOT IN sub-queries.
Static array in these queries are fine, but when used sub-query inside, it reduces the performance and also there are limitation on the number of elements in these queries depending on different databases.

Do Not use queries like IN or NOT IN

These queries must be written using EXISTS or NOT EXISTS predicates to avoid serious performance issues.

Note: Oracle optimizes IN fairly well same as EXITS, So both can be used. But not the same case with NOT IN vs NOT EXISTS. Better to use NOT EXISTS even on Oracle instead of NOT IN predicate.

Storing Session in database
It is very easy to store session data in relational databases, but it is the most expensive thing.
Session data are stored and retrieved more frequently and can create scenario when the database can go unresponsive for the more important work to be done.
If there is a need of database for string session data, then use ‘Memcache’ or ‘Redis’.

Make use of Ranges instead of comparisons for numbers
No more if x > 1000 && x < 2000 nonsense. Instead:

  year = 1972
  case year
    when 1970..1979: "Seventies"
    when 1980..1989: "Eighties"
    when 1990..1999: "Nineties"
  end


Use find_each
Do not use each method for iterating large set of records(ActiveRecord::Relation). It will fetch all the record in a single query from the database and will dump into the memory, causing
performance degradation if the there is not enough memory. So, better make use of find_each method, this method not only etches record in batches from the database but yields those once at a time too. Once the current batch is processed another batch will be fetched from the database. So no need to have all the records at once in the memory.
Example:

Book.where(:published => true).find_each do |book|
  puts "Do something here!"
end


Use pluck instead of map
We often need to find a single column from the database, So many of us make use of map method on ActiveRecord::Relation.
Like:

User.active.select(:name).map(&:name)

Or, even worse:

User.active.map(&:name)

Instead use pluck method:
Example:

User.active.pluck(:name)


Rescue to the rescue
Use rescue for one liner statements which may throw error instead of condition:

hsh = {:age => 50}
hsh[:name].downcase #=> Error
hsh[:name].try(:downcase) #=> nil

Or, even worse:

hsh[:name] ? hsh[:name].downcase : nil #=> nil

Instead use as below:

hsh[:name].downcase rescue nil #=> nil

OR

hsh[:name].to_s.downcase #=> ''


Allow both single item and and array to make use of the same loop without condition
I have observed that few folks write something like this

(items.is_a?(Array) ? Items : [items]).each do |item|
  #------------------
end

Instead use following:

[*items].each do |item|
  #------------------
end

Above code will take care of items containing either a single object or array.

Try to reduce redundancy using Ruby’s logic
method for checking odd number:

def odd?(x)
  x % 2 == 0 ? false : true
end

Instead make use of Ruby logics and simply it somewhat more.

def odd?(x)
  x % 2 != 0
end


SQL Injection

Never interpolate the input from the outside world into ActiveRecord query directly.

query = "SELECT * FROM users WHERE name = '#{name}' AND password = '#{password'} LIMIT 1"
results = User.connection.execute(query)

Above query will behave unexpected whenever a sql injection string is passed and name nad password:
Example:

name: ' OR 'a' = 'a
password: ' OR 'a' = 'a

above input will generate query something like

SELECT * FROM users WHERE name = '' OR 'a' = 'a' AND password = '' OR 'a' = 'a' LIMIT 1

And above query will always return a user. Instead make use of ‘where’ or ‘find_by’ methods these will automatically escape the commas(‘/”) in the input value and will not allow query to return true and will check the record in database exactly what it had been passed as name and password.

ActiveRecord::Base::exists
Sometime we just need to check for existence of any record in the database. And I have seen coders are writing something like:
To check whether published books are present in the database or not:

books = Book.where(:published => true)
books.count > 0 # will return true if published books exists in datatbase.

Instead use ActiveRecord::Base::exists method as:

Book.exists?(:published => true)


N+1 queries
So, Basically it happens when we load a bunch of objects from the database, and then for each one of those objects we make 1 or more database query.
Example:

class Customer  { where(active: true) }
end
class Address < ActiveRecor...
  belongs_to :customer
end
@customers = Customer.active
   # this is fine the customer object is already in memory
   # this one fires one query to the database for finding Address record for each custome

Suppose we have 100 active customers then total database queries will be 101.
1 query for finding all the active customers and other 100 queries for finding an address for each.
Instead try eager_loading and just fire 2 queries for all.

@customers = Customer.active.includes(:address)

The above query will fire 1 query for finding all the customers and other query for finding all the address associated to the clients fetched in the first query.

LinkedInShare
This entry was posted in My Voice. Bookmark the permalink.

5 Responses to Common mistakes in RoR

  1. Pingback: Common mistakes in RoR | Allerin | Scoop.it

  2. gwhosubex gwhosubex says:

    “Use pluck instead of map”

    Why?

    • Nicolas Mosconi Nicolas Mosconi says:

      Because pluck will get only that field from the db as opposed to map which will get all the data for all the records and then will iterate over each object collecting the required field

  3. kris kris says:

    I prefer Array(items) over [*items], as it’s much more readable.

  4. Ben Ben says:

    @gwhosubex
    Because you aren’t loading each record.
    http://apidock.com/rails/ActiveRecord/Calculations/pluck

Leave a Reply

Your email address will not be published. Required fields are marked *

You may use these HTML tags and attributes: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>