#accepts_nested_attributes_for (often) considered harmful
TL;DR: #accepts_nested_attributes_for is not evil but it should be used infrequently. It often results in brittle code. Consider using the redtape gem (which offers an implementation of the “Form Object” pattern), instead.
As a long time #accepts_nested_attributes_for critic, I was excited when Bryan Helmkamp introduced us to the idea of the “Form obect” in his 7 Patterns to Refactor Fat ActiveRecord Models blog post.
#accepts_nested_attributes_for is used, in ActiveRecord classes, to reduce the amount of code in Rails applications needed to create/update records across multiple tables with a single HTTP POST/PUT. As with many things Rails, this is convention-driven: the ActiveRecord classes expect to receive their POST/PUT parameter according to specific naming conventions used solely for nested data.
While this sometimes results in less code, it often results in brittle code.
In order to benefit from this coupling, we now have to:
- Write forms/HTTP clients that will send the nested data expected by the model
- Have the receiving controller action simply passes the data through to the correct “top-level” model’s #new method.
- Not require additional massaging of the HTTP params otherwise we’re just writing more code which is what #accepts_nested_attributes_for is trying so hard to avoid.
In my experience, use of nested forms often requires at least some massaging of the data if not outright removal of #accepts_nested_attributes_for at a later time in favor of handling the data manually within the controller action.
That is, it’s brittle.
So let’s go on a tangent: “best practices”.
“Best practices” are dangerous. Few, if any, so-called “best practices” are universally best. They’re perhaps best in a particular context. Yet developers often treat them as panaceas (a discussion for another day…).
So let’s put this in context:
Only use #accepts_nested_attributes_for if all of these conditions are met:
- The form/API data requires no pre-processing prior to hand-off to your ActiveRecord::Base’s #new method.
- (If you have a UI,) The UI fields can map, one-to-one, to the model fields; You know that you can afford to tightly couple the view and model so that they vary codependently on one another.
Consider using redtape if your input data requires pre-processing prior to hand-off to the model. Obviously, you can do this in a private/protected method of your controller as well.
Ultimately, the goal is to have working, maintainable code that satisfies your needs with minimal effort. Choose the simplest path to arrive there. Consider your options and weight them accordingly.
1: I know what some devs out there are thinking: “But, Evan, SRP says I should always allow them to vary independently.” Yeah, well, sometimes you should break the rules but that should be the exception and not the norm. Not that SRP is a “rule” so much as a “guideline”. Pretty much the only “rule” of software development is “thy program should execute deterministically”.
UPDATE 1: I had considered making this argument in my Frustration Driven Development talk but, then, I didn’t have a solution at the time. I hate providing a criticism without presenting an alternative…
UPDATE 2: Redtape::Form includes ActiveModel::Validations. You can add validations to your Redtape::Form just as you would an ActiveRecord::Base subclass. Beware: Don’t duplicate validations from your model in your Redtape::Form. Instead, add validations if your form inputs are raw values requiring processing before being used in your model. Your form can then catch these errors early and provide contextually useful error messages.
Posted by evan on Wednesday, November 07, 2012blog comments powered by Disqus