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

Claudication

I think you mean "create!" not "create" in Method 1. "create" won't raise an exception; it simply returns false on validation errors. – Diastasis

Answer 1

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.

Nickelic

chaos's point that, "exceptions shouldn't be used for routine conditions" is a good one. Yet Rails has decided otherwise: "rescue_from ActiveRecord::RecordInvalid" is common practice. – Diastasis

Answer 2

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

Fabliau

Answer 3

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.

Reparative

Since this is a create action, are you saying that version 1 is good if I have client side validation, but version 2 is better if I am using server side validation? – Claudication

Not quite... with few exceptions, you always want to validate on the server-side, as a security precaution (client-side validation can always be disabled by a malicious attacker)... (splitting into two comments) – Reparative

My rule of thumb, is I use save() when I expect the operation is likely to complete without running into a DB constraint, and create() when I expect that it might... does that help? I also always implement a global exception handler to catch stray errors. – Reparative

in your answer you say "if you expect all create statements to work, use (create)". here you are saying to use create when you expect the operation will run into a db constraint (i think you mean fail). also, i was thinking validate on both client and server – Claudication

Answer 4

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.

Cinchonidine