http://habrahabr.ru/post/250007/

Программисты делают ошибки в коде. Некоторые просто раздражают (ошибки, не программисты – прим. переводчика), другие могут быть действительно опасными для приложения.
Представляю вам свою подборку из 10 распространенных ошибок разработчиков Ruby/RoR и советов о том, как их избегать. Надеюсь, они помогут сократить вам время отладки кода.

1. Двойное отрицание и сложные условия.

if !user.nil?
  # ...
end

unless user.blank?
  # ...
end

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

Двойные отрицания сложны для чтения. Каждый раз я трачу пару секунд для того, чтобы распарсить условие. Используйте Rails API и пишите user.present? вместо !user.blank?.

Я также редко считаю применение unless оправданным, особенно с множественными условиями. Как быстро вы поймете, в каком случае данное условие будет выполнено?

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

2. Использование save вместо save! без проверки возвращаемого значения.

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

В чем проблема данного кода? Вы не узнаете об ошибке сохранения, если она произойдет. Ни единого следа не останется в ваших логах, и вы потратите уйму времени, пытаясь разобраться: «Почему же в базе данных нет пользоветелей?».
Если вы уверены, что данные всегда корректны и запись должна сохраняться – используйте bang методы (save!, create! и т.д.). Используйте save, только если вы проверяете возвращаемое значение:

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

3. Использование self без необходимости.

class User
  attr_accessor :first_name
  attr_accessor :last_name

  def display_name
    "#{self.first_name} #{self.last_name}"
  end
end

В случае self.first_name достаточно написать first_name, и результат не изменится. Но это скорей вопрос стиля, и никаких негативных последствий за собой не несет. Однако помните, что self необходимо использовать при присваивании значений.

self.first_name = "John"

4. Проблема N+1 запроса.

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

Если вы посмотрите в логи при выполнении данного кода, то увидите, что выполняется 11 запросов к базе данных: один на выборку постов и по одному на каждого пользователя. Рельсы не в состоянии предугадать ваши желания и сделать запрос на выгрузку сразу 10 пользователей (возможно, даже вместе с постами). Для того, чтобы направить Рельсы в нужную сторону, можно использовать метод includes (подробней здесь).

5. Булево поле с тремя значениями.

Предполагается, что булево поле может иметь только два значения – true или false, не так ли? А как насчет nil? Если вы забыли указать значение по умолчанию и добавить опцию null: false в вашей миграции, вам придется иметь дело с тремя булевыми значениями – true, false and nil. В результате имеем подобный код:

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

# пост опубликован
if post.published
  # ...
end

# пост неопубликован
unless post.published
  # ...
end

Если вам нужно обрабатывать три различных значения – используйте текстовое поле.

6. Потерянные записи после удаления.

Рассмотрим простой пример:

class User < ActiveRecord::Base
  has_many :posts
end

class Post < ActiveRecord::Base
  belongs_to :user
  validates_presence_of :user
end

Если мы удалим пользователя, то связанные с ним посты перестанут быть корректными (что, скорей всего, приведет к ошибкам в приложении). В этом случае необходимо обрабатывать удаление связанных объектов:

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

7. Использование кода приложения в миграциях.

Предположим, что у вас есть следующая модель:

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

и вы хотите добавить в нее поле points (очки). Для этого вы создаете миграцию. Вы также хотите установить значение поля points для существующих пользователей равным 10. Для этого вы пишите в миграции:

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

Запускаете миграцию, и — Ура! — все сработало: существующие пользователи имеют по 10 очков. Время идет, код меняется, и вот вы решаете удалить константу User::ACTIVE. С этого момента ваши миграции не работают, вы не можете запустить миграции с нуля (вы получите ошибку uninitialized constant User::ACTIVE).

Совет: не используйте код приложения внутри миграций. Используйте, например, временные Rake задачи для изменения имеющихся данных.

Примечание переводчика:
На мой взгляд, гораздо удобнее использовать временные миграции для изменения значений. Особенно при выкатке изменений на production: вероятность забыть запустить rake отнюдь не равна нулю, а миграции за вас запустит Capistrano.
А чистить временные миграции можно после релиза.

8. Вездесущий puts.

Оставшийся после отладки puts засоряет логи и вывод во время прогона тестов. Используйте Rails.logger.debug, который позволит фильтровать логи в зависимости от выбранного уровня.

Примечение переводчика:

А лучше научитесь пользоваться дебаггером. При этом не забывайте удалять binding.pry перед коммитом.

9. Забываем про map.

Я часто вижу подобный код:

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

Это как раз тот случай, когда нужно использовать map: лаконично и идиоматично.

users = posts.map do |post|
  post.user
end

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

10. Не используем Hash#fetch.

name = params[:user][:name]

Что здесь не так? Данный код вызовет исключение (NoMethodError: undefined method `[]' for nil:NilClass), если в хэше не будет ключа user. Если вы рассчитываете на то, что ключ должен быть хэше, используйте Hash#fetch:

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

Тогда вы получите более осмысленное исключение:

This will give you a meaningful exception – KeyError: key not found: :user

Вместо заключения.

А какие у вас любимые ошибки?