# two or more render/redirect in the same method

I often have method like this one with two or more render to do due to catching the error for example. I currently do something like this:

def update
@user = current_user
if params[:user][:camera_type].present? && params[:user][:lens_type].present? && params[:user][:photo].present?
r = Photo.instance.Upload(camera_type: @user.id,
album: @user.default_album,
photo: params[:user][:photo],
lens_type: params[:user][:lens_type],
camera_type: params[:user][:camera_type])
@user.camera_type = params[:user][:camera_type]
@user.lens_type = params[:user][:lens_type]
@user.photo = params[:user][:photo]
@user.save!
render action: 'edit'
return
end
rescue Photo => e
redirect_to photo_path, :flash => { :alert => t(:text_photo_edit_failed) }
end


But may be there is a better way to do it ?

• can you fix the indentation? – tokland Mar 15 '13 at 22:27
• @tokland Fixed it - was thinking about doing that earlier, but wasn't even sure I knew how. A lot going on there, none of it clear :/ – Flambino Mar 15 '13 at 23:37

Some notes:

• Keep your controllers lean, this is too much code for an action.
• The photo upload should be done as a part of the object's save chain (after validation), so create a before_save callback in the User model that returns false if the upload fails.
• You set r (cryptic name) but don't use it anywhere? does the upload raise an exception on error?

At the end the controller should look extremely simple, something like:

def update
@user = current_user

if @user.update_attributes(params[:user])
render action: 'edit'
else
redirect_to photo_path, flash: {alert: t(:text_photo_edit_failed)}
end
end


Any additional logic should be in User. Models are easy to test in isolation, controllers on the other hand have a lot of state to worry about.

• Agreed, most of this should be moved into models. There's also some weirdly suspect bits of code here suggesting you need to work out some kinks in your model layer, like Upload as a method name, or camera_type: @user.id. What is a user's ID doing there, being called a "camera_type"? You also generally shouldn't have to check for specific parameters to be present, rather you prevent models from being moved into an invalid state at the model level, via validations. – meagar Mar 15 '13 at 19:24
• @meagar camera_type: @user.id is the typo mistake. my bad. – bl0b Mar 15 '13 at 23:47