10 ошибок в Ruby / Ruby on Rails, которые легко исправить

[Wadik]

Ошибками это назвать тяжело. Скорее тонкости использования языка и фреймворка, а их, в зависимости от масштаба проекта, ну очень много может возникнуть…

[voff]

Я, в общем, далек от Ruby, но предложенный автором вариант в пункте 2, кажется тоже не слишком хорошим. Разве в Ruby нет механизма исключений?

[Helsus]

Вообще, такой вариант считается хорошей практикой, даже scaffold вроде так генерит. Это же не исключение, что пользователь, например, ввёл некорректный email. Надо просто отрендерить опять форму, но уже с показанными ошибками.

[ef_end_y]

Почему не исключение? Если мы уже пишем, то некорректные данные должны вызывать исключение. Мы должны были их проверить до записи

[printercu]

save обычно используется вместе с if, и по else идет рендер страницы с ошибками:

if user.save
  redirect_to user_path(user)
else
  render 'edit'
end

# или так:
user.save
if user.errors.any?
# ...

save! в этом случае покажет пользователю страницу ошибки.

Лучше этот совет изменить, я думаю: если вы не обрабатываете ошибку (вы уверены на момент написания кода, что ошибок не может быть), то напишите save!, а если обрабатываете, то save.

[mkuzmin]

Есть корректный способ в миграциях менять данные:

#псевдокод
class Migration < BaseMigration
  class User < AR::Base
  end

  def up
    User.update_all smth: 123
  end

  def down
  end
end

Просто объявлем минимальные модели для миграции. Если нужны сложные проеобразования, то лучше написать 1 sql запрос и не использовать activerecord.

Даже если в коде проекта есть модель пользователя, в миграции будет использоваться модель из неймспейса миграции.

[Nike0]

По первому пункту могу посоветовать ruby style guide (https://github.com/bbatsov/ruby-style-guide). Там описаны конкретно эти примеры (двойное отрицание, множественные условия в unless и т.д.).
И вообще, на самом деле этот гайд учит писать код не так, чтобы он работал, а чтобы его понимали другие люди, которые потом будут его читать.
А также позволяет уменьшить количество использование лишних блоков кода.

[DsideSPb]

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

Сомнительный совет, когда есть enum (Rails 4.1+).

[palkan]

«Поддержка» enum в Рельсах тоже сомнительна.

[DsideSPb]

Приведённые там доводы сомнительны.

Автор утверждает, что enum надо использовать для значений, у которых хорошо выражена численная природа. Но позвольте, что тогда не так с обычными числами?

Автор утверждает, что если он надумает дать дамп данных аналитику, то аналитик не поймёт, что там за чиселки встречаются, если нет явного словаря, который только усложнит работу. Это верно. Зато числа компактнее в памяти, быстрее обрабатываются. Выбирайте, что для вас важнее и насколько.

Автор утверждает, что сравнение состояний между собой приводит к странным результатам. Ну, не надо так делать, если это плохо работает. Используйте числа. То, что enum это числа — деталь реализации (см. конец первого абзаца), обязательной чертой перечисляемого типа является только отношение эквивалентности, но не порядка. Просто компьютеры привыкли к числам и могут их обрабатывать быстрее всего.

[alex_bel]

Я использую self для пояснения:
self.foo — атрибут
foo — метод

[shock_one]

Атрибут — это тоже метод. Attribute reader. Getter.

[alex_bel]

Это для рельсовых моделей. Когда атрибут (self.something) — это поле в базе.

[tom]

Атрибут в руби по сути это переменная экземпляра класса (instance variable), имя которой всегда начинается с символа @. Получить доступ к атрибуту за пределами класса напрямую нельзя. Для чтения атрибута используется reader-метод, а для установки значения атрибута используется writer-метод. Либо в этих же целях можно при необходимости использовать методы instance_variable_get и instance_variable_set.

Для удобства определены методы модуля attr_reader и attr_writer, которые создают методы для чтения и записи указанных атрибутов. Также существует метод attr_accessor, который создает и reader-, и writer-метод.

По соглашению имя геттера и сеттера (ридера и врайтера) атрибута "@attribute" будут называться attribute и attribute= соответственно.

Исходя из этого имеем:

self.foo — вызов метода, который может являться геттером атрибута @foo
foo — вызов метода, который может являться геттером атрибута @foo
@foo — чтение атрибута (переменной экземпляра)

В статье утверждается, что self.foo необходимо использовать при присвоении. Это так, потому что foo = 5 это создание и присвоение локальной переменной. А self.foo = 5 это вызов writer-метода foo=(5).

[Toy]

А я люблю unless. Это мой самый любимый момент в Ruby. Я был в восторге от такой возможности, когда только изучал язык.

[matiouchkine]

Это же очень простая дилемма: выступая в роли писателя все любят `unless` и вырвиглазные регулярки. Как только мы превращаемся в читателей, копаясь в чужом коде, сразу хочется уканделябрить человека, использующего кашу из логических операций, со скобками, да еще и с неявным семантическим логическим отрицанием.

[Envek]

Регулярки можно писать и не вырвиглазными, с комментами и выравниванием, как например тут: github.com/arnau/ISO8601/blob/master/lib/iso8601/duration.rb#L193-L208, но это надо хотеть писать читаемый код :-)
А unless хорош, когда после него только одно выражение (без && или ||)

[matiouchkine]

А и я не говорил, что регулярки — это плохо :)

Насчет `unless` я придерживаюсь мнения, что короткая форма — добро, а длинная — зло, которое всегда говорит о плохой структуре / именовании (переменная userIsEmpty вместо hasUser, например).

[shock_one]

Private не изменяет видимость методов класса. Многие считают, что метод bar в этом примере будет приватным.

class Foo
  private

  def self.bar
    puts 'bar'
  end
end

Foo.bar # => bar

Если вам нужен приватный метод класса, используйте privat_class_method или singleton class (class << self).

[shock_one]

Часто возникает ситуация, когда в методе нужно создать объект, как-то его изменить и возвратить. Очевидной имплементация будет

def agent
  agent = Mechanize.new
  agent.user_agent = 'Mozilla'
  agent
end

С этим кодом все хорошо, но он не очень элегантный. Используя Object#tap можно превратить его в

def agent
  Mechanize.new.tap { |agent| agent.user_agent = 'Mozilla' }
end

[valyukov]

tap хорош, но лучше в таких случаях использовать:

yeild self if block_given?

внутри метода initialize инстанцируемого объекта.

тогда код будет чуточку лаконичней:

def agent
  Mechanize.new { |agent| agent.user_agent = 'Mozilla' }
end

Кстати, автор библиотеки используемой в примере такую возможность дал:
github.com/sparklemotion/mechanize/blob/master/lib/mechanize.rb#L217

[duke_pro]

Недавно видел куча кода типа

Model.all.present?

не надо так

Model.exists?

[fr33z3]

Почему-то ничего не было написано про классический User.find, кидающий исключение в случае, если не найдено ни одного юзера и замену ему User.find_by_id

[stAndrei]

Такая же ситуация, как с методом save! Если не обрабатывается find_by_id, то лучше find использовать.

[fr33z3]

Да, я как бы в курсе если что… Просто это наиболее часто встречающаяся ошибка среди начинающих рельсовиков — полагать, что find вернеть nil, что update_attributes просто обновляет аттрибуты без вызова сохранения. И что значит, «если не обрабатывается find_by_id»? Каждому оператору свое предназанчение, мне например иногда важнее вызвать find, который в случае чего вызовет у меня exception и кинет мне в багтрекер ошибку. При этом это помогает мне сделать архитектура приложения, которая сама по себе не падает ну или дальнейшее выполнение которой мне уж точно не нужно. Например background таски какие-нибудь. В рельсах же в контроллерах такое лучше не делать по причине того, что нам все-таки по возможности нужно показать либо мессадж с ошибкой пользователю, либо редиректнуть его, либо еще чего похлеще сделать.

[Envek]

В рельсах начиная с 4.0 рекомендуют использовать не find_by_id(value), а find_by(id: value), лучше читается, когда полей для поиска несколько (сравните find_by_name_and_surname(v1, v2) и find_by(name: v1, surname: v2)).

[printercu]

github.com/rails/activerecord-deprecated_finders
> Note that find(primary_key), find_by..., and find_by...! are not deprecated.

[int03e]

Но работает это медленнее, потому что find_by_xxx это все через method_missing.

[printercu]

Первый раз только, потом уже метод будет доопределен. github.com/rails/rails/blob/master/activerecord/lib/active_record/dynamic_matchers.rb#L18

[palkan]

Да, method_missing примерно в три раза медленне (попробуйте Gist).

Но речь идет о доле микросекунды, в то время как запрос к базе занимает десятки-сотни милисекунд.

Так что профита в использовании find_by(id: ...) вместо find_by_id(...) никакого нет, остается лишь дело вкуса / командных стайл-гайдов.

[phoenixweiss]

Ну я бы не назвал это прямо ошибками. Скорее некоторый относительно субъективный стайл-гайд.

А вот что касается bang-методов — так вообще уж извините, но это ересь. Вообще плохой пример. Особенно если в проекте используется модульная система и большинство однотипных методов подобного рода вынесены в единый логический модуль — это совет-убийца. Учитывая особенности фреймворка, я бы лучше порекомендовал делать явные проверки, либо (что еще правильнее де-факто) — любой функционал стараться покрывать тестами. Тогда даже если у нас поменяются данные и тест не пройдет — мы узнаем об этом заранее, до того как на реальном проекте какие-то реальные данные вообще хоть куда-то пойдут.

По поводу unless тоже не совсем согласен — приведенный пример не самый удачный. Поправьте меня если неправ, но двойные отрицания лучше всего заменяются на условия, преобразуясь в цепочку условий. В зависимости от ситуации, их можно и обратить, что повысит удобочитаемость. Тот же RubyMine подобные вещи инспектирует и предлагает в некоторых моментах замены как раз подобного рода (не всегда удачно, но у нас по крайней мере есть выбор что удобнее).

А вообще в свое время один умный и опытный товарищ поделился со мной двумя замечательными ссылками:
github.com/arbox/ruby-style-guide
github.com/arbox/rails-style-guide

Советую как минимум, ознакомиться.

[erlyvideo]

> Если мы удалим пользователя, то связанные с ним посты перестанут быть корректными

foreign keys же.

[grossws]

github.com/matthuhiggins/foreigner до 4.2 и c 4.2 из коробки api.rubyonrails.org/v4.2.0/classes/ActiveRecord/ConnectionAdapters/SchemaStatements.html#method-i-add_foreign_key:

Creating a cascading foreign key

add_foreign_key :articles, :authors, on_delete: :cascade

generates:

ALTER TABLE "articles" ADD CONSTRAINT articles_author_id_fk FOREIGN KEY ("author_id") REFERENCES "authors" ("id") ON DELETE CASCADE

Поддерживаются postgresql и mysql.

[grossws]

До этого в моделях ещё можно было использовать has_* :name, dependent: :delete_all, что давало аналогичный результат, но требовало дополнительных запросов к базе.

[erlyvideo]

давно пора уже из кед вылезать.

[Cthulhu]

Жадный лоадинг иногда уместно использовать вместе с bullet.