我必须将一些信息导出到csv。我写了这段代码,我不太喜欢。我不知道如何重构它并去掉嵌套循环。
我的关系是:顺序有很多步,移动有很多站。
我必须将所有这些导出到csv,所以我将为同一订单有多行。
这是我的(低质量)代码:

def to_csv
  CSV.generate(headers: true) do |csv|
    csv << h.t(self.first.exported_attributes.values.flatten) # headers
    self.each do |order|
      order.moves.map do |move|
        move.stops.map do |stop|
          order_data = order.exported_attributes[:order].map do |attributes|
            order.public_send(attributes)
          end
          move_data = order.exported_attributes[:move].map do |attributes|
            move.decorate.public_send(attributes)
          end
          stop_data = order.exported_attributes[:stop].map do |attributes|
            stop.decorate.public_send(attributes)
          end
          csv << order_data + move_data + stop_data
        end
      end
    end
  end
end

我昨天做的:
  def to_csv
    CSV.generate(headers: true) do |csv|
      csv << h.t(self.first.exported_attributes.values.flatten) # headers
      self.each do |order|
        order.moves.each do |move|
          move.stops.each do |stop|
            csv << order.exported_attributes[:order].map { |attr| order.public_send(attr) } +
              order.exported_attributes[:move].map { |attr| move.decorate.send(attr) } +
              order.exported_attributes[:stop].map { |attr| stop.decorate.send(attr) }
          end
        end
      end
    end
  end

最佳答案

我闻到的最大的气味不是嵌套循环,而是从每个模型中获取值的近乎重复的味道。
让我们用相同的名称,在exported_valuesOrderMove上,将复制提取到类似的方法中:

class Order
  def exported_values
    exported_attributes[:order].map { |attrs| { public_send(attrs) }
  end
end

class Move
  def exported_values
    order.exported_attributes[:stop].map { |attrs| { decorate.public_send(attrs) }
  end
end

class Stop
  def exported_values
    move.order.exported_attributes[:move].map { |attrs| { decorate.public_send(attrs) }
  end
end

并在Stop中使用它们:
def to_csv
  CSV.generate(headers: true) do |csv|
    csv << h.t(first.exported_attributes.values.flatten) # headers
    each do |order|
      order_values = order.exported_values
      order.moves.each do |move|
        order_and_move_values = order_values + move.exported_values
        move.stops.each do |stop|
          csv << order_and_move_values + stop.exported_values
        end
      end
    end
  end
end

上面还有一些小的改进:
为了提高效率,在最外层的循环中获取并连接导出的值。
由于循环是为副作用而不是返回值而完成的,因此循环将使用to_csv而不是each移动和停止。
删除不必要的map用法。
现在self.还不错。但是它仍然有一点feature envy(也就是说,它在其他对象上调用了太多的方法),所以让我们在模型上提取更多的方法:
def to_csv
  CSV.generate(headers: true) do |csv|
    csv << h.t(first.exported_attributes.values.flatten) # headers
    each { |order| order.append_to_csv(csv) }
  end
end

class Order
  def append_to_csv(csv)
    values = exported_values
    moves.each { |move| move.append_to_csv(csv, values) }
  end
end

class Move
  def append_to_csv(csv, prefix)
    values = exported_values
    stops.each { |stop| stop.append_to_csv(csv, prefix + values) }
  end
end

class Stop
  def append_to_csv(csv, prefix)
    csv << prefix + exported_values
  end
end

不再嵌套循环。提取的方法有点重复,但我认为如果提取重复的话,就不清楚了。
接下来,我们可以尝试将to_csv方法重构为单个方法。
也许exported_values可以被分解为每个类上的一个方法,该方法不接受参数,只返回该类的导出属性。
两个方法的另一个区别是Order#exported_attributes不需要Order,但其他类需要。如果它有一个装饰器,就用它代替实际的订单;如果没有,就给它一个假订单:
class Order
  def decorator
    self
  end
end

然后可以在模块中定义一个.decorator方法,并将其包含在所有三个类中:
def exported_values
  exported_attributes.map { |attrs| { decorator.public_send(attrs) }
end

还有一个可能的改进:如果每个模型的导出值在实例的生命周期内保持不变是可以的,那么可以这样缓存它们
def exported_values
  @exported_values ||= exported_attributes.map { |attrs| { decorator.public_send(attrs) }
end

并内联exported_values方法中的values局部变量,从这些方法中的父对象获取“前缀”,而不是将它们作为参数传递。
可能所有新方法都应该提取到decorators而不是模型;我不确定您的decorators是用于csv生成还是仅用于其他目的。

关于ruby-on-rails - 如何在导出到CSV的Ruby方法中重构嵌套循环?,我们在Stack Overflow上找到一个类似的问题:https://stackoverflow.com/questions/37461910/

10-14 15:04
查看更多