When creating an object in Ruby on Rails, which method of saving do you prefer, and why?

Question

When writing the "create" method for an object in a Ruby on Rails app, I have used two methods. I would like to use one method for the sake of cleaner and more consistent code. I will list the two methods below. Does anyone know if one is better than the other? If so, why?

Method 1:

def create1
  # is this unsecure? should we grab user_id from the session
  params[:venue]['user_id'] = params[:user_id]

  begin
    venue = Venue.create(params[:venue])
    @user_venues = @user.venues
    render :partial => 'venue_select_box', :success => true, :status => :ok
  rescue ActiveRecord::RecordInvalid
    render :text => 'Put errors in here', :success => false, :status => :unprocessable_entity
  end
end

Method 2:

def create2
   # is this unsecure? should we grab user_id from the session
  params[:venue]['user_id'] = params[:user_id]

  venue = Venue.new(params[:venue])
  if venue.save
    @user_venues = @user.venues
    render :partial => 'venue_select_box', :success => true, :status => :ok
  else
    render :text => 'Put errors in here', :success => false, :status => :unprocessable_entity
  end
end

Answer 1

I'm of the school of thought that exceptions shouldn't be used for routine conditions, so I'd say the second is better.

Answer 2

It depends. If you expect all of the create statements to work, use the former, because the failure to create-and-save is exceptional, and may possibly be a condition from which the program can't readily recover. Also, if you use relational integrity (foreign_key_migrations by RedHill Consulting), this will throw exceptions on foreign key violations, so you probably want to catch them whenever creating or updating.

The second is workable, and good if the query not succeeding is something you expect as part of the day-to-day operation of that particular action.

Also, your code comment about session being insecure -- the session is the place to put the user_id. As long as you're checking to verify that the user has been authenticated before doing anything else, you'll be okay.

Answer 3

I totally agree with Don's comment. But I would even go one step further with the user_id part and set it as a before filter on the model.

Answer 4

class VenuesController < ApplicationController
  def create
    @venue = @user.venues.create!(params[:venue])
    render :partial => 'venue_select_box', :success => true, :status => :ok
  end

  rescue_from ActiveRecord::RecordInvalid do
    render :text => 'Put errors in here', :success => false, :status => :unprocessable_entity
  end
end

Using @user.venues in this way ensure that the user ID will always be set appropriately. In addition, ActiveRecord will protect the :user_id field from assignment during the course of the #create! call. Hence, attacks from the outside will not be able to modify :user_id.

In your tests, you may verify that doing a POST to :create raises an ActiveRecord::RecordInvalid exception.