I recently read Exceptional Ruby written by Avdi Grimm and, whilst I was generally impressed with it (it's worth a read, it made me think about things I do) there's one bit that I still struggle to approve of: using ActiveRecord::Base#save rather than ActiveRecord::Base#save!. It's not that I disagree with Avdi's sentiments, that exceptions should only be used in exceptional circumstances, but that ActiveRecord doesn't provide the middle ground that I think is needed.
My problem with save is that it relies on the developer calling it to check the return result which, from bitter experience, doesn't always happen.
What I want to see is something like this:
my_record.save do
# Something went wrong, clean up!
return
end
# Successful save, continue as normal
I'm suggesting that the definition of save could be modified like this (although I don't agree with alias_method_chain but that's a whole other story!):
class ActiveRecord::Base
def save_with_error_callback(&block)
yield unless save_without_error_callback
end
alias_method_chain(:save, :error_callback)
end
If the developer doesn't pass the block then the yield will raise an exception, otherwise the error handling block will be called.
I might be completely mad with this but, until there's a way to force checking of the return result from save, I'll always advocate save! in preference. What do people think?
Because of the dynamic nature of Ruby, and the very fact that I've just shown how to modify the behaviour of a method, the best thing a code checker could do is warn that the "return value" isn't being checked and, from bitter experience, people ignore that kind of stuff. ;)