Refactoring verschachtelte Schleifen in einer Ruby-Methode, die nach CSV exportiert

Question

Ich bin neu in Ruby, und ich muss Informationen in CSV exportieren. Ich habe diesen Code geschrieben und ich mag ihn nicht wirklich. Ich weiß nicht, wie ich es umgestalten und die verschachtelten Schleifen loswerden kann.

Meine Beziehungen sind die folgenden: Reihenfolge hat viele Züge, Bewegung hat viele Stopps.

Ich muss alles in CSV exportieren, also werde ich mehrere Zeilen für die gleiche Reihenfolge haben!

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 

Es ist nicht von guter Qualität Code ..

Ich tat dies gestern:

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:

Danke für die Antwort unten, kann immer noch nicht von der loswerden Nested Loops, aber zumindest ist es gut strukturiert ohne Schlüsselwert großes Array :)

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 

mit diesem in der abstrakten Dekorateur Klasse:

def exported_attributes 
    [] 
    end 

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

und in jedem Dekorateur der Ordnung, verschieben, stoppen, ich neu definiert wieder exported_attributes.

Answer 1

Der größte Geruch, den ich rieche, ist nicht die verschachtelten Schleifen, sondern die fast-Duplizierung, wie die Werte von jedem Modell erhalten werden.

Lassen Sie uns, dass die Vervielfältigung in ähnliche Methoden extrahieren mit dem gleichen Namen, exported_values, auf Order, Move und 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 

und sie in 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 

Die oben hat einige zusätzliche kleinere Verbesserungen:

• Die exportierten Werte in den äußersten möglichen Schleifen für die Effizienz abrufen und verketten.
• Schleife über bewegt und stoppt mit each statt mit map, da die Schleifen für Nebenwirkungen statt Rückgabewerte getan werden.
• Entfernen Sie unnötige Verwendungen von self..

Jetzt ist to_csv nicht so schlecht. Aber es hat noch ein wenig feature envy (das heißt, es ruft zu viele Methoden auf andere Objekte), so lassen Sie uns mehr Methoden auf die Modelle extrahieren:

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 

Keine verschachtelten Schleifen. Die extrahierten Methoden sind etwas duplikativ, aber ich denke, wenn die Duplizierung extrahiert würde, wären sie unklar.

Als nächstes könnten wir versuchen, die exported_values Methoden in einer einzigen Methode zu refaktorieren.

• Vielleicht Order#exported_attributes könnte auf jeder Klasse in ein Verfahren aufgebrochen werden, die keine Argumente nehmen und nur die exportierten Attribute der Klasse. Der andere Unterschied zwischen den Methoden ist, dass Order.decorator nicht benötigt, aber die anderen Klassen tun. Wenn es einen Dekorateur hat, benutze das anstelle der tatsächlichen Reihenfolge; wenn nicht, ist es nur geben eine gefälschte:

  class Order 
      def decorator 
      self 
      end 
  end 
  

Sie könnten dann eine einzelne exported_values Methode in einem Modul definieren und sie in allen drei Klassen:

def exported_values 
    exported_attributes.map { |attrs| { decorator.public_send(attrs) } 
end 

Es gibt eine weitere Mögliche Verbesserung: Wenn es für die exportierten Werte eines jeden Modells OK war, für die Lebensdauer einer Instanz gleich zu bleiben, konnten Sie sie wie folgt zwischenspeichern

def exported_values 
    @exported_values ||= exported_attributes.map { |attrs| { decorator.public_send(attrs) } 
end 

und inline die values Locals in den append_to_csv Methoden und erhalten Sie die "Präfixe" von übergeordneten Objekten in diesen Methoden, anstatt sie als Parameter übergeben.

Möglicherweise sollten alle neuen Methoden zu den Dekoratoren und nicht zu den Modellen extrahiert werden. Ich bin mir nicht sicher, ob Ihre Decorators für CSV-Generierung oder nur für andere Zwecke sind.