variation = Variation.find(params[:vid1]) if params[:vid1] arp_id = variation.nil? ? @page.email_list : variation.arp_id
Refactorings
No refactoring yet !
s
April 13, 2011, April 13, 2011 17:40, permalink
If you really want one line, you can, but it seems like maybe there is more going on here than should be one-lined.
arp_id = (params[:vid1] && (variation = Variation.find(params[:vid1])).nil? ? @page.email_list : variation.arp_id -better maybe- variation = Variation.find_by_id(params[:vid1]) arp_id = variation.arp_id unless variation.nil? arp_id ||= @page.email_list
Adam
April 13, 2011, April 13, 2011 20:20, permalink
Variation.find_by_id(params[:vid1]).try(:arp_id) || @page.email_list
Josh
April 19, 2011, April 19, 2011 00:34, permalink
nice cleanup Adam, small adjustment: #find_by_id is the same as #find
Variation.find(params[:vid1]).try(:arp_id) || @page.email_list
Josh Nichols
April 25, 2011, April 25, 2011 20:42, permalink
You can get it down to one line, but it's probably more readable/intention revealing to expand it a little. I tend to favor long hand if/else rather than the ternary operator.
Also, assuming Variation is an ActiveRecord model, `find` either returns an object or railes RecordNotFound, so you don't need to check if it found something was returned
arp_id = if params[:vid1]
Variation.find(params[:vid1]).arp_id
else
@page.email_list
end
Josh Nichols
April 25, 2011, April 25, 2011 20:42, permalink
You can get it down to one line, but it's probably more readable/intention revealing to expand it a little. I tend to favor long hand if/else rather than the ternary operator.
Also, assuming Variation is an ActiveRecord model, `find` either returns an object or railes RecordNotFound, so you don't need to check if it found something was returned.
arp_id = if params[:vid1]
Variation.find(params[:vid1]).arp_id
else
@page.email_list
end
Josh Nichols
April 25, 2011, April 25, 2011 20:43, permalink
You can get it down to one line, but it's probably more readable/intention revealing to expand it a little. I tend to favor long hand if/else rather than the ternary operator.
Also, assuming Variation is an ActiveRecord model, `find` either returns an object or railes RecordNotFound, so you don't need to check if it found something was returned.
arp_id = if params[:vid1]
Variation.find(params[:vid1]).arp_id
else
@page.email_list
end
How can I make it in one line? or is there a better alternative