代码之家 › 专栏 › 技术社区 › user181452

如何重构导出为CSV的Ruby方法中的嵌套循环?

refactoring csv ruby ruby-on-rails

2

user181452 · 技术社区 · 10 年前

我必须将一些信息导出到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

1 回复 | 直到 7 年前

1

4

Dave Schweisguth 10 年前

我闻到的最大味道不是嵌套循环,而是从每个模型中获取值的方式的近似重复。

让我们将这种重复提取到同名的类似方法中, exported_values 在…上 Order , Move 和 Stop :

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

并将其用于 to_csv :

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

以上还有一些小的改进:

获取导出的值并将其连接到尽可能最外层的循环中,以提高效率。
循环移动和停止 each 而不是与 map ,因为循环是为了副作用而不是返回值。
删除不必要的使用 self. .

现在 到csv 也没那么糟糕。但它仍然有一点 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

不再有嵌套循环。提取的方法有点重复,但我认为如果提取重复,就不清楚了。

接下来,我们可以尝试重构 导出值 方法转换为单个方法。

可能 Order#exported_attributes 可以分解为每个类上的一个方法,该方法不接受参数,只返回该类的导出属性。
这些方法之间的另一个区别是 顺序 不需要 .decorator 但其他的类是这样的。如果它有一个装饰器,只需使用它而不是实际的顺序;如果没有,就给它一个假的:
```
class Order
  def decorator
    self
  end
end
```

然后可以定义单个 导出值 方法,并将其包含在所有三个类中:

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

并内联 values 当地人 append_to_csv 方法和在这些方法中,从父对象获取“前缀”,而不是将它们作为参数传递。

可能所有的新方法都应该提取到装饰器中,而不是提取到模型中;我不确定您的装饰器是用于CSV生成还是仅用于其他目的。