Today, I was building a controller action that was polymorphically finding and associating an object. I could have used a simple dumb solution, such as this:
app/controllers/watches_controller.rb
1 class WatchesController < ApplicationController 2 def create 3 @watch = Watch.build(:watcher => current_person) 4 @watch.subject = case 5 when params[:person_id] 6 Person.find(params[:person_id]) 7 when params[:event_id] 8 Event.find(params[:event_id]) 9 else 10 raise ArgumentError, "Don't know how to handle other keys... #{params.keys.inspect}" 11 end 12 @watch.save! 13 flash[:notice] = "You're watching #{@watch.subject.name}" 14 redirect_to root_url 15 end 16 end
That sucked. Really bad. Why did I have to have a case/switch statement in my controller? Why not use a simpler alternative? I could have gone the full polymorphic route too:
app/controllers/watches_controller.rb
1 class WatchesController < ApplicationController 2 def create 3 @watch = Watch.build(:watcher => current_person) 4 5 # For illustration purposes, this is fine, but INSECURE!!! 6 @watch.subject_type = params[:subject_type] 7 @watch.subject_id = params[:subject_id] 8 @watch.save! 9 10 flash[:notice] = "You're watching #{@watch.subject.name}" 11 redirect_to root_url 12 end 13 end
Then, I remembered that my controller was already a ResourceController implementation. I opened up the code and used this instead:
app/controllers/watches_controller.rb
1 class WatchesController < ResourceController::Base 2 belongs_to :person, :event 3 4 create.before do 5 @watch.watcher = current_person 6 @watch.subject = parent_object 7 end 8 end
I needed to add a bit of infrastructure (some routes and has_many declarations in my models), but those were benefits (I know I’ll need them later anyway). My controller code is simpler, and the intention is clearer, I think.
In the same vein, are you following #standup on Twitter?
blog comments powered by Disqus
