Does passing the params collection from controller to model break MVC?

Question

I want to pass the params collection from the controller to the model to parse filtering and sorting conditions. Does having a method in the model that takes the params from the controller break MVC?

Answer 1

I don't believe it does, but then again I am not a rails veteran by any means. Typically, the params hash is used in the controller and that action may or may not read and write model information, so I guess if the params were to go through a method belonging to the model, it would be the same thing.

Either way I think you will still need to send the params through a controller, so why not do your processing there, then send the processed data to the model through a method of the model?

Answer 2

I would say it definitely does.

The params hash has lots of things your models shouldn't need. Basically you're ignoring the C part of MVC. What you want to do will work (aka it will execute) But I think you should pass in the parameters as seperate entities.

Answer 3

It depends. You are passing a hash of data to the model and saying "make sense of this".

class Model < ActiveRecord::Base
  def update_from_params(params)
    ....
  end
end

class ModelsController < ActionController::Base
  def update
    ...
    @model.update_from_params(params)
  end
end

This is OK. But you may find yourself wanting to do this from many different actions. You are unlikely to be able to make the params exactly the same in each case, so you will need multiple methods on your model, one for each action:

class Model < ActiveRecord::Base
  def update_from_update_params(params)
    # do stuff
  end

  def update_from_settings_params(params)
    # do different stuff
  end

end

class ModelsController < ActionController::Base
  def update
    ...
    @model.update_from_update_params(params)
  end

  def change_settings
    ...
    @model.update_from_settings_params(params)
  end
end

This is not OK and you are making the model do controller work. A reasonable halfway house is to create a method on your model that accepts a canonical data hash, and then translate between the params and the canonical hash in the controller:

class Model < ActiveRecord::Base
  def update_from_data(hash)
    validate_data!(hash)
    # do stuff
  end
end

class ModelsController < ActionController::Base
  def update
    ...
    @model.update_from_data(translate_update_params)
  end

  def change_settings
    ...
    @model.update_from_data(translate_change_settings_params)
  end
end

Although you should make sure to carefully document the format of the data hash that the model accepts. We actually go so far as to use a YAML validation library (Rx) to ensure that the model only accepts valid data.

Sorry about the long answer, but I haven't got the time to write a shorter one ;).

Answer 4

Thanks everyone for the answers so far. Maybe a bit more about what I'm doing will help explain it.

This is the method in the model

def self.do_search(keyword,params)

        Rental.search(keyword, 
                      :per_page => self.per_page,
                      :page => page,
                      :conditions => will_condition(params),
                      :order => will_sort(params))

end

The params come into place on the will_condition and will_sort helper methods. These methods parse the params to find filters or sort_by/dir parameters. The acceptable filters and sortable_fields are defined like so:

  @@searchable_fields = ['property_status_id', 'neighborhood_id', 'borough_id', 'number_of_bedrooms']  
  cattr_reader :searchable_fields  

  @@sortable_fields =   ['number_of_bedrooms','rent','street_address']
  cattr_reader :sortable_fields

It's pretty handy, but it feels kind of messy passing in the whole params. The reason I'm keeping it in the model is that I'd like to package it up as a gem to reuse it and limit the number of moving parts. I could possibly add some helper methods to the controller (i.e. will_sort_params and will_condition_params) that do the nitpicking, but those would need to know what model they are operating on to grab the constants...

Answer 5

I would suggest passing a subset of params, this way you are passing only what the Model needs.

In the controller:

# in controller
def search
  Model.search(params[:search][:options])
end

Just make sure your inputs are 'namespaced' so you get a nested hash:

<!-- in view -->
<input type='text' name='search[options][keywords]' />
<input type='text' name='search[options][conditions]' />
<input type='text' name='search[options][sort]' />

Then in your model:

def self.do_search(criteria)

        Rental.search(criteria[:keywords], 
                      :per_page => self.per_page,
                      :page => page,
                      :conditions => criteria[:conditions],
                      :order => criteria[:sort])

end