I’ve been writing a lot of CRUDY tools lately. They all display a listing, allow you to delete an item, let you view an item, let you create an item, and let you edit an item. Standard Rails goodness. The thing is, all their ‘save’ methods are pretty different. Stuff needs to happen.
Creating a bunch of new.rhtml and edit.rhtml files wrapping a _form.rhtml partial got pretty old pretty fast. I decided I’d rather have new and edit wrap a save method and have a form.rhtml template which was capable of handling both actions.
First, the controller. This one’s for (you guessed) blog entries.
class EntriesController < ApplicationController def new; save end def edit; save end def save @entry = params[:id] ? Entry.find(params[:id]) : Entry.new if request.post? @entry.attributes = params[:post] return redirect_to :action => :list if @entry.save end render :action => :form end end
Like I said, new and edit just wrap the save action. The save method figures out if it needs to instantiate a new entry or an existing entry based on the presence of params[:id]. If it’s a post request, it sets the attributes of the Entry object and then does some specific pre-save setup.
There are a few cool things about setting up your create / update actions this way. One is that on validation failure you’ll keep the url you want. For instance, if your create fails because a title is not present you will be shown your error messages while still on /entries/new (rather than something like /entries/create which can futz up a refresh).
Of course, we don’t get much benefit on the views side if we’re already wrapping a _form.rhtml partial. But it sure is nice to have a clean controller, even for simple things.
Just a thought: Would it be better to just alias the new and edit to save so you don’t have to actually repeat the save in the two actions?
Re comment above: it would I think. Within your view you could then access the params directly to discover which action was called, and modify the layout accordingly.
With the present setup you can access controller.action_name in the view just fine, but aliasing does seem more Ruby-like. Thanks for the tip.
Shouldn’t the save method be private or protected to prevent it from being accessed as a public view method?
I’m not convinced. How is this better than having separate methods that each do the appropriate things with properly refactored common functionality? You are putting a conditional in the middle of the method to figure out which action you’re in, which is the reverse of what you want to do. You could certainly refactor out the common pieces like the first line into a separate method that you call from each action, but inverting the logic the way you have makes things more brittle and harder to maintain, not less. If you really want to do CRUD, check out the restful routes that just got checked into trunk (used to be SimplyRestful plugin).
josh: You’re right in that this does not fit nicely with SimplyRestful. But it is simple, and it is incredibly easy to maintain. After all, everything is in one place. It’s hard to make the case that splitting up 8 lines of code into multiple methods is more maintainable than centralizing it all—especially when what you’re doing is dirt simple. The fact that the code is somewhat reminiscent of respond_to helps as well: it’s a slightly different take on an already common idiom.
The thing about wrapping a partial is it gives you the ability to have different headings, buttons, etc depending on whether you are creating a new item or editing an existing one. I suppose you could set up instance variables in the new and edit methods for those things depending on how you feel about the particulars of separating controllers and views.
As for josh’s concern, I think this is refactored common functionality. It’s not as if there’s a huge if/else block—most of the functionality is shared. The whole ‘keeping URLs’ thing is somewhat of a red herring though; you can do that many different ways. Looking at some of my controllers, they could maybe use some refactoring, but in the grand scheme of things, they’re still so much better than any Java or PHP code I’ve written that I’m content to leave them and move on to the next feature implementation.
Chime in.