Return render in Ruby on Rails

Question

Is it bad to do return render? I am doing this to NOT also allow a redirect.

def create
 @patient = Patient.new(params[:patient])
 if [email protected]
 @patient.user.password = ''
 @patient.user.password_confirmation = ''
 return render is_admin? ? 'new_admin' : 'new'
 else
 #Fire off an e-mail
 PatientMailer.welcome_email(@patient).deliver
 end
 if current_user == @patient
 sign_in @patient
 else
 redirect_to patients_path
 end
 end

Answer 1

Writing it like this makes it look the render returns a meaningful value that is then returned by create and used by the code that calls create, which is not the case. So instead I would write:

render is_admin? ? 'new_admin' : 'new'
return

This makes it clear that render is solely used for its side effects and create does not return a value (other than nil).

Answer 2

There are two sensible ways to write that function.

The first way, with the early return, treats the special case more like an error condition:

def create
 @patient = Patient.new(params[:patient])
 if [email protected]
 @patient.user.password = ''
 @patient.user.password_confirmation = ''
 render is_admin? ? 'new_admin' : 'new'
 return
 end
 # Fire off an e-mail
 PatientMailer.welcome_email(@patient).deliver
 if current_user == @patient
 sign_in @patient
 else
 redirect_to patients_path
 end
end

The other way outlines all the possible outcomes:

def create
 @patient = Patient.new(params[:patient])
 if [email protected]
 @patient.user.password = ''
 @patient.user.password_confirmation = ''
 render is_admin? ? 'new_admin' : 'new'
 else
 # Fire off an e-mail
 PatientMailer.welcome_email(@patient).deliver
 if current_user == @patient
 sign_in @patient
 else
 redirect_to patients_path
 end
 end
end

I personally have a slight preference for the first way, since the failure to save is more like an error. Your original code, which is a hybrid of the two, feels awkward.