MVC design decisions

[atkolkma]

I have a question about the best approach for building out some business logic in a rails app. Really its a question about how to approach MVC architecture.

Background: In my app I have a CSV file that represents an inventory of vehicles. The csv file (inventory feed) is regularly updated by a third party via ftp. My app needs to periodically load the csv file, do some heavy parsing on it, and then persist the data to an Inventory database/model. It's clear that I need a Vehicle class to store all of my vehicles in my inventory. What is not clear is how to go about loading and parsing the csv.

Originally, I was planing on the Vehicle class to do all of these jobs. Then I considered it might be a better idea to have a clearer separation of concerns. So I decided to create a Feed class to do the csv loading and parsing. But then I started to think about it... In all of the RoR tuts I read, each Class gets a Model, View and Controller. It's clear that a Feed class would need a controller, but it seems non standard to have a controller without a model. On the other hand, I don't need to persist any data in the Feed class, so what's the need for a Model and database?

So should I make a Feed Class in a separate lib/feed.rb file? In a Model without a database? Should it be a rake task? Should I smash the functionality into the Vehicle MVC? Are their other design options?

[toreyheinz]

I should be able to steer you in the right direction once I get back to the computer. On Feb 12, 2014 7:19 PM, "Adam Kolkman" notifications@github.com wrote:

I have a question about the best approach for building out some business logic in a rails app. Really its a question about how to approach MVC architecture.

Background: In my app I have a CSV file that represents an inventory of vehicles. The csv file (inventory feed) is regularly updated by a third party via ftp. My app needs to periodically load the csv file, do some heavy parsing on it, and then persist the data to an Inventory database/model. It's clear that I need a Vehicle class to store all of my vehicles in my inventory. What is not clear is how to go about loading and parsing the csv.

Originally, I was planing on the Vehicle class to do all of these jobs. Then I considered it might be a better idea to have a clearer separation of concerns. So I decided to create a Feed class to do the csv loading and parsing. But then I started to think about it... In all of the RoR tuts I read, each Class gets a Model, View and Controller. It's clear that a Feed class would need a controller, but it seems non standard to have a controller without a model. On the other hand, I don't need to persist any data in the Feed class, so what's the need for a Model and database?

So should I make a Feed Class in a separate lib/feed.rb file? In a Model without a database? Should it be a rake task? Should I smash the functionality into the Vehicle MVC? Are their other design options?

— Reply to this email directly or view it on GitHubhttps://github.com/WestMichiganRubyTraining/discussion/issues/58 .

[billgathen]

MVC is a useful design pattern, but what you're describing is the Adapter pattern. You have data in one format (CSV) and you need it in another (ActiveRecord). Since you need to load the data periodically, you can decouple from the Rails workflow entirely. Rails controllers broker HTTP requests to business logic, but you don't need to do that here.

I'd suggest a rake task that does nothing but call a dedicated library module (VehicleCollector, maybe?) and hand it the filename you're pulling in. Rake tasks are more difficult to test, so move as much logic as possible out into pure ruby objects. You can run the rake task from a cron job if the timing is regular.

VehicleCollector should be a module that does nothing but pipe the input file through Ruby's standard CSV library and yield each record to a mapper object (VehicleMapper?) that maps a single row onto a single AR record.

VehicleCollector should return the collection of mapped objects, and the rake task can call "save" on each record.

Your Vehicle object knows nothing about CSV files, so if that changes later, your Vehicle class is untouched. You can test the mapping easily with lots of edge cases without having to build whole CSV file every time by testing VehicleMapper directly. Do some integration testing on VehicleCollector to make sure it's wired correctly and you should be good to go. Remember to check for malformed and empty records, as well as empty files!

Hope that's useful. I'm sure lots of others will weigh in and you'll have plenty of options to choose from.

[atkolkma]

Extremely useful, Bill. I was already playing with this approach a bit, but the finer points were totally lost to me. This helps a lot.

On Wed, Feb 12, 2014 at 8:31 PM, Bill Gathen notifications@github.comwrote:

MVC is a useful design pattern, but what you're describing is the Adapter pattern. You have data in one format (CSV) and you need it in another (ActiveRecord). Since you need to load the data periodically, you can decouple from the Rails workflow entirely. Rails controllers broker HTTP requests to business logic, but you don't need to do that here.

I'd suggest a rake task that does nothing but call a dedicated library module (VehicleCollector, maybe?) and hand it the filename you're pulling in. Rake tasks are more difficult to test, so move as much logic as possible out into pure ruby objects. You can run the rake task from a cron job if the timing is regular.

VehicleCollector should be a module that does nothing but pipe the input file through Ruby's standard CSV library and yield each record to a mapper object (VehicleMapper?) that maps a single csv row onto a single AR record.

VehicleCollector should return the collection of mapped objects, and the rake task can call "save" on each record.

Your Vehicle object knows nothing about CSV files, so if that changes later, your Vehicle class is untouched. You can test the mapping easily with lots of edge cases without having to build whole CSV file every time by testing VehicleMapper directly. Do some integration testing on VehicleCollector to make sure it's wired correctly and you should be good to go. Remember to check for malformed and empty records, as well as empty files!

Hope that's useful. I'm sure lots of others will weigh in and you'll have lots of options to choose from.

Reply to this email directly or view it on GitHubhttps://github.com/WestMichiganRubyTraining/discussion/issues/58#issuecomment-34939496 .

[toreyheinz]

Yes Bill, I have been finding more and more that the business logic of my apps don't fit within the MVC pattern.

It's helpful to think of Rails as mostly a system for routing HTTP requests to business logic, with a bunch of really nice helpers!

I have been solving things like this with Interactors https://github.com/collectiveidea/interactor.

I will try my best to give a high level run through, via code.

## app/interactors/collect_vehicles
require 'net/ftp'
class CollectVehicles
  include Interactor

  def setup
    # This may be a bad assumption, 
    # but just in case you actually need to retrieve the file via FTP
    @file = Tempfile.new('inventory.csv')
    ftp.getbinaryfile("ftp/path/to/inventory.csv", @file)
  end

  def perform
    CSV.foreach(@file, headers: true) do |csv_row|
      parsed_row = ParseInventoryRow.perform(row: csv_row)

      if parsed_row.success? # Interactors return success unless they fail!
        # use `find_or_initalize_by` to avoid creating dups, or having to clear the records first.
        Vehicle.find_or_initalize_by(vehicle_id: row[:vehicle_id]) do |vehicle|
          vehicle.update_attributes! parsed_row.attributes
        end
      else
        next
      end
    end
  end

private
  def ftp
    @ftp ||= Net::FTP.open('ftp.example.com', 'username', 'password')
  end
end

## app/interactors/parse_inventory_row.rb
class ParseInventoryRow
  include Interactor

  def perform
    if valid_row?
      context[:attributes] = {
        vehicle_id: row[:vehicle_id],
        price:      row[:vehicle_msr_cost].to_i,
        desc:       row[:vehicle_description],
        ...
      }
    else
      fail!(message: 'Invalid Row')
    end
  end

private 
  def valid_row?
    ## Check for row validness
  end
end

To schedule this you could look at the whenever gem https://github.com/javan/whenever

gem 'whenever', :require => false

## config/schedule.rb
every 1.day, :at => '4:30 am' do
  runner "CollectVehicles.perform"
end

Sorry for the lack of explanation, if you have questions please feel free to ask. I will try to answer.

The example above could have easily been done without Interactor gem, but I have been liking the consistent pattern it provides.

[atkolkma]

Thanks Torey. This gives me a bit to chew on. I'll let you know if I need help working through this example.

I'm new to Rails, so it's good to hear that some of my impressions about implementation are not totally off. Lately, I've been seeing the occasional need to break out of the MVC paradigm, but not knowing when/if it is appropriate. This helps a lot.

On Wed, Feb 12, 2014 at 10:25 PM, Torey Heinz notifications@github.comwrote:

Yes Bill, I have been finding more and more that the business logic of my apps don't fit within the MVC pattern.

It's helpful to think of Rails as mostly a system for routing HTTP requests to business logic, with a bunch of really nice helpers!

I have been solving things like this with Interactors https://github.com/collectiveidea/interactor.

I will try my best to give a high level run through, via code.

app/interactors/collect_vehiclesrequire 'net/ftp'class CollectVehicles

include Interactor

def setup

This may be a bad assumption,

# but just in case you actually need to retrieve the file via FTP
@file = Tempfile.new('inventory.csv')
ftp.getbinaryfile("ftp/path/to/inventory.csv", @file)

end

def perform CSV.foreach(@file, headers: true) do |csv_row| parsed_row = ParseInventoryRow.perform(row: csv_row)

  if parsed_row.success? # Interactors return success unless they fail!
    # use `find_or_initalize_by` to avoid creating dups, or having to clear the records first.
    Vehicle.find_or_initalize_by(vehicle_id: row[:vehicle_id]) do |vehicle|
      vehicle.update_attributes! parsed_row.attributes
    end
  else
    next
  end
end

end private def ftp @ftp ||= Net::FTP.open('ftp.example.com', 'username', 'password') endend

app/interactors/parse_inventory_row.rbclass ParseInventoryRow

include Interactor

def perform if valid_row? context[:attributes] = { vehicle_id: row[:vehicle_id], price: row[:vehicle_msr_cost].to_i, desc: row[:vehicle_description], ... } else fail!(message: 'Invalid Row') end end private def valid_row?

Check for row validness

endend

To schedule this you could look at the whenever gem https://github.com/javan/whenever

gem 'whenever', :require => false

config/schedule.rbevery 1.day, :at => '4:30 am' do

runner "CollectVehicles.perform"end

Sorry for the lack of explanation, if you have questions please feel free to ask. I will try to answer.

The example above could have easily been done without Interactor gem, but I have been liking the consistent pattern it provides.

Reply to this email directly or view it on GitHubhttps://github.com/WestMichiganRubyTraining/discussion/issues/58#issuecomment-34945043 .