4
\$\begingroup\$

I have an app whose sole purpose is to seed data files and add the data to different CSVs which are zipped and exported by the user. My application controller is filled with lines that all look like this:

  def export_tips
    @appointments = Appointment.order('service_id')
    send_data @appointments.to_csv_tips, filename: 'tips.csv'
  end

  def export_ticketpayments
    @appointments = Appointment.order('service_id')
    send_data @appointments.to_csv_ticketpayments, filename: 'ticketspaymentitems.csv'
  end

  def export_batchmanifest
    @batchmanifests = Batchmanifest.all
    send_data @batchmanifests.to_csv_batchmanifest, filename: "batch_manifest-#{Date.today}.csv"
  end

  def export_pets
    @clients = Client.all
    send_data @clients.to_csv_pets, filename: 'pets.csv'
  end

  def export_clients
    @clients = Client.all
    send_data @clients.to_csv_clients, filename: 'clients.csv'
  end

I have it in the application controller because I used it in multiple different areas including creating single CSV exports and creating complex zip files with multiple zips and CSVs inside.

Some things that I have tried to cleanup the code include:

  • Different variables of this: def csv_export (model, filename) @model.pluralize = (model.titleize).all send_data @model.pluralize.filename, filename: filename end
  • Having each one in its own controller (could not access them from different views and other controllers easily)
  • I also tried to figure out how to create my own module, but was unable to do so.

My application record is just as bad with repeated lines simply meant to export the CSVs:

      def self.to_csv_appointments
        attributes = %w[appointment_id location_id employee_id client_id child_id notes 
        has_specific_employee start_time end_time]
        CSV.generate(headers: true) do |csv|
          csv << attributes
          all.each do |appointment|
            csv << attributes.map { |attr| appointment.send(attr) }
          end
        end
      end
    
      def self.to_csv_appointmentservices
        attributes = %w[appointment_id service_id price duration]
        CSV.generate(headers: true) do |csv|
          csv << attributes
          all.each do |appointment|
            csv << attributes.map { |attr| appointment.send(attr) }
          end
        end
      end

      def self.to_csv_tickets
        attributes = %w[ticket_id location_id client_id ticket_status employee_id 
        employee_id start_time]
        headers = %w[ticket_id location_id client_id status employee_id 
        closed_by_employee_id closed_at]
         CSV.generate(headers: true) do |csv|
         csv << headers
          all.each do |appointment|
           csv << attributes.map { |attr| appointment.send(attr) }
          end
        end
      end

For the application record, I have tried similar methods as those listed for the application controller, but to no avail. Again, I use the code in application record instead of in the individual model files because I need to access these in multiple parts of the site.

The code from the application controller is used mostly in the static controller and buttons on the view files. I need the ability to create the file sets, as listed below, but also allow the user to export just one CSV.

Examples from static controller to built the zip files:

def create_appointments_zip
  file_stream = Zip::OutputStream.write_buffer do |zip|
    @appointments = Appointment.order('service_id')
    zip.put_next_entry "appointment_manifest.csv"; zip << File.binread("#{Rails.root}/app/assets/csvs/appointment_manifest.csv")
    zip.put_next_entry "appointments.csv"; zip << @appointments.to_csv_appointments
    zip.put_next_entry "appointment_services.csv"; zip << @appointments.to_csv_appointmentservices
    zip.put_next_entry "appointment_statuses.csv"; zip << @appointments.to_csv_appointmentstatuses
  end
  file_stream.rewind
  File.open("#{Rails.root}/app/assets/csvs/appointments.zip", 'wb') do |file|
    file.write(file_stream.read)
  end
end

 def export_salonset
    create_appointments_zip
    create_tickets_zip
    create_inventory_zip
    create_memberships_zip
    file_stream = Zip::OutputStream.write_buffer do |zip|
      @saloncategories = Saloncategory.all
      @salonservices = Salonservice.all
      @clients = Client.all
      @locations = Location.all
      @salonpricings = Salonpricing.all
      @staffs = Staff.order("location_id")
      zip.put_next_entry "batch_manifest.csv"; zip << File.binread("#{Rails.root}/app/assets/csvs/batch_manifest_simple_salon.csv")
      zip.put_next_entry "categories.csv"; zip << @saloncategories.to_csv_saloncategories
      zip.put_next_entry "clients.csv"; zip << @clients.to_csv_clients
      zip.put_next_entry "employees.csv"; zip << @staffs.to_csv_staff
      zip.put_next_entry "locations.csv"; zip << @locations.to_csv_locations
      zip.put_next_entry "pricings.csv"; zip << @salonpricings.to_csv_pricings
      zip.put_next_entry "services.csv"; zip << @salonservices.to_csv_salonservices
      zip.put_next_entry "appointments.zip"; zip << File.binread("#{Rails.root}/app/assets/csvs/appointments.zip")
      zip.put_next_entry "inventories.zip"; zip << File.binread("#{Rails.root}/app/assets/csvs/inventories.zip")
      zip.put_next_entry "tickets.zip"; zip << File.binread("#{Rails.root}/app/assets/csvs/tickets.zip")
      zip.put_next_entry "addonmappings.csv"; zip << File.binread("#{Rails.root}/app/assets/csvs/addonmappings.csv")
    end
    file_stream.rewind
    respond_to do |format|
      format.zip do
        send_data file_stream.read, filename: "salon_set.zip"
      end
    end
    file_stream.rewind
    File.open("#{Rails.root}/app/assets/csvs/salon_set.zip", 'wb') do |file|
      file.write(file_stream.read)
    end
  end

Link to my repository, if that is helpful https://github.com/atayl16/data-wizard/blob/master/app/controllers/application_controller.rb https://github.com/atayl16/data-wizard/blob/master/app/models/application_record.rb

I know there must be a better way than writing these same lines over and over. The code works, my site works (amazingly), but I would be embarrassed for any seasoned developer to see the repository without laughing. Any help is appreciated!

\$\endgroup\$
1
  • 3
    \$\begingroup\$ Welcome to Code Review, I hope someone here can help you. You might want to try creating a list of CSV files that you can pass into a generic function that generates the zip files. I don't know ruby so I can't help beyond that. \$\endgroup\$
    – pacmaninbw
    Commented Aug 9, 2020 at 22:25

3 Answers 3

2
\$\begingroup\$

First thing I would do is separate CSV serialization from export data generation. For example the export data generation could be:

  def self.to_export
    attributes = %w[appointment_id location_id employee_id client_id child_id notes 
    has_specific_employee start_time end_time]
      all.map do |appointment|
        attributes.map { |attr| appointment.send(attr) }
      end
    end
  end

Then you'd have a method that takes an array of primitive Ruby objects and serializes to CSV.

The fancy name for this is presenter pattern.

Your zip file method is similar in that it does data processing, CSV serialization and zip export. Have one method that builds the data you want, (re)use the CSV serialization we just went over, add another method that takes a file and creates a zip file from it.

\$\endgroup\$
1
\$\begingroup\$

Noticed that you call new_method.pluralize five times in each pass through the loop. It's a small thing, but setting a variable once on each pass through would be slightly more efficient...

\$\endgroup\$
0
\$\begingroup\$

Thank you everyone for the advice! Adding in another thing I did to get the controllers cleaned up. I ended up using metaprogramming to clean my export controllers up. Here is an example in which I excluded some items from the array for brevity:

  ["bundle", "attendee", "location", "membership", "client", "staff"].each do |new_method|
    define_method("#{new_method.pluralize}") do
      instance_variable_set("@#{new_method.pluralize}", new_method.camelcase.constantize.all)
      instance_var = instance_variable_get("@#{new_method.pluralize}")
      send_data instance_var.public_send("to_csv_#{new_method.pluralize}"), filename: "#{new_method.pluralize}.csv"
    end
  end

I was able to remove 30 methods from my newly created export controller. Here is the code after pushing up the changes https://github.com/atayl16/data-wizard/blob/0011b6cf8c1fe967d73a569fa573cedc52cb8c72/app/controllers/export_controller.rb

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.