Move update_attributes into class and check for non-nil params to update only

Question

There are a number of things wrong with the code here:

 @cust = Customer.find_by_id(params[:customer_id])
        upd = @cust.update_attributes({:city_id => params[:city],
                                       :zip_code => params[:zip],
                                       :first_name => params[:first_name],
                                       :last_name => params[:last_name],
                                       :phone => params[:phone].each { |s| s.gsub!(/\D+/,'')},
                                       :street1 => params[:street1],
                                       :street2 => params[:street2],
                                       :email => params[:email]
    })

1. It's currently in a controller action - I should move it to the Customer class
2. My params don't exactly match the keys in my collection (MongoDB) - but it's a bit too late for me to deal with that...
3. I use the .each with a block inside the update, I feel like I should separate that into it's own method
4. While I don't explicitly call save, it does actually save. The documentation on update_attributes says that I should call save explicitly or it won't...

Here are some questions:

1. What is the right way to move this into the model such that it will return TRUE or FALSE so that my controller can then set a flash[:success] or flash[:error] ?
2. How do I normalize my params so that I can write ONE UNIVERSAL update method like @cust.update_attributes(params) which will intelligently update only non-nil params ?
3. Is it absolutely worth the time to go back to the view and normalize the name attributes of each input to match the param?

I have other methods that update more or less params depending on the view, and I want to just have a single method "update_customer" that will take any old params hash with any values and only update the values that exist.

Answer 1

class Customer
  def update_customer params
    update_attributes(:city_id => params[:city],
                      :zip_code => params[:zip],
                      :first_name => params[:first_name],
                      :last_name => params[:last_name],
                      :phone => phone_number_digest(params[:phone]),
                      :street1 => params[:street1],
                      :street2 => params[:street2],
                      :email => params[:email])
  end

  def phone_number_digest *numbers
    numbers.map{ |s| s.gsub!(/\D+/,'')}
  end

end

In your controller

@cust = Customer.find(params[:customer_id])
@cust.update_customer(params)

This keeps it relatively clean and logically separates your persistence logic.

Also to answer your questions.

@cust = Customer.find(params[:customer_id])
if @cust.update_customer(params)
  #do something
else
  #do something else
end

1. Ruby executes the code in conditionals, so you could just have it like I have above.

2. you would probably want to do this logic in the controller, as only allow the keys with non nil params to work. This one is bit trickier, but definitely can be done. Perhaps you want to build your sanitized hash of params then pass it in. in that case, perhaps the ** operator(double splat for keyword args) would be useful.

3. Definitely worth your time. Naming is quite possibly one of the most important ideas in programming. You want to read code that you worked on a year or 2 ago without having to dive into the internals, and if you properly name you classes and methods, it becomes much simpler.