Alain, to me and in this case, being verbose is a feature, not a bug. Back to your example, I don't think a commentable_id really belongs to the controller. Most of my other comments apply to your code too.
I think the load_commentable before filter used in this RailsCasts episode is somewhat brittle and, probably too complicated to be used as an example, especially when demonstrating this feature to new users.
I would rather go with a simpler alternative. Something like:
def load_commentable
@commentable = if params[:article_id]
Article.find(params[:article_id])
elsif params[:photo_id]
Photo.find(params[:photo_id])
elsif params[:event_id]
Event.find(params[:event_id])
end
end
Or even more verbose:
def load_commentable
if params[:article_id]
@commentable = Article.find(params[:article_id])
elsif params[:photo_id]
@commentable = Photo.find(params[:photo_id])
elsif params[:event_id]
@commentable = Event.find(params[:event_id])
end
end
I see several benefits in this kind of code:
Code is less clever, which is a good thing for controller code.
Filter code is decoupled from changes to routes. For example, if a scope or namespace was added or the path option was used to produce easier to read routes, you would not need to change filter code or tests.
Scopes can be added independently to each find call. Articles could be published and events active, without the need for aliases.
Code produces no temporary objects nor relies on string processing which, over time, add up to app execution time.
Tests are easier to follow and coverage easier to check.
I would also like to mention that, in real world, some of the most popular uses of polymorphic associations, like comments, are a bad pattern.
In this example, article comments will never be queried as event comments nor -most probably- listed together and, they even might end up having very different requirements. In Ruby, behavior can always be shared using modules.
Still, there are some valid uses for polymorphic associations but, way less than what one would think of first.
P.S. thanks so much for your work and contribution! I admit this is one of the very few cases where I don't agree with the code presented. :-)
I don't think that would be good practice. Some questions:
What about route path translation?
Why would you need a parameterized route when you won't be adding new models in runtime? It's just 3 cases, not infinite associations.
Lastly, this could potentially clash with other routes in your
routes.rb
file depending on your routes order.Alain, to me and in this case, being verbose is a feature, not a bug. Back to your example, I don't think a
commentable_id
really belongs to the controller. Most of my other comments apply to your code too.Ryan,
I think the
load_commentable
before filter used in this RailsCasts episode is somewhat brittle and, probably too complicated to be used as an example, especially when demonstrating this feature to new users.I would rather go with a simpler alternative. Something like:
Or even more verbose:
I see several benefits in this kind of code:
Code is less clever, which is a good thing for controller code.
Filter code is decoupled from changes to routes. For example, if a
scope
ornamespace
was added or thepath
option was used to produce easier to read routes, you would not need to change filter code or tests.Scopes can be added independently to each
find
call. Articles could bepublished
and eventsactive
, without the need for aliases.Code produces no temporary objects nor relies on string processing which, over time, add up to app execution time.
Tests are easier to follow and coverage easier to check.
I would also like to mention that, in real world, some of the most popular uses of polymorphic associations, like comments, are a bad pattern.
In this example, article comments will never be queried as event comments nor -most probably- listed together and, they even might end up having very different requirements. In Ruby, behavior can always be shared using modules.
Still, there are some valid uses for polymorphic associations but, way less than what one would think of first.
P.S. thanks so much for your work and contribution! I admit this is one of the very few cases where I don't agree with the code presented. :-)