def create
@brand = Brand.create()
@brand.title = params[:title]
@brand.upload_logo(params[:logo]) unless params[:logo].blank?
@brand.order = params[:order]
@brand.priority = params[:priority]
puts YAML::dump(@brand)
plataforms = Plataform.find(:all, :conditions => ["id IN (?)", params[:plataforms]])
@brand.plataforms = plataforms
params[:pt].each do |k, v|
I18n.locale = :pt
eval "@brand.#{k} = v"
end
params[:en].each do |k, v|
I18n.locale = :en
eval "@brand.#{k} = v"
end
respond_to do |format|
if @brand.save
# if 1 == 1
flash[:notice] = 'Brand was successfully created.'
format.html { redirect_to(@brand) }
format.xml { render :xml => @brand, :status => :created, :location => @brand }
else
format.html { render :action => "new" }
format.xml { render :xml => @brand.errors, :status => :unprocessable_entity }
end
end
end
Refactorings
No refactoring yet !
Aaron Scruggs
November 24, 2009, November 24, 2009 22:21, permalink
No rating.
Login to rate!
Does this code actually work as is?
If so, the low hanging fruit is to push alot of that code into the model and get rid of the "eval"
Perhaps something like this:
def create
respond_to do |format|
@brand = Brand.initialize_from_params(params)
if @brand.save
# if 1 == 1
flash[:notice] = 'Brand was successfully created.'
format.html { redirect_to(@brand) }
format.xml { render :xml => @brand, :status => :created, :location => @brand }
else
format.html { render :action => "new" }
format.xml { render :xml => @brand.errors, :status => :unprocessable_entity }
end
end
end
class Brand
def self.initialize_from_params(params)
brand = Brand.new #Brand.create() -- you should not create the model before adding the attributes it causes more database calls
brand.title = params[:title]
brand.upload_logo(params[:logo]) unless params[:logo].blank? #you should get rid of this as do the upload as a before_create callback
brand.order = params[:order]
brand.priority = params[:priority]
#puts YAML::dump(@brand) #this is unnecessary
brand.plataforms = Plataform.find(:all, :conditions => ["id IN (?)", params[:plataforms]]) #cut down to one line, this seems wrong, but I couldn't make a better suggestion without knowing more about the codebase
[:pt, :en].each do |local|
I18n.locale = local
brand.send(k,v)
end
end
end
I have a multilingual admin (for English and Portuguese) that needs to save data for both languages at the same time, like Description EN field and Description PT field in the same form.
Globalize2 makes some magic and I don't know exactly how to save this. I'll post my controller action here, that obviously needs some refactoring. Thanks, people!