views:

31

answers:

2

Oddly I'm having a hard time finding good docs about basic error handling in rails. I'd appreciate any good links as well as thoughts on a handling errors in a really basic method like this:

  def self.get_record(id)
    People.first( :conditions => ["id = ?", id] )
  end

1) I could verify that id != nil, and that it's numeric.

2) I could also then verify that a record is found.

Is there anything else?

Are both #1 and #2 recommended practice? In both cases would you simply create a flash message with the error and display it, or is that giving away too much information?

+2  A: 

As I'm sure you know, this is just like People.find(id), except that find raises an error.

However, People.find_by_id(id) returns nil if no record is found, which I suspect takes care of all you need. You don't need to check that what you put into ActiveRecord is the correct data type and the like; it handles SQL injection risks, so to check ahead of time would not affect actual behavior.

If we're just looking at the show action, though, there's an even more elegant way: rather than using find_by_id and checking for nil, use find, let an error bubble up, and let the controller catch it with rescue_from. (By default, in production, ActiveRecord::RecordNotFound will be caught and rescued by showing a generic 404, but you can customize this behavior if necessary.)

class UsersController < ApplicationController
  rescue_from ActiveRecord::RecordNotFound, :with => :not_found

  def show
    @user = User.find params[:id]
  end

  protected
    def not_found
      flash[:error] = "User not found"
      redirect_to users_path
    end
end

Code untested, for illustrative purposes only ;)

Matchu
A: 

Don't do flash[:notice]'s insted just say that "Record not found"

The two things required by you can be done as follows:

1) I could verify that id != nil, and that it's numeric.

def self.get_record(id)
  People.first( :conditions => ["id = ?", id] ) if id.integer? unless id.blank?
end

2) I could also then verify that a record is found.

def self.get_record(id)
  @people = People.first( :conditions => ["id = ?", id] ) if id.integer? unless id.blank?
  flash[:notice] = @people.blank? # this will print true/false depending on value in @people
end

Hope it works for you. :D

Rohit