10 errors in Ruby / Ruby on Rails that are easy to fix

Original author: Adam Niedzielski
  • Transfer
Programmers make mistakes in the code. Some are just annoying (errors, not programmers - approx. Translator), others can be really dangerous for the application.
I present to you my selection of 10 common mistakes of Ruby / RoR developers and tips on how to avoid them. Hope they help shorten your code debugging time.

1. Double negation and difficult conditions.


if !user.nil?
  # ...
end
unless user.blank?
  # ...
end
unless user.active? || address.confirmed?
  # ...
end

Double negatives are hard to read. Each time I spend a couple of seconds to parse the condition. Use the Rails API and write user.present? instead of! user.blank ?.

I also rarely find unless unless justified, especially with multiple conditions. How quickly do you realize in which case this condition will be met?

unless user.active? || address.confirmed?
...
end


2. Using save instead of save! without checking the return value.


user = User.new
user.name = "John"
user.save

What is the problem with this code? You will not be aware of a save error if it occurs. Not a single trace will be left in your logs, and you will spend a lot of time trying to figure out: "Why are there no users in the database?"
If you are sure that the data is always correct and the record should be saved, use the bang methods (save !, create! Etc.). Use save only if you check the return value:

if user.save
...
else
...
end


3. Using self without need.


class User
  attr_accessor :first_name
  attr_accessor :last_name
  def display_name
    "#{self.first_name} #{self.last_name}"
  end
end

In the case of self.first_name, just write first_name and the result will not change. But this is rather a matter of style, and does not carry any negative consequences. However, remember that self must be used when assigning values.

self.first_name = "John"


4. Problem of N + 1 request.


posts = Post.limit(10)
posts.each do |post|
   do_smth post.user
end

If you look at the logs when executing this code, you will see that 11 database queries are being executed: one for a selection of posts and one for each user. Rails are not able to predict your desires and make a request for unloading 10 users at once (possibly even with posts). In order to direct the Rails in the right direction, you can use the includes method (more details here ).

5. A boolean field with three values.


It is assumed that a Boolean field can have only two values ​​- true or false, right? What about nil ? If you forgot to specify the default value and add the null: false option in your migration, you will have to deal with three boolean values ​​- true, false and nil. As a result, we have a similar code:

# пост новый, не опубликован и не неопубликован
if post.published.nil?
  # ...
end
# пост опубликован
if post.published
  # ...
end
# пост неопубликован
unless post.published
  # ...
end

If you need to handle three different values, use a text box.

6. Lost records after deletion.


Consider a simple example:

class User < ActiveRecord::Base
  has_many :posts
end
class Post < ActiveRecord::Base
  belongs_to :user
  validates_presence_of :user
end

If we delete the user, then the posts associated with it will cease to be correct (which, most likely, will lead to errors in the application). In this case, you need to handle the removal of related objects:

class User < ActiveRecord::Base
  has_many :posts, dependent: :delete
end


7. Using application code in migrations.


Suppose you have the following model:

class User < ActiveRecord::Base
  ACTIVE = "after_registration"
end

and you want to add a points field to it. To do this, you create a migration. You also want to set the value of the points field for existing users to 10. For this, you write in the migration:

User.where(status: User::ACTIVE).update_all(points: 10)

Start the migration, and - Hurray! - it worked: existing users have 10 points. Time goes by, the code changes, and now you decide to remove the User :: ACTIVE constant. From this point on, your migrations do not work, you cannot start migrations from scratch (you will get the error uninitialized constant User :: ACTIVE ).

Tip: do not use application code inside migrations. Use, for example, temporary Rake tasks to modify existing data.

Translator's note:
In my opinion, it is much more convenient to use temporary migrations to change values. Especially when rolling out changes to production: the probability of forgetting to launch rake is by no means zero, and Capistrano will launch the migration for you.
And you can clean temporary migrations after release.

8. The omnipresent puts .


Remaining after debugging puts clogs the logs and output during the test run. Use Rails.logger.debug, which allows you to filter logs depending on the selected level.

Note translator:

And better learn how to use the debugger. In this case, do not forget to remove binding.pry before committing.

9. Forget about map .


I often see similar code:

users = []
posts.each do |post|
  users << post.user
end

This is exactly the case when you need to use map : succinctly and idiomatically.

users = posts.map do |post|
  post.user
end
# а я бы написал так (прим. переводчика)
users = posts.map(&:user)


10. Do not use Hash # fetch .


name = params[:user][:name]

What is wrong here? This code will throw an exception ( NoMethodError: undefined method `[] 'for nil: NilClass ) if there is no user key in the hash. If you expect the key to be a hash, use Hash # fetch:

name = params.fetch(:user)[:name]

Then you will get a more meaningful exception:

KeyError: key not found: :user


Instead of a conclusion.


What are your favorite mistakes?

Also popular now: