Software Engineer make mistakes. Some of them are just annoying (for others to read) and some are really dangerous. Here is my selection of 10 mistakes done by Ruby / Ruby on Rails developers. These tips are easy to follow and can save you much time of later debugging.
Double Negative and Complex Conditionals
1 2 3 4 5 6 7 8 9 10 11
Double negative is hard to read. Every time I encounter it, I spend a couple of seconds on parsing the condition. Use the API which Rails gives you
user.present? instead of
I also rarely see any usage for unless, especially with complex conditionals connected by && and ||. How fast can you decide when unless
user.active? || address.confirmed? will fire?
Using Save Instead of save! and Not Checking Return Value
1 2 3
What is wrong with this piece of code? It will fail silently when user cannot be saved. There will be no single trace of this failure in your logs and you will spend time wondering: “why there are no users in the database”. If you expect that data is valid and model should be always saved successfully, then use bang versions –
create! and so on. Use save only when you handle the return value:
1 2 3 4 5
self When It’s Not Needed
1 2 3 4 5 6 7 8
In this case writing self.first_name is completely unnecessary, because
first_name will do. This is of course just matter of style and has no other negative consequences than overly verbose code. Please mind that you need
self in assignments:
self.first_name = "John".
N + 1 Queries
This is a vast topic, but I will try to give the simplest example. You want to display a list of posts with names of authors. Post model
belongs_to :user. If you just do
Post.limit(10) and then call
post.user.name in your views, you will make a separate database query for each user. That’s because Rails has no single chance to guess that you need users when you make the first query in the controller.
It’s easy to spot N + 1 queries problem just by looking at server’s logs:
1 2 3 4 5 6 7 8 9 10 11 12 13
You have to be explicit at telling what you need from the database. In the easy cases Rails includes method will do. You can read more about it in Rails guides - Eager Loading
Boolean Field with Three Possible Values
Boolean is supposed to have two possible values –
false, right? And how about
nil? If you do not specify default value and null: false in your migrations, you end up with boolean field with three possible values –
nil. This leads to nasty code like:
1 2 3 4 5 6 7 8 9 10 11 12 13 14
If you need three possible states – use string field with three
Orphaned Records After Destroy
When you destroy a model and it is required by associated records, you should handle it. It’s easy to find such cases:
1 2 3 4
User is required for post. Hence, we have to write:
1 2 3
puts in the code after some debugging session pollutes server logs and output of tests. Use
Rails.logger.debug so it’s later possible to adjust the desired log level.
Not Using map
I’ve seen such code many times:
1 2 3 4 5
This is exactly the case for using
map, which is shorter and more idiomatic:
1 2 3
Not Using Hash#fetch
What’s wrong with this code? It will throw
NoMethodError: undefined method `' for nil:NilClass if there is no user key in the hash. If you expect the key to always be present, use
This will give you a meaningful
exception – KeyError: key not found: :user.
Using Code from app/ in Migrations
Let’s say you have the following model:
1 2 3
and you want to add points field to it. So you create a migration. But you would also like to handle existing users: 10 points for active and 0 for the rest. You add to your migration:
It works and you are happy. Time passes by and you decide to remove
User::ACTIVE constant. Your migrations are now broken, you cannot run them from scratch, because
User::ACTIVE is undefined.
Never use code from app/ directory in migrations. If you need to update existing data and do it in a few environments (development, staging, production) create a Rake task and delete it once it’s executed in every environment.
So far so good, That’s it!!! See ya!!! :)