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

Question

Я новичок в Ruby, и я должен экспортировать информацию в 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

Обновить:

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

 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 не так уж и плохо Но у него все еще есть небольшая зависть к функциям (то есть он вызывает слишком много методов для других объектов), поэтому давайте выделим больше методов для моделей:

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

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

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