Рефакторинг вложенных циклов в Ruby-методе, который экспортируется в CSV

Question

Я новичок в Ruby, и мне нужно экспортировать информацию в CSV. Я написал этот код, и мне это не очень нравится. Я не знаю, как я могу реорганизовать его и избавиться от вложенных циклов.

Мои отношения следующие: у Ордена есть много движений, у Move есть много остановок.

Мне нужно экспортировать все это в 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 

Update:

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

def to_csv 
    CSV.generate(headers: true) do |csv| 
     csv << h.t(Order.first.decorate.exported_attributes + Move.first.decorate.exported_attributes + 
       Stop.first.decorate.exported_attributes) 
     self.each do |order| 
     order.moves.each do |move| 
      move.stops.each do |stop| 
      csv << order.exported_values + move.decorate.exported_values + stop.decorate.exported_values 
      end 
     end 
     end 
    end 
    end 

с ними в абстрактном классе декоратора:

def exported_attributes 
    [] 
    end 

    def exported_values 
    exported_attributes.map { |attr| self.public_send(attr) } 
    end 

и в каждом декоратор порядке, двигаться, стоп, я снова пересмотрел exported_attributes.

Answer 1

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

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

В настоящее время to_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 

Нет больше вложенных циклов. Выделенные методы немного дублируют, но я думаю, что если бы дублирование было извлечено, они были бы неясны.

Далее мы могли бы попытаться реорганизовать методы exported_values в один метод.

• Order#exported_attributes Возможно, можно было бы разбить на метод по каждому классу, который не принимает никаких аргументов и возвращает только экспортируемые атрибуты этого класса.

• Другая разница между методами заключается в том, что Order не нуждается в .decorator, но другие классы. Если у него есть декоратор, просто используйте это вместо фактического заказа; если нет, то просто дать ему поддельный:

  class Order 
      def decorator 
      self 
      end 
  end 
  

Затем можно определить один exported_values метод в модуле и включить его во всех трех классах:

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 

и inline values местные жители в append_to_csv методах и получают «префиксы» из родительских объектов в этих методах вместо передачи их в качестве параметров.

Возможно, все новые методы должны быть выделены декораторам, а не моделям; Я не уверен, предназначены ли ваши декораторы для генерации CSV или только для других целей.