Wrong way, Right way - Part 3 - Avoid unnecessary load in conditionals
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?
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?
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.