Как Вы присваиваете переменную с результатом если.. еще блок?

Мне кажется, что второй вариант будет более читабельным для типичного программиста . Я не рубиновый парень, поэтому я не понимал, что if / else возвращает значение .... Итак, чтобы взять меня в качестве примера (и да, это моя точка зрения: D), второй вариант выглядит как хороший выбор.

инкапсуляция ...

@products = get_products

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

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

Другой вариант, он позволяет избежать повторения @products и поддерживает выравнивание if с else .

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

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

Я тоже не специалист по Ruby, но тревожные звоночки сразу же раздаются по поводу области применения второй команды, будет ли эта переменная вообще доступна после завершения вашего блока if?

Во-первых, если используется тройной, второй - если нет.

Первую практически невозможно прочитать.

Еще один подход:

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

Мне не нравится, что вы используете пробелы в вашем первом блоке. Да, я 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 на самом деле похож на функцию, которая может быть ненужной оценен.

Как программист 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), которыми пользуется большинство людей.

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

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

Предполагая, что ваши модели выглядят так:

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]