Ruby on Rails - Самые распространенные ошибки новичка. Часть 1 - Модель и база данных
Всем привет. В компании, в которой я работаю, я также являюсь тренером по Ruby on Rails. На момент написания статьи я провел уже 3 внутренних курса и мне есть чем поделиться. На основе этого опыта я разработал онлайн курс - rubyboost.ru Моей целью было создание программы, которая подготовит качественного сотрудника на достаточно высоком уровне за 2 месяца. Честно сказать, первый курс не дал того, что я хотел. Мы просто получили стажеров, которые были знакомы с нашим основным стеком. Разрабатывая первую программу, я ставил себе очень простую задачу - "Мои выпускники должны уметь сдавать задачу за 40 комментариев" У нас в компании качество кода является жестким требованием. В среднем все новички сдают задачу за 50 - 100 комментариев (кстати многие не выдерживают такого и просто уходят). Моей целью было снизить это количество до 40. К сожалению, первый курс с этой задачей не справился, но оно и понятно, я просто рассказывал, как я пишу код, без явного акцента на многие важные детали.
После первого курса я проанализировал все комментарии, которые оставлял к коду студентов и к коду наших джуниоров и был дико удивлен. Более 50 одинаковых ошибок совершают все новички. Я каждому оставлял одинаковые комментарии снова и снова. Уже во втором потоке нашего курса, я, во-первых, заставил всех использовать rubocop. Во-вторых - подготовил, и оформил список всех этих ошибок. Студенты второго потока сдают задачи за 15-30 комментариев, что в 3 раза превышает изначальный показатель. Списком всех этих ошибок хочу поделиться с Вами.
Немного об оформлении. Все ошибки разделены на 4ре категории. Кроме того, есть Особо важные ошибки выделенные красным цветом. Эти ошибки могут критично сказаться на работе проекта и команды.
Перед началом. хотел бы еще упомянуть о том, что одни и те же задачи нужно решать одинаково. У Вас лично, или в команде, должен быть style guide, который охватывает все ключевые моменты оформления и организации кода. Одинаковый код позволит легче взаимодействовать команде и допускать меньше ошибок.
- Модели и база данных (11 ошибок всего, из них 4 важных)
- Контроллеры (6 ошибок всего, из них 3 важных)
- The Ruby Way и качество кода (15 ошибок всего, из них 5 важных)
- Общие ошибки без категории (8 ошибок всего, из них 4 важные)
Первая часть посвящена моделям. Этот слой содержит в себе всю бизнес логику, тут же и самое большое количество ошибок в ее реализации.
Итак, новички:
Не используют автоматически генерируемые методы.
Обычно Rails и многие гемы добавляют много полезных вспомогательных методов в объекты, с которыми работают. Нужно знать эти особенности и использовать. Например, для boolean полей rails автоматически добавляет предикаты. Это методы, название которых заканчивается на знаком вопроса. Использовать их считается правилом хорошего тона.
################################
## Это плохо! ##
################################
if course.visible
# do something
end
################################
## А вот это хорошо ##
################################
if course.visible?
# do something
end
Не понимают, откуда берется N +1 зарос
Понимание того, как ORM работает с базой очень важно, но не всегда есть хотя бы даже на базовом уровне. Поэтому, почти не используются методы includes, preload,eager_load. Не используется гем bullet.
В первом случае выполнится N+1 запрос к базе данных, где - N - количество сданных домашних заданий. Это может быть и 10, и 20 и даже 100 запросов. Во втором коде будет всего 2 запроса!
################################
## Это плохо! ##
################################
@homeworks = lesson.homeworks
# В представлении есть запросы к user
- @homeworks.each do |homework|
%p homework.user.email
################################
## А вот это хорошо ##
################################
@homeworks = lesson.homeworks.includes(:user)
# В представлении есть запросы к user
- @homeworks.each do |homework|
%p homework.user.email
Не используют скоупы
Скоупы позволяют скрыть имплементацию базы данных и уникализировать код. Кроме того их использование делают код намного читаемей. Потому что они показывают намерения разработчика, а не структуру базы.
################################
## Это плохо! ##
################################
def index
@lessons = Сourse.lessons.order(position: :asc)
end
################################
## А вот это хорошо ##
################################
class Lesson < ActiveRecord::Base
belongs_to :course
scope :by_position, -> { order(position: :asc) }
end
def index
@lessons = course.lessons.by_position
end
Не знают разницу между size и count у ActiveRecord::Relation
Разница в том, что count - всегда сделает запрос, size - проверит, а не посчитано ли нужное значение в каком-нибудь поле. Подробнее - смотреть screencast про CounterCache
Мюсли обычного новичка
Я всегда пишу count просто потому, что мне привычно это слово из других языков и оно мне больше нравится.
И мюсли того, кто читал rails guide
Я всегда пишу size потому что знаю, что в случае необходимости добавления кеша, мне нужно будет просто обновить пару строк, не выискивая count по всему проекту
Не знают разницу между after_create и after_commit
Данные модели, в том числе ее новый ID внутри after_create доступен, а извне - еще нет. Потому что транзакция еще не завершена.
Если после создания записи в базе данных, я хочу положить её ID в redis или любое другое хранилище. то:
В случае с after_create это может привести к невалидным данным, если ID будет использован до того, как транзакция дошла до конца.
В случае использования Sidekiq или любой другой BackgroundJob я всегда буду использовать after_commit, что гарантирует целостность моих данных.
Всегда используют ORM
Хотя работа с объектами и является самой удобной, она достаточно медленна и требовательна по памяти. Не всегда есть понимание, какой код как работает, и как его лучше оптимизировать
################################
## Это плохо! ##
################################
Article.all.each { |article| article.delete }
Article.all.map { |article| article.title }
Course.all.select { |course| course.created_at < 5.years.ago }.each { |course| course.articles.delete_all }
################################
## А вот это хорошо ##
################################
Article.delete_all
Article.pluck(:title)
old_courses_ids = Course.where(‘created_at < ?’, 5.years.ago’).pluck(:id)
Article.where(course_id: old_courses_ids).delete_all
Не знают разницу между dependent destroy vs delete_all
dependent destroy перед удалением выберет вся связанные записи, построит их объекты и на каждом так же вызовет метод destroy. Такой подход удалит все связанные данные. Престает работать на больших объемах данных
dependent delete_all выполнит удаление через SQL запрос, это быстро, но придется самому заботиться о целостности базы
Не используют методы с бэнгом
Согласно соглашений, в имя метода добавляется bang (!) в 2х случаях: если метод изменяет объект, на котором он вызывается или если метод выбрасывает исключение, в случае неудачного выполнения. О второй особенности часто забывают. Если в коде что-то не так, об этом нужно узнать как можно раньше. Т.е, если вы никак не обрабатываете результат сохранения записи в базу данных, то Вам лучше выбросить исключение, чтобы узнать о том, что в конкретном участке кода могут обрабатываться невалидные данные.
В этом случае если во входных данных будет невалидная статья, она просто проигнорируется.
################################
## Это плохо! ##
################################
class Article
validates :body, length: { minimum: 200 }
end
articles_data.each do |article_data|
Article.create(article_data)
end
################################
## А вот это хорошо ##
################################
# Возможны 2 решения
articles_data.each do |article_data|
Article.create!(article_data)
end
# В этом случае программист узнает когда на вход попадут данные, которые он не ожидал
articles_data.each do |article_data|
article = Article.new(article_data)
unless article.save
puts ‘Can not save article’
#process this situation
end
end
# Дайте пользователю возможность решить, что делать.
В миграциях не задают дефолтные поля
Если у модели у поля должно быть значение по умолчанию, его нужно установить через базу данных.
################################
## Это плохо! ##
################################
class Article
after_initialize :set_default_status
def set_default_status
self.status = ‘pending’
end
end
################################
## А вот это хорошо ##
################################
class MyMigration
def up
change_column :articles, status, :string, default: ‘pending’
end
def down
change_column :articles, status, :string
end
end
В миграциях не задают constraints
Чем больше ограничений мы наложим на базу, тем надежнее будет наше приложение. Не забываем про null: false. Профиль, ни при каких условиях, не может существовать без пользователя.
################################
## Это плохо! ##
################################
class MyMigration
def change
add_column :profiles, user_id, :integer
end
end
################################
## А вот это хорошо ##
################################
class MyMigration
def change
add_column :profiles, user_id, :integer, null: false
end
end
В миграциях не пишут обратную миграцию
Миграция должна позволять себя откатить, иначе, какой в ней смысл.
################################
## Это плохо! ##
################################
class MyMigration
def up
change_column :articles, status, :string, default: ‘pending’
end
end
################################
## А вот это хорошо ##
################################
class MyMigration
def up
change_column :articles, status, :string, default: ‘pending’
end
def down
change_column :articles, status, :string
end
end
С моделями пока что все, если есть что добавить - присылайте в комментариях.
Продолжение следует!
Английскую версию этой статьи можно почитать в нашем корпоративном блоге тут
Комментарии