Как назначить переменную с результатом блока if..else?

Question

Я поспорил с коллегой о том, как лучше назначить переменную в блоке if..else. Его оригинальный код был:

@products = if params[:category]
  Category.find(params[:category]).products
else
  Product.all
end

Я переписал это так:

if params[:category]
  @products = Category.find(params[:category]).products
else
  @products = Product.all
end

Это также можно переписать однострочно с использованием оператора ternery (? :), но давайте представим, что назначение продукта длиннее 100 символов и не может поместиться в одну строку.

Что из двух вам понятнее? Первое решение занимает немного меньше места, но я подумал, что объявление переменной и присвоение ей трех строк после может быть более подвержено ошибкам. Мне также нравится видеть, что мои if и else выровнены, так что моему мозгу легче их анализировать!

Answer 1

В качестве альтернативы синтаксису в ответе badp я хотел бы предложить:

@products = 
  if params[:category]
    Category.find(params[:category]).products
  else
    Product.all
  end

Я утверждаю, что это имеет два преимущества:

1. Равномерный отступ: каждый уровень логического вложения имеет ровно два пробела (ОК, может быть, это просто вопрос вкуса)
2. Горизонтальная компактность: Более длинное имя переменной не будет сдвигать код с отступом после отметки столбца 80 (или любого другого)

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

Отказ от ответственности: это мой собственный особенный подход, и я не знаю, в какой степени он используется в других местах сообщества Ruby.

Редактировать: Я должен отметить, что ответ мацадлера также похож на этот. Я думаю, что наличие некоторых отступов полезно. Надеюсь, этого будет достаточно, чтобы сделать отдельный ответ.

Answer 2

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

Я также согласен с предложением bp о том, что вы можете сделать его более понятным, сделав отступ для целого выражения if так, чтобы все это было визуально справа от назначения. Это абсолютно эстетично, но я думаю, что это делает его более легким для просмотра и должно быть понятнее даже для тех, кто не знаком с языком.

Так же, как в стороне: Этот тип if вовсе не уникален для Ruby. Он существует во всех Лиспах (Common Lisp, Scheme, Clojure и т. Д.), Scala, во всех ML (F #, OCaml, SML), Haskell, Erlang и даже в прямом предшественнике Ruby, Smalltalk. Это просто не распространено в языках, основанных на C (C ++, Java, C #, Objective-C), которые используют большинство людей.

Answer 3

Мне не нравится использование пробелов в первом блоке. Да, я Pythonista, но я полагаю, что могу сказать правду, говоря, что первые могут выглядеть запутанно в середине другого кода, возможно, вокруг других if блоков.

Как насчет ...

@products = if params[:category] Category.find(params[:category]).products
            else                 Product.all
            end

@products = if params[:category]
              Category.find(params[:category]).products
            else                
              Product.all
            end

---

Вы также можете попробовать ...

@products = Product.all #unless a category is specified:
@products = Category.find(params[:category]).products if params[:category]

... но это плохая идея, если Product.all на самом деле является функцией, которую потом можно было бы излишне оценить.

Answer 4

герметизация ...

@products = get_products

def get_products
  if params[:category]
    Category.find(params[:category]).products
  else
    Product.all
  end
end

Answer 5

Просто другой подход:

category = Category.find(params[:category]) if params[:category]
@products = category ? category.products : Product.all

Answer 6

Еще один подход - использовать блок, чтобы обернуть его.

@products = begin
  if params[:category]
    Category.find(params[:category]).products
  else
    Product.all
  end
end

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

@products ||= begin
  if params[:category]
    Category.find(params[:category]).products
  else
    Product.all
  end
end

Это то, что вы не можете сделать с переписанным кодом, и он правильно выровнен.

Answer 7

@products =
if params[:category]
  Category.find(params[:category]).products
else
  Product.all
end

Это еще один вариант, он позволяет избежать повторения @products и сохраняет if выровненным с else.

Answer 8

Если ваши модели выглядят так:

class Category < ActiveRecord::Base
  has_many :products
end
class Product < ActiveRecord::Base
  belongs_to :category
end

Вы можете сделать что-то еще более безумное, как это:

#assuming params[:category] is an id
@products = Product.all( params[:category] ? {:conditions => { :category_id => params[:category]}} : {})

Или, вы можете использовать сексуальную, лениво загруженную named_scope функциональность:

class Product < ActiveRecord::Base
  ...

  #again assuming category_id exists
  named_scope :all_by_category, lambda do |cat_id|
    if cat_id
      {:conditions => {:category_id => cat_id}}
    end
  end

  #if params[:category] is a name, and there is a has and belongs to many
  named_scope :all_by_category, lambda do |cat_name|
    if cat_name
      {:joins => :categories, :conditions => ["categories.name = ?",cat_name]}
    end
  end
  ...
end

используется как

@products = Product.all_by_category params[:category]

Answer 9

Я думаю, что лучший код:

@products = Category.find(params[:category])&.products.presence || Product.all

«&» после определения гарантирует, что метод «products» не будет оценивать, если категория равна нулю.

Answer 10

Я бы сказал, что вторая версия более читабельна для людей, не знакомых с этой структурой в ruby. Так что + за это! С другой стороны, первая конструкция более сухая.

Поскольку я смотрю на это немного дольше, я нахожу первое решение более привлекательным. Я программист на рубине, но раньше этим не пользовался. Обязательно начну!