Rubocop 声明太多 'if' 声明

Question

我正在使用 Rubocop 测量 Rails 应用程序的代码。这是我的代码：

def ratings_treatment
    if @rating.advertise_id
      advertise = Advertise.find(advertise_id)
      rating_average(advertise)
    elsif @rating.product_id
      product = Product.find(product_id)
      rating_average(product)
    elsif @rating.combo_id
      combo = Combo.find(combo_id)
      rating_average(combo)
    else
      establishment = Establishment.find(@rating.establishment_id)
      rating_average(establishment)
    end   
end

它没有通过关于 if 的测试。声称有太多if语句。

我考虑过将代码拆分为一些检查器方法，但想知道是否有更好的方法来获得良好的指标并仍然编写出良好的代码。

Answer 1

我认为您可以使用 switch 语句为 rating_average:

分配参数

def ratings_treatment
  average = case
            when @rating.advertise_id then Advertise.find(advertise_id)
            when @rating.product_id   then Product.find(product_id)
            when @rating.combo_id     then Combo.find(combo_id)
            else Establishment.find(@rating.establishment_id)
            end
  rating_average average
end

尽管 Rubocop 会抱怨

Style/EmptyCaseCondition: Do not use empty case condition, instead use an if expression

您还可以简化您的 if 表达式：

def ratings_treatment
  average = if @rating.advertise_id
              Advertise.find advertise_id
            elsif @rating.product_id
              Product.find product_id
            elsif @rating.combo_id
              Combo.find combo_id
            else
              Establishment.find @rating.establishment_id
            end
  rating_average average
end

Answer 2

如果您的 @rating 设置了关联，那么您可以说：

def ratings_treatment
  obj = @rating.advertise || @rating.product || @rating.combo || @rating.establishment
  rating_average(obj)
end

甚至：

def ratings_treatment
  rating_average(@rating.advertise || @rating.product || @rating.combo || @rating.establishment)
end

或者也许是一些奇特的东西（并且只对四个分支杀伤力过大）：

def ratings_treatment
  rating_average(
    %w[advertise product combo establishment].lazy.map { |m| @rating.send(m) }.find(&:present?)
  )
end

或者甚至：

def rated_object
  to_result = ->(m) { @rating.send(m) }
  %w[advertise product combo establishment]
    .lazy
    .map(&to_result)
    .find(&:present?)
end

def ratings_treatment
  rating_average(rated_object)
end

我什至建议像上面的 rated_object 方法这样的东西确实应该是 @rating 上的方法。您应该能够对已评级的事物进行评级，而无需手动遍历所有可能性。

你甚至可以将 lazy.map... 的内容修补到 Enumerable 中，如果你想要 Enumerable 中的某些东西一直表现得像 e.m1 || e.m2 || e.m3 || ...（在那里，做到了）短路。

虽然我不知道 Rubocop 会对这些恶作剧做出什么反应。

Answer 3

这是一个经典的风格违规案例，它有更深层次的根本原因，即建模问题。在您的情况下，看起来您实际想要实现的是 polymorphic association。如果您将 Rating 更改为：

# app/models/rating.rb
class Rating < ApplicationRecord
  belongs_to :rateable, polymorphic: true
end

您可以让 Rating 与任意数量的事物相关，只需将其添加到关联的另一端：

# app/models/product.rb
class Product < ApplicationRecord
  has_many :pictures, as: :rateable
end

你原来的方法，如果还需要的话，变成：

def ratings_treatment
  rating_average(@rating.rateable) 
end

Rubocop 声明太多 'if' 声明

Rubocop claims for too many 'if' statements

ruby

rubocop