Over the weekend I had a chance to review the receipts controller of ExpenseLynx with Railsian extraordinaire and Gears Within Gears blogger Brian Guthrie. One of the glaring faults of this class was easily the manual mapping of form parameters to the Receipt constructor. Admittedly, I had only the vaguest of ideas on how to cut these hideous nine lines down in both the create and update methods.
Context
@receipt = Receipt.new(:purchase_date => params[:receipt][:purchase_date],
:total => params[:receipt][:total],
:store_id => Store.find_or_create_by_name(params[:receipt][:store_name]).id,
:expensable => params[:receipt][:expensable],
:expensed => params[:receipt][:expensed],
:note => params[:receipt][:note],
:user => current_user,
:participants => participants)
I knew from scaffolding that Rails was smart enough to take the parameters of a receipt form and build an object out of it, then save or update it.
I didn’t know what to do with my seemingly-unique requirements:
- Interacting with the store form is unnecessary
- Receipts require store names
- If the store name already exists, it should be tied to that store
- If the store doesn’t exist, the receipt form must create it
The result of conversing is that we can push this logic down into the receipt model, which really makes a lot of sense. A drawback that crosses my mind is now my receipt model knows how to create a store. Losing 16 lines in a controller, however, is an easy sell.
Downwards!
A setter is really all that’s needed on my model.
def store_name=(name)
self.store = Store.find_or_create_by_name(name)
end
Changes should always be tested, particularly with a dynamic language like Ruby. I’ve covered my bases with a couple of RSpec tests.
describe Receipt do
let(:user) { Factory :user }
describe "store relationship" do
it "should create a new store if one doesn't exist with a new receipt" do
Store.count.should == 0
Receipt.create(:purchase_date => 1.day.ago, :user => user, :total => 4.32, :store_name => "Delta")
Store.count.should == 1
end
it "should find a current store if it exists" do
Store.create(:name => "Ikea")
Receipt.create(:purchase_date => 1.day.ago, :user => user, :total => 9.23, :store_name => "Ikea")
Receipt.find(1).store.should == Store.find_by_name("Ikea")
end
end
# ...
end
Wow — that was easy. Now we can get rid of that
Store.find_or_create_by_name
business. Incidentally, :store_id
could have
always just been :store
.
@receipt = Receipt.new(:purchase_date => params[:receipt][:purchase_date],
:total => params[:receipt][:total],
:store => params[:receipt][:store_name],
:expensable => params[:receipt][:expensable],
:expensed => params[:receipt][:expensed],
:note => params[:receipt][:note],
:user => current_user,
:participants => participants)
The only special parameters left are current_user
, offered up by the devise
gem I’m using for authentication, and participants
, which is provided by a
service that will undoubtedly be covered in depth soon. Without further ado,
let’s merge.
@receipt = Receipt.new(params[:receipt].merge(:user => current_user, :participants => participants))
Just like that, I was able to commit a much cleaner controller.
Fun Files
ExpenseLynx is an open-source project I started during my last semester at Loyola University Chicago. Its aim is to provide receipt-based tracking of expenses and their corresponding metadata. With help from the Ruby community and ThoughtWorkers as far away as Melbourne, it’s quickly approaching release-quality.