save! > save
Published almost 4 years ago

Thoughtbot folks have a great article on not expecting exceptions – save bang your head, active record will drive you mad. I’ll admit, just like the poster, I used to use save! in controllers to DRY my code. And have a global rescue_from in application.rb. But over the time, I changed the camp and now I’m fully in that “Don’t expect expectations” camp. Some things are more important that DRYing 3 lines of code.

But I’d want to take this a step further. When you’re not expecting something to fail, always use the methods that raise exceptions on failure.

So I strongly disagree with the poster on this :

I think ActiveRecord::Base#save! and ActiveRecord::Base.update_attributes! should be pulled from the public API

I would advocate just the opposite for certain cases. In many of the code reviews we’ve done via ActionRails, the following pattern was seen in many of the models :

def do_something
  self.foo = 'bar'
  save
end

def create_items
  names.each {|n| self.items.create :name => n }
end

In the snippets above, it’s not checking for cases where the save fails. And for good reasons that they’re not likely to fail as code is changing some very minor. But in these scenarios, a failure would be an unexpected situation. Hence you should always use save! or create!.

There could be easily be any unexpected reasons the above save could fail. And using save! protects you from those situations and help catch those minor programming mistakes early, which otherwise could prove to be very costly in terms of time/efforts. So the above code should really be :

def do_something
  self.foo = 'bar'
  save!
end

def create_items
  names.each {|n| self.items.create! :name => n }
end

However, if you’re using exceptions for flow control, this practise won’t always help you :

def create
  @user = User.create! params[:user]
  redirect_to @user
rescue ActiveRecord::RecordNotSaved
  flash[:notice] = 'Unable to create user'
  render :new
end

As this catches the exception ActiveRecord::RecordNotSaved, unexpected save! failures from your model methods/callbacks will get caught too. And hide the real error.

Moral of the story :

  • Don’t expect exceptions
  • Use methods throwing exceptions when you’re not expecting a failure. For example, everywhere you’re not checking if save or create fails when working with Active Record objects, always use save! and create! instead.