我必须将一些信息导出到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_values
,Order
和Move
上,将复制提取到类似的方法中:
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/