Permalink
5 comments
on commit
Please
sign in to comment.
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Browse files
Let the scaffold example use the "set shared record" pattern to expla…
…in callbacks
- Loading branch information
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
5 comments
on commit 339e4e8
There was a problem hiding this comment.
There was a problem hiding this comment.
Nice!
I've untangled my fair share of messy and/or flat out wrong lookup logic that got that way by being reimplemented in a ton of controller actions.
There was a problem hiding this comment.
I'm not a fan. An example where this actually makes things harder: http://intertwingly.net/projects/AWDwR4/checkdepot-187-32/section-10.2.html.
There was a problem hiding this comment.
before
# GET /carts/1
# GET /carts/1.json
def show
begin
@cart = Cart.find(params[:id])
rescue ActiveRecord::RecordNotFound
logger.error "Attempt to access invalid cart #{params[:id]}"
redirect_to store_url, :notice => 'Invalid cart'
else
respond_to do |format|
format.html # show.html.erb
format.json { render :json => @cart }
end
end
endafter
before_action :set_cart
def show
respond_to do |format|
format.html # show.html.erb
format.json { render :json => @cart }
end
end
private
def set_cart
unless @cart = Cart.find_by_id(params.require(:id))
logger.error "Attempt to access invalid cart #{params[:id]}"
redirect_to store_url, :notice => 'Invalid cart'
end
endIMO that's an improvement - keeps the action focused on crafting the response.
There was a problem hiding this comment.
Completely agree. Even though I would treat a missing cart as an exceptional state and use a regular find with a rescue instead. But that's a further optimization.
…On Dec 7, 2012, at 21:47, Jeremy Kemper ***@***.*** wrote:
before
# GET /carts/1
# GET /carts/1.json
def show
begin
@cart = Cart.find(params[:id])
rescue ActiveRecord::RecordNotFound
logger.error "Attempt to access invalid cart #{params[:id]}"
redirect_to store_url, :notice => 'Invalid cart'
else
respond_to do |format|
format.html # show.html.erb
format.json { render :json => @cart }
end
end
end
after
before_action :set_cart
def show
respond_to do |format|
format.html # show.html.erb
format.json { render :json => @cart }
end
end
private
def set_cart
unless @cart = Cart.find_by_id(params.require(:id))
logger.error "Attempt to access invalid cart #{params[:id]}"
redirect_to store_url, :notice => 'Invalid cart'
end
end
IMO that's an improvement - keeps the action focused on crafting the response.
—
Reply to this email directly or view it on GitHub.
Why the extra blank line?