Ruby Saved My Life

In my Rails controllers, I have a lot of this:

 
@project = Project.find(params[:project])
if @project
  @project_product = @project.project_products.create(params[:project_product])
  if @project_product.valid?
   # Do something with success
  else
   # Do something with error
  end
else
  # Do something with error
end
 

The thing that I don't like about that style is that the controller is validating the creation of a ProjectProduct. There is the conditional " if @project " branch that basically says that the ProjectProduct requires a valid Project. I feel like the controller shouldn't be the one to determine that - this isn't a case of validating params (like if you preventing url hacking). Rails sort of even lets you know that you shouldn't be using that syntax because attaching the error to @project_product (which you would be sending back to the form as an error message), is impossible as @project_product hasn't even been initiated yet.

For these reasons I have no opted to do something like this:

(assuming ProjectProduct.rb validates the presence of :project, which it should - that's the entire point, that the model in this case should be requiring the existence of a project for ProjectProduct, not the controller)

 
@project = Project.find(params[:project])
@project_product = ProjectProduct.create(params[:project_product].merge(:project => @project))
if @project_product.valid?
  # Do something with success
else
  # Do something with error
end
 

Benefits to this:

1. Shorter Controller Method
2. One less conditional branch
3. Validation by Model and not Controller
4. No hacks to get Rails to generate error message for you.

A new best practice for myself.