Rails core - Apr 2009 - find_or_create_by_xxx!()

All,

There''s a bit of behavior I''d like to tweak around the
find_or_create_by methods to help make it more obvious when a
developer (especially newbies) screws up by forgetting to initialize
required fields.  Let''s say you have some code like this:

car = dealership.cars.find_or_create_by_model("corvette")
car.modelyear = 2003
car.save
dealership.save

and let''s say that Car looks like this:
class Car < ActiveRecord::Base
 validates_presence_of :modelyear
 belongs_to :dealership
end

At the moment (checked in ActiveRecord 2.2.2 and 2.3.2) the code above
will silently create your car, but won''t save it to the database and
won''t attach it to your dealership.  If you change line 1939 of
ActiveRecord::Base from this:
                    #{''record.save'' if instantiator ==
:create}
to this (note addition of bang):
                    #{''record.save!'' if instantiator ==
:create}
 then you get the much more helpful error message:
"Validation failed: Modelyear can''t be blank"
instead of a silent failure.

So, my proposal is that we change the behavior of find_or_create_by_xxx
() so that it can also be called with a bang like this:
find_or_create_by_xxx!() and will throw an exception if the save
fails.  This would be in line with the current behavior of find_by_xxx!
() that will throw an exception if it doesn''t find anything.

Yes, I know I could solve the problem by doing something like this:

car = dealership.cars.find_or_create_by_model("corvette")
raise(ValidationError) if car.new_record?

but that''s a lot of code for something that would only truly fail if I
screwed up in my original development.

I tried to put my proposed patch to ActiveRecord::Base.method_missing
in like this:
if match.bang?
       #{''record.save!'' if instantiator == :create}
else
       #{''record.save'' if instantiator == :create}
end

But that fails because "match" is not in scope?  Don''t know
because my
ruby foo isn''t quite strong enough to understand everything
that''s
going on in that method.





--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---

You should take a look into dynamic_finder_match.rb file too. In this
file you will see that they already check when a bang is sent,but not
in the find_or_create cases. You can refactor it a little bit to check
in any case.

Then you would have to change the method missing method this way:

          #{''record.save'' if instantiator == :create
&& !match.bang?}
          #{''record.save!'' if instantiator == :create
&& match.bang?}

The match is not in scope because you are inside the method inside the
class eval, that''s why you should put everything inside the
interpolation.

On Apr 3, 10:45 pm, trcull <trc...@kroppel.net>
wrote:
> All,
>
> There''s a bit of behavior I''d like to tweak around the
> find_or_create_by methods to help make it more obvious when a
> developer (especially newbies) screws up by forgetting to initialize
> required fields.  Let''s say you have some code like this:
>
> car = dealership.cars.find_or_create_by_model("corvette")
> car.modelyear = 2003
> car.save
> dealership.save
>
> and let''s say that Car looks like this:
> class Car < ActiveRecord::Base
>  validates_presence_of :modelyear
>  belongs_to :dealership
> end
>
> At the moment (checked in ActiveRecord 2.2.2 and 2.3.2) the code above
> will silently create your car, but won''t save it to the database
and
> won''t attach it to your dealership.  If you change line 1939 of
> ActiveRecord::Base from this:
>                     #{''record.save'' if instantiator ==
:create}
> to this (note addition of bang):
>                     #{''record.save!'' if instantiator ==
:create}
>  then you get the much more helpful error message:
> "Validation failed: Modelyear can''t be blank"
> instead of a silent failure.
>
> So, my proposal is that we change the behavior of find_or_create_by_xxx
> () so that it can also be called with a bang like this:
> find_or_create_by_xxx!() and will throw an exception if the save
> fails.  This would be in line with the current behavior of find_by_xxx!
> () that will throw an exception if it doesn''t find anything.
>
> Yes, I know I could solve the problem by doing something like this:
>
> car = dealership.cars.find_or_create_by_model("corvette")
> raise(ValidationError) if car.new_record?
>
> but that''s a lot of code for something that would only truly fail
if I
> screwed up in my original development.
>
> I tried to put my proposed patch to ActiveRecord::Base.method_missing
> in like this:
> if match.bang?
>        #{''record.save!'' if instantiator == :create}
> else
>        #{''record.save'' if instantiator == :create}
> end
>
> But that fails because "match" is not in scope?  Don''t
know because my
> ruby foo isn''t quite strong enough to understand everything
that''s
> going on in that method.
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"Ruby on Rails: Core" group.
To post to this group, send email to rubyonrails-core@googlegroups.com
To unsubscribe from this group, send email to
rubyonrails-core+unsubscribe@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/rubyonrails-core?hl=en
-~----------~----~----~----~------~----~------~--~---