Thoughts.random

Note: I’ve been using this technique for a while that I’d forgotten I was doing it. Thumbs up to CollectiveIdea for their post which reminded me and inspired the following.

Part of a series about common Ruby mistakes that make things slower or are just plain wrong. Not the style of the code (indentation, methods names etc), but the actual code itself. All examples use Ruby 1.9.2 and Rails 3.0.x.

Part 3 - Avoid unnecessary load in conditionals

The Wrong Way

if post.comments.any? && post.author.present? && post.public?
  puts post.comments.inspect
end

Whats wrong? The conditionals are executed in order. Suppose the post isn’t even public. Before we even get to that check at the end, we’ve hit the database twice; once to check for comments, and another to check for an author. 2 database queries that didn’t need to be run. This may not seem like much, but when you’ve got many of them in your app, you may be causing yourself unnecessary page load times.

The Right Way

if post.public? && post.author_id? && post.comments.any?
  puts post.comments.inspect
end

Whats better? We check if the post is public right away. If it isn’t, we’ve saved ourselves unneeded queries. On top of that, since we don’t use the author inside the block, we don’t need to load them. All the check wanted to know is if they have an author id. Rails provides a nice boolean style method for every field on an ActiveRecord model. So lets use them, in that case, utilizing ‘author_id?’ to check for an author without any additional queries. Then, finally, the comments are loaded, only when the other two, less time consuming checks have passed.

Questions? Comments? Let me know what you thought.

Part of a series about common Ruby mistakes that make things slower or are just plain wrong. Not the style of the code (indentation, methods names etc), but the actual code itself. All examples use Ruby 1.9.2 and Rails 3.0.x.

Part 2 - Work with eager loading, not against it

The Wrong Way

class Post < ActiveRecord::Base
  def last_comment
    comments.order(:position).last
  end
end

Post.includes(:comments).each do |post|
  puts post.last_comment
end

Whats wrong? The last_positioned_comment instance method on Post is adding an order to comment query, which negates the includes(:comments) part of the Post select query. As a result, you’ll get one query for all posts, one query for all comments, and one query for each post to get the last comment. (pattern: 1P + 1C + 1*P)

The Right Way

class Post < ActiveRecord::Base
  def last_comment
    comments.sort_by(&:position).last
  end
end

Post.includes(:comments).each do |post|
  puts post.last_comment
end

Whats better? Because no extra sort is added to the comments, the ones that were preloaded still match. So you end up with only two queries, one for all posts, and one for all comments. (pattern: 1P + 1C). To end up getting the proper sorting, you simply use Ruby’s sort_by method along with symbol to proc syntax to create a nice clean implementation that works with eager loading.

Questions? Comments? Let me know what you thought.

Part of a series about common Ruby mistakes that make things slower or are just plain wrong. Not the style of the code (indentation, methods names etc), but the actual code itself. All examples use Ruby 1.9.2 and Rails 3.0.x.

Part 1 - Optimized conditional data sections

The Wrong Way

@comments = Comment.order(:position)
if @comments.count > 0
  @comments.each do |comment|
    …  
  end
end

Whats wrong? The conditional fires one query for count, and then the loop fires another one to get them again and instantiate them.

The Right Way

@comments = Comment.order(:position)
if @comments.to_a.any?
  @comments.each do |comment|
    …
  end
end

Whats better? Only one query is made. The conditional loads the records, so if there are any to loop over, they are already loaded and ready to go without an extra query.

Questions? Comments? Let me know what you thought.