something learned

What's new (and disappointing) in edge-rails

Posted by trevor Thu, 07 Jun 2007 05:24:56 GMT

Recently a changeset (6591) made its way into Rails’ trunk. A few people are blogging about how cool it is but I’m going to blog about why it’s bad.

So I tied an onion on my belt…

It actually all started with an earlier changeset (6588) which, in my opinion, made url helpers for nested resources just a twinge uglier. What 6588 does is clean up syntax in routes.rb:

# Before 6588
map.resources :houses do |house|
  house.resources :walls
end

# After 6588   
map.resources :houses, :has_many => :walls

No argument here – that’s a great idea. However, such a change poses a problem with polymorphic resources. For the uninitiated, the term “polymorphic resource” generally refers to a resource that appears in multiple routes.

When you re-use the same resource in multiple routes you have to disambiguate the url helpers using a :name_prefix argument.

# Before 6588
map.resources :houses do |house|
  house.resources :walls, :name_prefix => 'house_'
end

map.resources :outbuildings do |outbuilding|
  outbuilding.resources :walls, :name_prefix => 'outbuilding_'
end

So now you can call either house_wall_path(@house, @wall) or outbuilding_wall_path(@outbuilding, @wall).

Unfortunately there’s no easy way to get the shiny new :has_many syntax in routes.rb while still expressing the name_prefix. Changeset 6588 solves that by introducing some extra behavior.

Now, all your nested routes are automatically disambiguated with an implicit :name_prefix (unless you override it using the old-style block syntax and set :name_prefix to nil). Leaving you with:

# After 6588   
map.resources :houses, :has_many => :walls
map.resources :outbuildings, :has_many => :walls

Polymorphic goodness right? No, as it happens.

... as was the fashion at the time.

The problem here is that auto-name_prefix is imposing a rather significant tax.

For now let’s (conveniently) ignore the possibility of polymorphic resources. After all, I know that I have far fewer polymorphic resources in my routes.rb than I have simple plain-jane nested resources – I suspect that’s true for a lot of people.

Returning to the original example:

# Before 6588
map.resources :houses do |house|
  house.resources :walls
end
# wall_path(@house, @wall) #=> /houses/5/walls/6

# After 6588   
map.resources :houses, :has_many => :walls
# house_wall_path(@house, @wall) #=> /houses/5/walls/6

Did you see how the url helper for my wall just got longer? Maybe I’m the exception but I spend a lot more time typing out url helpers than I do typing out routes. And because I have far more non-polymorphic resources than polymorphic ones, I don’t need all this automatic url helper disambiguation.

The implicit :name_prefix is just adding cruft to plain old nested resources.

Now, to take the ferry cost a nickel…

It’s around about now that the resources should never be nested more than 1 level deep cargo-cult will see how I’m passing both @house and @wall in my arguments and start to get itchy trigger fingers.

Personally I think it’s a shame that “the amount of nesting you need is proportional to the ideas you’re expressing” isn’t nearly as catchy as “never more than 1 level deep” but there you go.

But what’s interesting is that the “1-layer” folks are doing something that looks like a polymorphic resource (the relationship being modeled isn’t polymorphic, the controller is just being re-used). Assume we just have :houses (and no :outbuildings):

# 1-layer person says:
map.resources :houses, :has_many => :walls
map.resources :walls, :has_many => :windows
map.resources :windows

# instead of just:
map.resources :houses do |house|
  house.resources :walls, :has_many => :windows
end

All of a sudden the url helpers for :walls and :windows need to be disambiguated.

From what I can tell, the benefits for the “1-layer” folks are a) shorter urls (regardless of how expressive they are?) and b) they avoid having to supply ‘parents’ in the arguments for the deeply nested url helpers.

And while I can’t help with short urls, some time ago I created a plugin from which I created a patch that makes a deeply-nested window_path(@house, @wall, @window) callable as window_path(@window). No need to explicitly supply parents, they will be inferred if you choose not to supply them.

By the way, if you think the above “1-layer” style is just fine, consider that a logged-in user only has permission to view or edit the data for their associated houses. So User.has_many :houses. What does the code look like in windows_controller.rb when ensuring the user should be looking at window 13 for example?

See, I’m not blindly a “1-layer” guy so I’d always have the house_id in the url – and my controllers would just do a current_user.houses.find(params[:house_id]). Pretty straight-forward in my opinion.

and in those days, nickels had pictures of bumblebees on ‘em…

So back to yesterday’s changeset (6591). This changes the guts of Rails’ polymorphic_url() method in ways that, on the surface, look pretty cool.

Again, for the uninitiated, polymorphic_url() is a method that used to be part of the simply_helpful plugin and has recently been factored into Rails’ trunk.

The reason why polymorphic_url() is cool is because it decides what url helper to invoke based on the class of the argument you pass and in the case of ActiveRecord objects, whether the object is a new_record? or not.

By the way, polymorphic_url() isn’t really about polymorphic resources, it’s more about “I have an object, call the relevant url helper please”.

polymorphic_url Something.new #=> somethings_path() #=> /somethings
polymorphic_url Something.find(:first) #=> something_path(your_first_record) #=> /somethings/1

Layered on top of that behavior, are url_for() and form_for(). The form_for() helper is especially handy because it will either use :get or :post as the method for your form depending on whether the object is a new_record? or not.

Now you can use just one form for your new and existing records.

form_for Something.new #=> /somethings, :method => :post
form_for Something.find(1) #=> /somethings/1, :method => :put

All very cool. What’s the problem then?

‘Give me five bees for a quarter,’ you’d say.

The problem is that deep in the bowels of polymorphic_url a new feature has arrived with changeset 6591. Now you can pass an array to form_for (you couldn’t before) to signify that you have a nested object. Under the hood, polymorphic_url will call the correct url helper according to current Rails convention.

# routes.rb
map.resources :houses, :has_many => :walls

# In a view:
form_for [@house, Wall.new] #=> house_walls_path(), :method => :post
form_for [@house, Wall.find(1) #=> house_wall_path(1), :method => :put

Again, in isolation that looks fine but note that it assumes the implicit :name_prefix values for my url helper names which, as I’ve already stated, are just noise when you’re not dealing with polymorphic resources.

So if I want to use form_for (specifically to have the benefit of the new_record? checking) while passing the parents as arguments, I have no choice but to use the name-prefixed form of url helpers – and that affects me whenever I just need a url.

Or I could take the stance that others are taking:

”Yeah, url helpers sure got ugly because of the auto-name_prefix thing but now just call url_for([@house, @wall]) and it’s much clearer”

Except it’s not.

• It’s opaque.
• And it’s incomplete (how about a url to preview a new record?).
• And the interface has that leaky bolted-on feeling (wrapping your objects in an Array to overcome the method signature of url_for? That’s cheap).
• Did I mention opaque?

Now where were we? Oh yeah…

What we have is a situation where truly-polymorphic resources (where the resource can have different parent types), of which I personally have comparatively few, and pseudo-polymorphic resources (as created by the “1-layer” folks), of which I have even fewer, are making life harder for non-polymorphic nested resources.

Perhaps I’m in the minority but I believe the auto-name_prefix feature that was introduced in 6588 should be killed-off before any more leaky layers are added on top of it.

And polymorphic resources should be treated as an edge-case. It should be “if you need it, put in the work” rather than “if you don’t need it, put in the work”.

Prior to changeset 6588, url helpers were clearer, simpler, and involved fewer keystrokes for the vast majority of cases.

The important thing was that I had an onion on my belt

Finally, if you do need polymorphic resources, your clue is in the name.

POLYMORPHIC. Using a monomorph (the same controller) to model something polymorphic seems a little bone-headed don’t you think?

The caboose sample app (blog entry) has an example of how I believe you should model polymorphic resources. resource_fu wires it all together but it has moved on a bit since then (and still lacks full documentation) so the sample-app is a bit out-of-date. When I get time I’ll correct both of those issues.