proposal: deprecate save() in favor of save!()

rubyonrails-core
1

hello,

currently in rails you have two methods to save a record: save and save!.

I think save is often used wrong because the return value is not always checked.

even the documentation is not very clear about the subtle different about the two methods. for save the first sentence is:

Saves the model.

it’s not clearly mentioned there that it will not save the model at all when validation fails. so developers will introduce bugs when not always checking for the return value e.g. in delayed job methods or in rake tasks (or maybe even in controllers).

my proposal would be to deprecate the usage of save and introduce a new method called try_save which has the same implementation as the old save method. this would make the implementation and the usage of the saving method more clear.

what do you think?

2

I think the documentation is very clear about this:

Saves the model.
If the model is new a record gets created in the database, otherwise the existing record gets updated.
By default, save always run validations. If any of them fail the action is cancelled and save returns false.

If it is not we should fix the documentation but for me it is not a good idea to rename save to try_save. The Rails conventions for bang methods are clear.

​

3

Changing this method’s name reeks of bikeshedding.

Any one with more than a day’s worth of Rails experience knows the difference between these two methods and how to use them.

In my experience, the return value for save is almost always checked.

If we deprecate save and then have only save!, you would be encouraging the use of exceptions as flow control and that’s well-known as a coding smell.

4

I think the simplest solution would be for you to suggest (or submit a PR for) a change to the documentation that you think does make the difference clear.

Changing the functionality of either call would be a Bad Thing.

5

Generally nearly all of the Jr. devs I work with know this, this is very basic to the early learning process of working with Rails.

Most code I see uses the if @foo.save … else … end style in almost all cases and does flow control based on the return value of save.

-Jason

6

OP - Would it clarify the issue in your mind if

“Saves the model.”

were changed to

“Attempts to save the model.”

-k-

7

yes I think changing the documentation in that direction already makes it really more clear