Halaman

Sabtu, 11 Juni 2011

Rails general coding style

Code:

# somethings_controller.rb
def create
  @something = Something.new params[:something]
  @something.save!
  if @something.some_state?
    @something.initialize_for_some_state
  end
  redirect_to success_url
rescue
  render :action => "failed"
end

Hint:

  1. should avoid rescue without specifying the exception, in above code, use
    rescue ActiveRecord::RecordInvalid
  2. better yet, don't use any rescue at all, use save instead of save! with some if
  3. post processing should be done in callback.
    after_create {|something| something.initialize_for_some_state if something.some_state}

Better Code:

# somethings_controller.rb
def create
  @something = Something.new params[:something]
  if @something.save
    redirect_to success_url
  else
    render :action => "failed"
  end
end

Code:

# somethings_controller.rb
def show
  @something = Something.find(:first, :conditions => "user_id = #{current_user.id} AND id = #{params[:id]} AND name = #{params[:name]}")
  respond_to do |format|
    format.html
    format.js do
      render :js => "$('#container').html('#{escape_javascript(render :partial => "something", :object => @something)}');"
    end
    format.json do
      render :json => @something.to_json
    end
  end
end

Hint:

  1. SQL injection warning! never use data from params or from user input directly in sql code. use something like this instead:
    :conditions => ["user_id = #{current_user.id} AND id = ? AND name = ?", params[:id], params[:name]]
  2. using model relation and dynamic finder you can avoid the conditions entirely
    @something = current_user.somethings.find_by_id_and_name params[:id], params[:name]
  3. you should check if nothing found (@something is nil)
  4. this is just my opinion, but in most case i'll avoid using respond_to, instead i'll use 3 different view files: show.html.erb, show.js.erb, show.json.erb. rails will automatically use the correct view.

Better Code:

# somethings_controller.rb
def show
  @something = current_user.somethings.find_by_id_and_name params[:id], params[:name]
  render_invalid "not found" if @something.nil? # read about render_invalid in http://tech.maysora.com/2010/12/rails-renderinvalid-method.html
end


to be continued about model

Tidak ada komentar:

Posting Komentar