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.
