[Rails-core] proposal: deprecate save() in favor of save!()

2015-06-22 Thread pseidemann
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?

---

post transferred from https://github.com/rails/rails/issues/20662

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at http://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.


Re: [Rails-core] proposal: deprecate save() in favor of save!()

2015-06-22 Thread Rafael Mendonça França
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.
​


On Mon, Jun 22, 2015 at 11:24 AM pseidemann  wrote:

> 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?
>
> ---
>
> post transferred from https://github.com/rails/rails/issues/20662
>
> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-core+unsubscr...@googlegroups.com.
> To post to this group, send email to rubyonrails-core@googlegroups.com.
> Visit this group at http://groups.google.com/group/rubyonrails-core.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at http://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.


Re: [Rails-core] proposal: deprecate save() in favor of save!()

2015-06-22 Thread Ryan Bigg
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. 

 

> On 23 Jun 2015, at 00:27, Rafael Mendonça França  
> wrote:
> 
> 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.
> 
> 
>> On Mon, Jun 22, 2015 at 11:24 AM pseidemann  wrote:
>> 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?
>> 
>> ---
>> 
>> post transferred from https://github.com/rails/rails/issues/20662
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Ruby on Rails: Core" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to rubyonrails-core+unsubscr...@googlegroups.com.
>> To post to this group, send email to rubyonrails-core@googlegroups.com.
>> Visit this group at http://groups.google.com/group/rubyonrails-core.
>> For more options, visit https://groups.google.com/d/optout.
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to rubyonrails-core+unsubscr...@googlegroups.com.
> To post to this group, send email to rubyonrails-core@googlegroups.com.
> Visit this group at http://groups.google.com/group/rubyonrails-core.
> For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at http://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.


Re: [Rails-core] proposal: deprecate save() in favor of save!()

2015-06-22 Thread Michael Pavling
On 22 Jun 2015 15:24, "pseidemann"  wrote:
>
> 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.
>
> what do you think?

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.

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at http://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.


Re: [Rails-core] proposal: deprecate save() in favor of save!()

2015-06-23 Thread Jason Fleetwood-Boldt
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



> On Jun 22, 2015, at 5:39 PM, Ryan Bigg  wrote:
> 
> 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. 
> 
>  
> 
> On 23 Jun 2015, at 00:27, Rafael Mendonça França  > wrote:
> 
>> 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.
>> 
>> 
>> 
>> On Mon, Jun 22, 2015 at 11:24 AM pseidemann > > wrote:
>> 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?
>> 
>> ---
>> 
>> post transferred from https://github.com/rails/rails/issues/20662 
>> 
>> 
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Ruby on Rails: Core" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to rubyonrails-core+unsubscr...@googlegroups.com 
>> .
>> To post to this group, send email to rubyonrails-core@googlegroups.com 
>> .
>> Visit this group at http://groups.google.com/group/rubyonrails-core 
>> .
>> For more options, visit https://groups.google.com/d/optout 
>> .
>> 
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Ruby on Rails: Core" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to rubyonrails-core+unsubscr...@googlegroups.com 
>> .
>> To post to this group, send email to rubyonrails-core@googlegroups.com 
>> .
>> Visit this group at http://groups.google.com/group/rubyonrails-core 
>> .
>> For more options, visit https://groups.google.com/d/optout 
>> .
> 
> 
> -- 
> You received this message because you are subscribed to the Google Groups 
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an 
> email to rubyonrails-core+unsubscr...@googlegroups.com 
> .
> To post to this group, send email to rubyonrails-core@googlegroups.com 
> .
> Visit this group at http://groups.google.com/group/rubyonrails-core 
> .
> For more options, visit https://groups.google.com/d/optout 
> .

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at http://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.


Re: [Rails-core] proposal: deprecate save() in favor of save!()

2015-06-23 Thread Kerri Miller
OP - Would it clarify the issue in your mind if

"Saves the model."

were changed to

"Attempts to save the model."

-k-


On Tue, Jun 23, 2015 at 7:36 AM, Jason Fleetwood-Boldt  wrote:

> 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
>
>
>
> On Jun 22, 2015, at 5:39 PM, Ryan Bigg  wrote:
>
> 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.
>
>
>
> On 23 Jun 2015, at 00:27, Rafael Mendonça França 
> wrote:
>
> 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.
> ​
>
>
> On Mon, Jun 22, 2015 at 11:24 AM pseidemann  wrote:
>
>> 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?
>>
>> ---
>>
>> post transferred from https://github.com/rails/rails/issues/20662
>>
>> --
>> You received this message because you are subscribed to the Google Groups
>> "Ruby on Rails: Core" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to rubyonrails-core+unsubscr...@googlegroups.com.
>> To post to this group, send email to rubyonrails-core@googlegroups.com.
>> Visit this group at http://groups.google.com/group/rubyonrails-core.
>> For more options, visit https://groups.google.com/d/optout.
>>
>
> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-core+unsubscr...@googlegroups.com.
> To post to this group, send email to rubyonrails-core@googlegroups.com.
> Visit this group at http://groups.google.com/group/rubyonrails-core.
> For more options, visit https://groups.google.com/d/optout.
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-core+unsubscr...@googlegroups.com.
> To post to this group, send email to rubyonrails-core@googlegroups.com.
> Visit this group at http://groups.google.com/group/rubyonrails-core.
> For more options, visit https://groups.google.com/d/optout.
>
>
>  --
> You received this message because you are subscribed to the Google Groups
> "Ruby on Rails: Core" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to rubyonrails-core+unsubscr...@googlegroups.com.
> To post to this group, send email to rubyonrails-core@googlegroups.com.
> Visit this group at http://groups.google.com/group/rubyonrails-core.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit this group at http://groups.google.com/group/rubyonrails-core.
For more options, visit https://groups.google.com/d/optout.


Re: [Rails-core] proposal: deprecate save() in favor of save!()

2015-06-24 Thread pseidemann
yes I think changing the documentation in that direction already makes it 
really more clear

On Tuesday, June 23, 2015 at 4:56:23 PM UTC+2, Kerri Miller wrote:
>
> OP - Would it clarify the issue in your mind if 
>
> "Saves the model."
>
> were changed to
>
> "Attempts to save the model."
>
> -k-
>
>
> On Tue, Jun 23, 2015 at 7:36 AM, Jason Fleetwood-Boldt <
> te...@datatravels.com > wrote:
>
>> 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
>>
>>
>>
>> On Jun 22, 2015, at 5:39 PM, Ryan Bigg > 
>> wrote:
>>
>> 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. 
>>
>>  
>>
>> On 23 Jun 2015, at 00:27, Rafael Mendonça França > > wrote:
>>
>> 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.
>> ​
>>
>>
>> On Mon, Jun 22, 2015 at 11:24 AM pseidemann > > wrote:
>>
>>> 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?
>>>
>>> ---
>>>
>>> post transferred from https://github.com/rails/rails/issues/20662
>>>
>>> -- 
>>> You received this message because you are subscribed to the Google 
>>> Groups "Ruby on Rails: Core" group.
>>> To unsubscribe from this group and stop receiving emails from it, send 
>>> an email to rubyonrails-co...@googlegroups.com .
>>> To post to this group, send email to rubyonra...@googlegroups.com 
>>> .
>>> Visit this group at http://groups.google.com/group/rubyonrails-core.
>>> For more options, visit https://groups.google.com/d/optout.
>>>
>>
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Ruby on Rails: Core" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to rubyonrails-co...@googlegroups.com .
>> To post to this group, send email to rubyonra...@googlegroups.com 
>> .
>> Visit this group at http://groups.google.com/group/rubyonrails-core.
>> For more options, visit https://groups.google.com/d/optout.
>>
>>
>> -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Ruby on Rails: Core" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to rubyonrails-co...@googlegroups.com .
>> To post to this group, send email to rubyonra...@googlegroups.com 
>> .
>> Visit this group at http://groups.google.com/group/rubyonrails-core.
>> For more options, visit https://groups.google.com/d/optout.
>>
>>
>>  -- 
>> You received this message because you are subscribed to the Google Groups 
>> "Ruby on Rails: Core" group.
>> To unsubscribe from this group and stop receiving emails from it, send an 
>> email to rubyonrails-co...@googlegroups.com .
>> To post to this group, send email to rubyonra...@googlegroups.com 
>> .
>> Visit this group at http://groups.google.com/group/rubyonrails-core.
>> For more options, visit https://groups.google.com/d/optout.
>>
>
>

-- 
You received this message because you are subscribed to the Google Groups "Ruby 
on Rails: Core" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to rubyonrails-core+unsubscr...@googlegroups.com.
To post to this group, send email to rubyonrails-core@googlegroups.com.
Visit