Программисты делают ошибки в коде. Некоторые просто раздражают (ошибки, не программисты – прим. переводчика), другие могут быть действительно опасными для приложения.
Представляю вам свою подборку из 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
Вместо заключения.
А какие у вас любимые ошибки?
Автор: palkan
