I''m sure it''s deliberate that after_initialize gets called
after an
object is instantiated from the database, but I''m equally sure that
it''s a bad idea - if nothing else, initialize doesn''t get
called
anywhere in the instantiate method chain.
More importantly though, it''s a huge performance hit for any
programmer who wants to do something like setting defaults on their
object before it gets written to the database, or even checked for
validity. At present, the only way to get callback behaviour that
triggers only when an object is first made is to do something like:
def after_initialize
if new_record?
do_stuff
end
end
But now she is paying the cost of a method call not only when an
object is initialized (which is fine, because the method actually
_does_) something, but also every time objects of that class are
pulled from the database (an eventuality which is already handled by
the after_find callback.
Being strict and only calling after_initialize from
initialize_with_callbacks and leaving instantiate_with_callbacks only
calling after_find can only make after_initialize more useful. If
someone really does need behaviour common to both points in an
object''s lifecycle, it''s easy enough to do:
after_find :common_behaviour
after_initialize :common_behaviour
and god is in his heaven and all is right with the world.
If the cost of changing the API in this way is deemed too high, please
at least consider adding some callback that really is only called
after initialization and not after instantiation. It''s too useful a
place to ignore.
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
> More importantly though, it''s a huge performance hit for any > programmer who wants to do something like setting defaults on their > object before it gets written to the database, or even checked for > validity.We have before_validation for this case.> If the cost of changing the API in this way is deemed too high, please > at least consider adding some callback that really is only called > after initialization and not after instantiation. It''s too useful a > place to ignore.Taking the case of 1: @customer = Customer.new(params[:customer]) 2: @customer.do_something 3: @customer.save! What we currently don''t have is a hook that will fire before line 2, but not when retrieving from a database. Providing defaults before validation was a common case, and I believe that''s where the before_validation hook came from. What other use cases do you have in mind? The name after_initialize is probably a little misleading because it fires even when an object is retrieved from the database. Strictly speaking this is ''initialization'', but from a user''s perspective it''s ''finding'' or ''retrieval''. Perhaps at the very least we could give it a less confusing, more hazardous sounding name. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 22/07/07, Michael Koziarski <michael@koziarski.com> wrote:> > > More importantly though, it''s a huge performance hit for any > > programmer who wants to do something like setting defaults on their > > object before it gets written to the database, or even checked for > > validity. > > We have before_validation for this case.No, you don''t. class Foo < ActiveRecord::Base def before_validate self.attrib_with_default ||= 99 end end p Foo.new.attrib_with_default # nil The default doesn''t get set until the object is about to be validated, but sometimes you need that default to be set correctly so that you can use an object before it''s either validated or saved. And are you really suggesting that everyone should call ''valid?'' on their objects if they want them to get their correct defaults set? Default setting belongs in the initialization phase, not in the validation phase.> > If the cost of changing the API in this way is deemed too high, please > > at least consider adding some callback that really is only called > > after initialization and not after instantiation. It''s too useful a > > place to ignore. > > Taking the case of > > 1: @customer = Customer.new(params[:customer]) > 2: @customer.do_something > 3: @customer.save! > > What we currently don''t have is a hook that will fire before line 2, > but not when retrieving from a database. Providing defaults before > validation was a common case, and I believe that''s where the > before_validation hook came from. What other use cases do you have > in mind?Well, the obvious case is the case where you''re implementing new in your controller. The typical boiler plate goes: def new @customer = Customer.new end That @customer is never going to be validated, but it does make sense for it to have its defaults (which belong in the model and not the controller or the view) set correctly so that ''new.rhtml'' doesn''t need any knowledge of any default values when rendering the form.> The name after_initialize is probably a little misleading because it > fires even when an object is retrieved from the database. Strictly > speaking this is ''initialization'', but from a user''s perspective it''s > ''finding'' or ''retrieval''. Perhaps at the very least we could give it > a less confusing, more hazardous sounding name.But ''after_initialize'' implies "call this after you call the instance''s initialize method", and it''s apparent that, in the case of object retrieval, the initialize method simply isn''t called (which makes sense, because the object is only initialized once, when it got created by calling its class''s new 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 -~----------~----~----~----~------~----~------~--~---
On 7/22/07, Piers Cawley <pdcawley@bofh.org.uk> wrote:> def new > @customer = Customer.new > end > > That @customer is never going to be validated, but it does make sense > for it to have its defaults (which belong in the model and not the > controller or the view) set correctly so that ''new.rhtml'' doesn''t need > any knowledge of any default values when rendering the form.Really hear you on that one. If we need to deal with default values in a better way, why not something like... class Foo < ActiveRecord::Base default :attrib, :to => 1 default :another_attrib, :to => :another_attrib_defaulter protected def another_attrib_defaulter # ... end end The second parameter being a hash for some readability and potentially for some more features... Thoughts? --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On Jul 22, 2007, at 1:14 PM, "Damian Janowski" <damian.janowski@gmail.com > wrote:> > On 7/22/07, Piers Cawley <pdcawley@bofh.org.uk> wrote: >> def new >> @customer = Customer.new >> end >> >> That @customer is never going to be validated, but it does make sense >> for it to have its defaults (which belong in the model and not the >> controller or the view) set correctly so that ''new.rhtml'' doesn''t >> need >> any knowledge of any default values when rendering the form. > > Really hear you on that one. > > If we need to deal with default values in a better way, why not > something like... > > class Foo < ActiveRecord::Base > default :attrib, :to => 1 > default :another_attrib, :to => :another_attrib_defaulter > > protected > def another_attrib_defaulter > # ... > end > end > > The second parameter being a hash for some readability and potentially > for some more features... > > Thoughts?Does default belong in the schema as it is essentially a db functionality? Or is there a case for more complex conditional logic? In the latter case, what about using an overloaded attr reader instead? courtenay --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 22/07/07, court3nay <court3nay@gmail.com> wrote:> > > On Jul 22, 2007, at 1:14 PM, "Damian Janowski" <damian.janowski@gmail.com > > wrote: > > > > > On 7/22/07, Piers Cawley <pdcawley@bofh.org.uk> wrote: > >> def new > >> @customer = Customer.new > >> end > >> > >> That @customer is never going to be validated, but it does make sense > >> for it to have its defaults (which belong in the model and not the > >> controller or the view) set correctly so that ''new.rhtml'' doesn''t > >> need > >> any knowledge of any default values when rendering the form. > > > > Really hear you on that one. > > > > If we need to deal with default values in a better way, why not > > something like... > > > > class Foo < ActiveRecord::Base > > default :attrib, :to => 1 > > default :another_attrib, :to => :another_attrib_defaulter > > > > protected > > def another_attrib_defaulter > > # ... > > end > > end > > > > The second parameter being a hash for some readability and potentially > > for some more features... > > > > Thoughts? > > Does default belong in the schema as it is essentially a db > functionality?No it isn''t. Consider the use case of the newly created ''template'' object that''s used by new.rhtml in some generic controller; this never goes near the database, but still needs to have its defaults correctly set. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On Jul 22, 2007, at 2:03 PM, "Piers Cawley" <pdcawley@bofh.org.uk> wrote:> > On 22/07/07, court3nay <court3nay@gmail.com> wrote: >> >> >> On Jul 22, 2007, at 1:14 PM, "Damian Janowski" <damian.janowski@gmail.com >>> wrote: >> >>> >>> On 7/22/07, Piers Cawley <pdcawley@bofh.org.uk> wrote: >>>> def new >>>> @customer = Customer.new >>>> end >>>> >>>> That @customer is never going to be validated, but it does make >>>> sense >>>> for it to have its defaults (which belong in the model and not the >>>> controller or the view) set correctly so that ''new.rhtml'' doesn''t >>>> need >>>> any knowledge of any default values when rendering the form. >>> >>> Really hear you on that one. >>> >>> If we need to deal with default values in a better way, why not >>> something like... >>> >>> class Foo < ActiveRecord::Base >>> default :attrib, :to => 1 >>> default :another_attrib, :to => :another_attrib_defaulter >>> >>> protected >>> def another_attrib_defaulter >>> # ... >>> end >>> end >>> >>> The second parameter being a hash for some readability and >>> potentially >>> for some more features... >>> >>> Thoughts? >> >> Does default belong in the schema as it is essentially a db >> functionality? > > No it isn''t. Consider the use case of the newly created ''template'' > object that''s used by new.rhtml in some generic controller; this never > goes near the database, but still needs to have its defaults correctly > set. >Right, that''s the point i was missing.. but couldn''t you do that as an overloaded reader? def name read_attribute[:name] || "default name" end I do like the idea of wrapping that functionality in your syntax as above, makes it a little quicker courtenay> >--~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> The default doesn''t get set until the object is about to be validated, > but sometimes you need that default to be set correctly so that you > can use an object before it''s either validated or saved. And are you > really suggesting that everyone should call ''valid?'' on their objects > if they want them to get their correct defaults set? Default setting > belongs in the initialization phase, not in the validation phase.You''ll note that this is line two mentioned below. Your original email cited "before checked for validity", and before_validation does that. Either way, we''re talking past one another here.> But ''after_initialize'' implies "call this after you call the > instance''s initialize method", and it''s apparent that, in the case of > object retrieval, the initialize method simply isn''t called (which > makes sense, because the object is only initialized once, when it got > created by calling its class''s new method.)This thin and literal interpretation of the word ''initialize'' makes sense only to those who care deeply about the implementation of AR. If we were to refactor Base.instantiate(record) to use .new, I''m not sure that many users would care, nor would they expect different callbacks to be called. I''m not rejecting the need for defaults beyond what the schema provides, merely suggesting that there''s probably a better name out there for the callbacks to achieve that behaviour. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> In the latter case, what about using an overloaded attr reader instead?you can use this in some situations, but if you have interdependent defaults for three or four fields it can get really confusing. Better to have that logic put in one common place, and a callback is probably the right place for it. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> > But ''after_initialize'' implies "call this after you call the > > instance''s initialize method", and it''s apparent that, in the case of > > object retrieval, the initialize method simply isn''t called (which > > makes sense, because the object is only initialized once, when it got > > created by calling its class''s new method.) > > This thin and literal interpretation of the word ''initialize'' makes > sense only to those who care deeply about the implementation of AR. > If we were to refactor Base.instantiate(record) to use .new, I''m not > sure that many users would care, nor would they expect different > callbacks to be called.I really do have to take issue with this; I would argue that the current behaviour of after_initialize is based on a ''thin and literal interpretation of the word "initialize"''. Decanting an object from storage is not initialization; initialization is what happens when an object is first created, back before it got poured into the storage jar. Consider a woollen gansey''s life cycle. It starts off as a length of wool which is then knitted (initialized) on circular needles into a delightful seemless garment. During cold weather, I wear it a lot. During warm weather, I put it in a drawer (storage). When it gets cold again, I don''t take the gansey out of the drawer and knit it again from scratch. Conceptually, that''s what''s happening with my active record object. It''s knitted, shoved into the drawer and pulled out again as needed. It only gets knitted once. Of course, what _really_ happens is that my ''gansey'' is stored in the teleporter''s pattern buffers (okay, if you insist, we''ll call it a database) and reconstituted from an entirely new set of atoms when I need it later. But that doesn''t matter. As far as the wearer is concerned, it''s the same gansey. Plus, there''s the argument from orthogonality... If after_initialize only fires when the gansey is first knitted, then I can add behaviour that fires after knitting and drawer removal by doing: after_find :do_something after_initialize :do_something But when after_initialize fires after find as well, I have to write after_initialize do |obj| if obj.new_record? ... end end and pay the cost of the method call every time I get a gansey out of the drawer anyway. No fun. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> I really do have to take issue with this; I would argue that the > current behaviour of after_initialize is based on a ''thin and literal > interpretation of the word "initialize"''. Decanting an object from > storage is not initialization; initialization is what happens when an > object is first created, back before it got poured into the storage > jar.I can buy your argument, but if I do so, there doesn''t seem to be a need for after_initialize: def initialize(attrs) do_stuff super end All we''d be doing is reinventing an initialize method which hid the arguments. Doesn''t seem particularly necessary, especially at the cost of backwards compatibility for the people who use after_initialize at present. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 22/07/07, Michael Koziarski <michael@koziarski.com> wrote:> > > I really do have to take issue with this; I would argue that the > > current behaviour of after_initialize is based on a ''thin and literal > > interpretation of the word "initialize"''. Decanting an object from > > storage is not initialization; initialization is what happens when an > > object is first created, back before it got poured into the storage > > jar. > > I can buy your argument, but if I do so, there doesn''t seem to be a > need for after_initialize: > > def initialize(attrs) > do_stuff > super > end > > All we''d be doing is reinventing an initialize method which hid the > arguments. Doesn''t seem particularly necessary, especially at the > cost of backwards compatibility for the people who use > after_initialize at present.Have you actually tried that? I have. It''s a nightmare. It''s long enough ago now that I can''t remember exactly what the nightmare was, but it was definitely no fun at all. A cursory examination ActiveRecord::Base suggests that it''s because the various attributes don''t work until after ''@attributes attributes_from_column_definition'' has been evaluated, and that gets evaluated when you call @super. I accept that there are people who rely on the current behaviour of after_initialize, so the trick will probably be to come up with a good name for the callback. ''after_new''? Another option might be to turn ActiveRecord::Base#initialize into a template method along the lines of: def initialize(attributes = nil) @attributes = attributes_from_column_definition @new_record = true ensure_proper_type initialize_defaults if self.respond_to?(:initialize_defaults) self.attributes = attributes unless attributes.nil? yield self if block_given? end which has the advantage of making the callback after the bones of the object are in place, but before any @attributes are assigned to. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
I wrote a plugin a while ago that does exactly that: http://svn.viney.net.nz/things/rails/plugins/active_record_defaults/ -Jonathan. On 7/23/07, Damian Janowski <damian.janowski@gmail.com> wrote:> > > On 7/22/07, Piers Cawley <pdcawley@bofh.org.uk> wrote: > > def new > > @customer = Customer.new > > end > > > > That @customer is never going to be validated, but it does make sense > > for it to have its defaults (which belong in the model and not the > > controller or the view) set correctly so that ''new.rhtml'' doesn''t need > > any knowledge of any default values when rendering the form. > > Really hear you on that one. > > If we need to deal with default values in a better way, why not > something like... > > class Foo < ActiveRecord::Base > default :attrib, :to => 1 > default :another_attrib, :to => :another_attrib_defaulter > > protected > def another_attrib_defaulter > # ... > end > end > > The second parameter being a hash for some readability and potentially > for some more features... > > Thoughts? > > > >--~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 22/07/07, Piers Cawley <pdcawley@bofh.org.uk> wrote:> On 22/07/07, Michael Koziarski <michael@koziarski.com> wrote: > > def initialize(attrs) > > do_stuff > > super > > end> Have you actually tried that? I have. It''s a nightmare. It''s long > enough ago now that I can''t remember exactly what the nightmare was, > but it was definitely no fun at all. > > A cursory examination ActiveRecord::Base suggests that it''s because > the various attributes don''t work until after ''@attributes > attributes_from_column_definition'' has been evaluated, and that gets > evaluated when you call @super.If you want the equivalent of after_initialize, wouldn''t you want to do_stuff after calling super? In any case, if you''re feeling funky, you can do: def initialize(attrs = {}, &block) attrs[:funky_default] ||= ''my funky default'' super end But I much prefer: def initialize(attrs = {}, &block) super self.funky_default ||= ''my funky default'' end Tom --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> If you want the equivalent of after_initialize, wouldn''t you want to > do_stuff after calling super? In any case, if you''re feeling funky, > you can do: > > def initialize(attrs = {}, &block) > attrs[:funky_default] ||= ''my funky default'' > super > end > > But I much prefer: > > def initialize(attrs = {}, &block) > super > self.funky_default ||= ''my funky default'' > endSorry, that''s what I meant, if you do stuff before @attributes is initialized you''ll get some nasty surprises. Without cases which can''t be solved by this pattern, I''m not sure that adding a new callback is justified. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Michael Koziarski wrote:>> If you want the equivalent of after_initialize, wouldn''t you want to >> do_stuff after calling super? In any case, if you''re feeling funky, >> you can do: >> >> def initialize(attrs = {}, &block) >> attrs[:funky_default] ||= ''my funky default'' >> super >> end >> >> But I much prefer: >> >> def initialize(attrs = {}, &block) >> super >> self.funky_default ||= ''my funky default'' >> end > > Sorry, that''s what I meant, if you do stuff before @attributes is > initialized you''ll get some nasty surprises. Without cases which > can''t be solved by this pattern, I''m not sure that adding a new > callback is justified. >Sorry but I must respectfully disagree. This issue caused me plenty of confusion when I started out with rails, I''ve seen lots of other people being confused about it, and you even admit here that if you don''t do it right you''ll get "nasty surprises". Any good framework should try to avoid the potential for nasty surprises where possible. Now, Jonathan''s plugin mentioned earlier in this thread sounds like a pretty good solution, but I had never heard of it until now. Maybe core could consider adopting that. But, I would say that setting default values for model objects is an *extremely* common practice... needed on just about every project. It seems like one way or another it would be nice to help rails developers avoid some gotchas with something that''s completely straightforward in most other languages/frameworks. Ben --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> But, I would say that setting default values for model objects is an *extremely* common practice... > needed on just about every project.Just wondering how did you achieve it previous to hearing about Jonathan''s plugin ? Why add a new callback when all you need is 4 lines of documentation on how to do it ? -- Cheers! - Pratik http://m.onkey.org --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Pratik wrote:>> But, I would say that setting default values for model objects is an *extremely* common practice... >> needed on just about every project. > > Just wondering how did you achieve it previous to hearing about > Jonathan''s plugin ? > > Why add a new callback when all you need is 4 lines of documentation > on how to do it ? >I do the after_initialize with an "if new_record?" in it... got that from a great post by Wes Gamble a while back where he outlined all the gotchas with setting default values. It was actually my impression from that post that one could *not* override initialize. So, my point really is that needing to override after_initialize, but remember to put the check for a new_record in there (plus that fact that it''s run on every load) is silly... and counter-intuitive. But, if overriding initialize is really completely safe and won''t cause lots of heartache then fine. I''m not one to stampede to adding stuff to the framework. I guess the question is, how bad a deal is it for people who miss the four lines of documentation versus how much of a change is to to provide a simple, obvious way to set model defaults. This is just one of those things that I need to explain to rails newcomers that seems silly and a bit embarrassing. Anyway, I think I''ll definitely check out Jonathan''s plugin on my next app... looks all nice and "DSL-ish". :-) Ben --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 24/07/07, Ben Munat <bmunat@gmail.com> wrote:> Anyway, I think I''ll definitely check out Jonathan''s plugin on my next app... looks all nice and > "DSL-ish". :-)The problem with the plugin approach here is that it''s monkeypatching a pretty fundamental method, and undocumented, method. And that makes it fragile; if the implementation of initialize ever changes (and there''s no reason that it shouldn''t), the plugin breaks. Still, if it''s the only game in town... --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Piers Cawley wrote:> On 24/07/07, Ben Munat <bmunat@gmail.com> wrote: >> Anyway, I think I''ll definitely check out Jonathan''s plugin on my next app... looks all nice and >> "DSL-ish". :-) > > The problem with the plugin approach here is that it''s monkeypatching > a pretty fundamental method, and undocumented, method. And that makes > it fragile; if the implementation of initialize ever changes (and > there''s no reason that it shouldn''t), the plugin breaks. > > Still, if it''s the only game in town... >Hmm, yeah that''s a good point... always the risk with monkeypatching. So, Piers, you''re the one that started this whole thing... if it really is safe to do: class MyModel < ActiveRecord::Base def initialize(attrs = {}, &block) super # all my initialization stuff here end end and this is documented in the AR::Base docs, then this does kind of seem like enough. I mean, it''s what I did all the time in my java life. I''d just swear that someone told me the holy beasts of doom would swarm down upon me if I ever tried to override AR::Base#initialize. Ben --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
I would personally do it as something like :
def initialize_with_defaults(attributes = nil, &block)
initialize_without_defaults(attributes) do
self.some_attribute = ''whatever'' # Setting default value
yield self if block_given?
end
end
alias_method_chain :initialize, :defaults
Looks like it''s working quite well.
Thanks,
Pratik
On Jul 24, 8:28 am, Ben Munat <bmu...@gmail.com>
wrote:> Piers Cawley wrote:
> > On 24/07/07, Ben Munat <bmu...@gmail.com> wrote:
> >> Anyway, I think I''ll definitely check out
Jonathan''s plugin on my next app... looks all nice and
> >> "DSL-ish". :-)
>
> > The problem with the plugin approach here is that it''s
monkeypatching
> > a pretty fundamental method, and undocumented, method. And that makes
> > it fragile; if the implementation of initialize ever changes (and
> > there''s no reason that it shouldn''t), the plugin
breaks.
>
> > Still, if it''s the only game in town...
>
> Hmm, yeah that''s a good point... always the risk with
monkeypatching.
>
> So, Piers, you''re the one that started this whole thing... if it
really is safe to do:
>
> class MyModel < ActiveRecord::Base
> def initialize(attrs = {}, &block)
> super
> # all my initialization stuff here
> end
> end
>
> and this is documented in the AR::Base docs, then this does kind of seem
like enough. I mean, it''s
> what I did all the time in my java life.
>
> I''d just swear that someone told me the holy beasts of doom would
swarm down upon me if I ever tried
> to override AR::Base#initialize.
>
> Ben
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
On 24/07/07, Ben Munat <bmunat@gmail.com> wrote:> So, Piers, you''re the one that started this whole thing... if it really is safe to do: > > class MyModel < ActiveRecord::Base > def initialize(attrs = {}, &block) > super > # all my initialization stuff here > end > end > > and this is documented in the AR::Base docs, then this does kind of seem like enough. I mean, it''s > what I did all the time in my java life. > > I''d just swear that someone told me the holy beasts of doom would swarm down upon me if I ever tried > to override AR::Base#initialize.Yeah, I think I''ve heard the Holy Beasts of Doom argument as well, I just can''t for the life of me remember where. I have certainly been bitten by trying to set the defaults before calling super though (because, dammit, that''s the logical place to set defaults). Thinking about it, the way to do that would be something like: def initialize(attrs = {}, &block) super attrs.reverse_merge(:default => ''this''), &block end but then you make yourself a hostage to fortune that everyone''s going to use symbols as keys. The post super approach to setting defaults has some edge case problems too in cases where nil is a legal value for some attribute (yeah, it''s a weird edge case, but an edge case all the same). You end up with ugly code like: def initialize(attrs = {}, &block) super unless attrs.has_key?(:content) || attrs.has_key?(''content'') self.content = "Write something here" end end The issue is, I think, that ActiveRecord::Base#initialize is doing two different ''sorts'' of things: class invariant metadata initialization, and instance specific initialization. You might compose the method as: def initialize(attributes = nil, &block) initialize_metadata initialize_instance(attributes, &block) end def initialize_metadata @attributes = attributes_from_column_definition @new_record = true ensure_proper_type end def initialize_instance(attributes self.attributes = attributes unless attributes.nil? yield self if block_given? end Maybe the right thing to do is to implement ActiveRecord::Base.new as: class ActiveRecord::Base def self.new(attributes = nil, &block) returning(self.allocate) do |instance| instance.initialize_metadata instance.initialize(attributes, &block) end end def initialize_metadata @attributes = attributes_from_column_definition @new_record = true ensure_proper_type end def initialize(attributes) self.attributes = attributes unless attributes.nil? yield self if block_given? end end Then anyone who wants to write code that initializes defaults before the actual attributes are set can do: def initialize(attributes = nil, &block) self.content = "Write something here" super end and everybody is happy. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 24/07/07, Pratik <pratiknaik@gmail.com> wrote:> > I would personally do it as something like : > > def initialize_with_defaults(attributes = nil, &block) > initialize_without_defaults(attributes) do > self.some_attribute = ''whatever'' # Setting default value > yield self if block_given? > end > end > alias_method_chain :initialize, :defaults > > Looks like it''s working quite well.Except in the case where it overrides a value that''s already been set by initialize_without_defaults. I''m not sure that''s the behaviour you want. Plus, imitating the action of super by hand coding seems like a bit of a waste of time somehow. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 7/24/07, Piers Cawley <pdcawley@bofh.org.uk> wrote:> Except in the case where it overrides a value that''s already been set > by initialize_without_defaults. I''m not sure that''s the behaviour you > want. >Yeah. initialize_without_defaults is basically AR::Base#initialize - so yeah, this will override values set by that. If you''re already overriding default values, I''m not sure how this might be a problem.> Plus, imitating the action of super by hand coding seems like a bit of > a waste of time somehow.Well, basically this is supplying a block to AR::Base#initialize which is executed by "yield self if block_given?" - this protect it against possible changes that might happen to AR::Base#initialize. It also allows you to override the default values you''ve set. So you do something like : Model.new do |model| model.i_dont_want_your_default = ''whatever'' end It might be useful in case of STI or in controllers where you want to override default values. So yeah, it''s not really imitating the action of super by hand, but using proper technique to set defaults I feel. This just feels cleaner. -- Cheers! - Pratik http://m.onkey.org --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 24/07/07, Pratik <pratiknaik@gmail.com> wrote:> > On 7/24/07, Piers Cawley <pdcawley@bofh.org.uk> wrote: > > > Except in the case where it overrides a value that''s already been set > > by initialize_without_defaults. I''m not sure that''s the behaviour you > > want. > > > > Yeah. initialize_without_defaults is basically AR::Base#initialize - > so yeah, this will override values set by that. If you''re already > overriding default values, I''m not sure how this might be a problem.Because what you''re actually doing having your block override the a value that''s been specified in the attributes hash with your default. Which probably isn''t what you want. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Then do :
def initialize_with_defaults(attributes = nil, &block)
initialize_without_defaults(attributes) do
self.some_attribute ||= ''whatever'' # Setting default
value if not present
yield self if block_given?
end
end
alias_method_chain :initialize, :defaults
On 7/24/07, Piers Cawley <pdcawley@bofh.org.uk>
wrote:>
> On 24/07/07, Pratik <pratiknaik@gmail.com> wrote:
> >
> > On 7/24/07, Piers Cawley <pdcawley@bofh.org.uk> wrote:
> >
> > > Except in the case where it overrides a value that''s
already been set
> > > by initialize_without_defaults. I''m not sure
that''s the behaviour you
> > > want.
> > >
> >
> > Yeah. initialize_without_defaults is basically AR::Base#initialize -
> > so yeah, this will override values set by that. If you''re
already
> > overriding default values, I''m not sure how this might be a
problem.
>
> Because what you''re actually doing having your block override the
a
> value that''s been specified in the attributes hash with your
default.
> Which probably isn''t what you want.
>
> >
>
--
Cheers!
- Pratik
http://m.onkey.org
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
On 24/07/07, Pratik <pratiknaik@gmail.com> wrote:> > Then do : > > def initialize_with_defaults(attributes = nil, &block) > initialize_without_defaults(attributes) do > self.some_attribute ||= ''whatever'' # Setting default value if not present > yield self if block_given? > end > end > alias_method_chain :initialize, :defaultsI think we''re starting back around the circle again, but what happens when the default value is ''true'' and the arguments specify ''false''. And, then, when you''ve solved that one, what happens when ''nil'' is a legal value, but the default value is non nil? --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Umm..I think that leaves us with something like:
class Item < ActiveRecord::Base
def initialize_with_defaults(attrs = nil, &block)
initialize_without_defaults(attrs) do
setter = lambda { |key, value| self.send("#{key}=", value)
unless !attrs.nil? && attrs.keys.map(&:to_s).include?(key) }
setter.call(''scheduler_type'',
''hotseat'')
yield self if block_given?
end
end
alias_method_chain :initialize, :defaults
end
Can you please point out situations where this will fails ?
Thanks !
-Pratik
On 7/24/07, Piers Cawley <pdcawley@bofh.org.uk>
wrote:>
> On 24/07/07, Pratik <pratiknaik@gmail.com> wrote:
> >
> > Then do :
> >
> > def initialize_with_defaults(attributes = nil, &block)
> > initialize_without_defaults(attributes) do
> > self.some_attribute ||= ''whatever'' # Setting
default value if not present
> > yield self if block_given?
> > end
> > end
> > alias_method_chain :initialize, :defaults
>
> I think we''re starting back around the circle again, but what
happens
> when the default value is ''true'' and the arguments
specify ''false''.
>
> And, then, when you''ve solved that one, what happens when
''nil'' is a
> legal value, but the default value is non nil?
>
> >
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
On 24/07/07, Pratik <pratiknaik@gmail.com> wrote:> > Umm..I think that leaves us with something like: > > class Item < ActiveRecord::Base > def initialize_with_defaults(attrs = nil, &block) > initialize_without_defaults(attrs) do > setter = lambda { |key, value| self.send("#{key}=", value) > unless !attrs.nil? && attrs.keys.map(&:to_s).include?(key) } > setter.call(''scheduler_type'', ''hotseat'') > yield self if block_given? > end > end > alias_method_chain :initialize, :defaults > end > > Can you please point out situations where this will fails ?Whenever I you try to understand it? The ''rewrite'' new approach means you have an easily overrideable ''initialize'' that works with all the edge cases you''re taking account of. Frankly, I''d much rather be able to write: def initialize(attrs = nil, &block) self.body = "Write something here" super end and know it''s going to work. Your mileage may vary. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 24/07/07, Piers Cawley <pdcawley@bofh.org.uk> wrote:> On 24/07/07, Pratik <pratiknaik@gmail.com> wrote: > > > > Umm..I think that leaves us with something like: > > > > class Item < ActiveRecord::Base > > def initialize_with_defaults(attrs = nil, &block) > > initialize_without_defaults(attrs) do > > setter = lambda { |key, value| self.send("#{key}=", value) > > unless !attrs.nil? && attrs.keys.map(&:to_s).include?(key) } > > setter.call(''scheduler_type'', ''hotseat'') > > yield self if block_given? > > end > > end > > alias_method_chain :initialize, :defaults > > end > > > > Can you please point out situations where this will fails ? > > Whenever I you try to understand it? > > The ''rewrite'' new approach means you have an easily overrideable > ''initialize'' that works with all the edge cases you''re taking account > of. Frankly, I''d much rather be able to write: > > def initialize(attrs = nil, &block) > self.body = "Write something here" > super > end > > and know it''s going to work. Your mileage may vary.Oh yes, I''ll bet dollars to doughnuts that the rewrite new approach is the faster of the two as well. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> Whenever I you try to understand it?You don''t have to lean on sarcasm when you have nothing constructive to say ( especially after you pointed out deftect in implementation and I corrected them ). May be you can focus on submitting a patch. patch -p0 speaks a lot louder than your sad grammar. AR:Base#initialize is supposed to be an internal method. And it should be flexible enough to be changed in future for whatever reasons. -- Cheers! - Pratik http://m.onkey.org --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On Jul 23, 2007, at 7:14 PM, Pratik wrote:> Why add a new callback when all you need is 4 lines of documentation > on how to do it ?because you can say that about all of the callbacks. a @ http://drawohara.com/ -- we can deny everything, except that we have the possibility of being better. simply reflect on that. h.h. the 14th dalai lama --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Umm...what would you suggest as alternative for before/after_add/remove association collection callbacks ? Anyways, I''m off this thread unless we see some patches. That makes arguing a lot easier :-) On 7/24/07, ara.t.howard <ara.t.howard@gmail.com> wrote:> > > On Jul 23, 2007, at 7:14 PM, Pratik wrote: > > > Why add a new callback when all you need is 4 lines of documentation > > on how to do it ? > > because you can say that about all of the callbacks. > > a @ http://drawohara.com/ > -- > we can deny everything, except that we have the possibility of being > better. simply reflect on that. > h.h. the 14th dalai lama > > > > > > >-- Cheers! - Pratik http://m.onkey.org --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On Jul 22, 2007, at 4:39 PM, Michael Koziarski wrote:> > I can buy your argument, but if I do so, there doesn''t seem to be a > need for after_initialize: > > def initialize(attrs) > do_stuff > super > end > > All we''d be doing is reinventing an initialize method which hid the > arguments. Doesn''t seem particularly necessary, especially at the > cost of backwards compatibility for the people who use > after_initialize at present. >the trick, of course, is that attrs are sometimes nil - also we (client code) don''t know what/if any block might be, or not be, for... i just had this issue last week and here is how i solved it class Agent < ActiveRecord::Base def initialize options, &block super((options || {}).reverse_merge!(defaults), &block) end def defaults Hash.new end end this is used in an STI situation so i also have code like this class Group < Agent def defaults { ''allow_login'' => false, } end end class User < Agent def defaults { ''allow_login'' => true, } end end and this seems to be working well. so my own suggestion wouldn''t be so much for another callback but a way to simply set the defaults for a record, perfering a method over a hash so that current object state and arbitrary logic might affect the result, Time.now, for instance. also, i strongly agree that this is a giant hole in the ar lifecycle. kind regards. a @ http://drawohara.com/ -- we can deny everything, except that we have the possibility of being better. simply reflect on that. h.h. the 14th dalai lama --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On Jul 22, 2007, at 4:39 PM, Michael Koziarski wrote:> > I can buy your argument, but if I do so, there doesn''t seem to be a > need for after_initialize: > > def initialize(attrs) > do_stuff > super > endone other nit-pick: it''s somewhat archaic and java like to __require__ client code to call super, one slip of the keyboard and someone is bug finding... it''s much nicer, imho, to relieve the client code from that burden via class ActionRecord::Base def self.new *args, &block returning(allocate) do |object| object.instance_eval do internal_initialize *args, &block # all real work here initialize *args, &block # default does nothing end end end end basically just following the paradigm whereby designing classes that a made to be inherited with any work in initialize is less that ideal... food for thought. cheers. a @ http://drawohara.com/ -- we can deny everything, except that we have the possibility of being better. simply reflect on that. h.h. the 14th dalai lama --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On Jul 24, 2007, at 11:15 AM, Pratik wrote:> > Umm...what would you suggest as alternative for > before/after_add/remove association collection callbacks ?i''m not suggesting an alternative, i saying that just because docs *can* take the place of easy code doesn''t mean they should. i''d just hate to see this in the docs: # this is how to write a ''before save'' filter in rails # # save = method :save # # define_method :save do |*argv| # p ''before_save'' # save.call *argv # end the declarative style of ar is a wonderful thing - better the leverage it rather than use docs imho. regards. a @ http://drawohara.com/ -- we can deny everything, except that we have the possibility of being better. simply reflect on that. h.h. the 14th dalai lama --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Yeah. You''re right. When I wrote that I didn''t really think about attr being false/nil etc. And I assumed the solution to be as simple as overriding initialize and calling super as the first statement. On 7/24/07, ara.t.howard <ara.t.howard@gmail.com> wrote:> > > On Jul 24, 2007, at 11:15 AM, Pratik wrote: > > > > > Umm...what would you suggest as alternative for > > before/after_add/remove association collection callbacks ? > > i''m not suggesting an alternative, i saying that just because docs > *can* take the place of easy code doesn''t mean they should. i''d just > hate to see this in the docs: > > # this is how to write a ''before save'' filter in rails > # > # save = method :save > # > # define_method :save do |*argv| > # p ''before_save'' > # save.call *argv > # end > > the declarative style of ar is a wonderful thing - better the > leverage it rather than use docs imho. > > regards. > > a @ http://drawohara.com/ > -- > we can deny everything, except that we have the possibility of being > better. simply reflect on that. > h.h. the 14th dalai lama > > > > > > >-- Cheers! - Pratik http://m.onkey.org --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On Jul 24, 2007, at 11:15 AM, Pratik wrote:> Anyways, I''m off this thread unless we see some patches. That makes > arguing a lot easier :-)--- vendor/rails/activerecord/lib/active_record/base.rb.org 2007-07-24 11:37:07.000000000 -0600 +++ vendor/rails/activerecord/lib/active_record/base.rb 2007-07-24 11:40:32.000000000 -0600 @@ -1502,10 +1502,14 @@ @attributes = attributes_from_column_definition @new_record = true ensure_proper_type - self.attributes = attributes unless attributes.nil? + self.attributes = (attributes || {}).stringify_keys.reverse_merge!(defaults.stringify_keys) yield self if block_given? end + def defaults + Hash.new + end + # A model instance''s primary key is always available as model.id # whether you name it the default ''id'' or set it to something else. def id a @ http://drawohara.com/ -- we can deny everything, except that we have the possibility of being better. simply reflect on that. h.h. the 14th dalai lama --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 24/07/07, ara.t.howard <ara.t.howard@gmail.com> wrote:> > > On Jul 22, 2007, at 4:39 PM, Michael Koziarski wrote: > > > > > I can buy your argument, but if I do so, there doesn''t seem to be a > > need for after_initialize: > > > > def initialize(attrs) > > do_stuff > > super > > end > > one other nit-pick: it''s somewhat archaic and java like to > __require__ client code to call super, one slip of the keyboard and > someone is bug finding... > > it''s much nicer, imho, to relieve the client code from that burden via > > class ActionRecord::Base > def self.new *args, &block > returning(allocate) do |object| > object.instance_eval do > internal_initialize *args, &block # all real work here > initialize *args, &block # default does nothing > end > end > end > end > > > basically just following the paradigm whereby designing classes that > a made to be inherited with any work in initialize is less that > ideal... food for thought.Except that, when you''re setting defaults, you really would like to interpose some behaviour between the bit that gets the object to the point where the various setter/getter methods will work and the bit where ActiveRecord calls self.attribs = ... One option would be to override ActiveRecord::Base.allocate to return an object that''s been got to that point, but then you''d be doing a chunk of work that then gets thrown away whenever an object is made through AR::Base.instantiate. Sadly, initialize tends to be one of those warts on an API where you really can''t get away with requiring client code to call super. You *could* add #before_initialize and #after_initialize hooks, but then we''re back where we started... --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On Jul 24, 2007, at 12:00 PM, Piers Cawley wrote:> Except that, when you''re setting defaults, you really would like to > interpose some behaviour between the bit that gets the object to the > point where the various setter/getter methods will work and the bit > where ActiveRecord calls self.attribs = ...yeah i agree completely. my impl just happened to skin my own cat quite nicely at the time, but a more general solution would do just that... before_initialize and after_initialize don''t seem to bad after all eh? kind regards. a @ http://drawohara.com/ -- we can deny everything, except that we have the possibility of being better. simply reflect on that. h.h. the 14th dalai lama --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 24/07/07, ara.t.howard <ara.t.howard@gmail.com> wrote:> > > On Jul 24, 2007, at 12:00 PM, Piers Cawley wrote: > > > Except that, when you''re setting defaults, you really would like to > > interpose some behaviour between the bit that gets the object to the > > point where the various setter/getter methods will work and the bit > > where ActiveRecord calls self.attribs = ... > > yeah i agree completely. my impl just happened to skin my own cat > quite nicely at the time, but a more general solution would do just > that...The trouble with patching a framework is that you have to account for every kind of cat under the sun. At the very least, you shouldn''t make it so that some cats are completely unskinnable. Anyhoo. Here''s my patch, complete with a new test. I''ve probably put methods in the wrong place, of course, but it does at least work. It''s against the ActiveRecord in Rails 1.2.3, but unless things have changed quite dramatically it should apply reasonably cleanly to the edge I think. Index: test/base_test.rb ==================================================================--- test/base_test.rb (revision 6690) +++ test/base_test.rb (working copy) @@ -47,6 +47,13 @@ attr_accessible :phone_number end +class DefaultedPerson < LoosePerson + def initialize(*args, &block) + self.first_name = ''Bob'' + super + end +end + class Booleantest < ActiveRecord::Base; end class Task < ActiveRecord::Base @@ -365,6 +372,11 @@ assert_raises(ActiveRecord::RecordNotFound) { topicReloaded Topic.find(99999) } end + def test_initialize_sets_defaults + person = DefaultedPerson.new + assert_equal("Bob", person.first_name) + end + def test_initialize_with_attributes topic = Topic.new({ "title" => "initialized from attributes", "written_on" => "2003-12-12 23:23" Index: lib/active_record/base.rb ==================================================================--- lib/active_record/base.rb (revision 6690) +++ lib/active_record/base.rb (working copy) @@ -453,6 +453,13 @@ end end + def new(attributes = nil, &block) #:nodoc: + returning(allocate) do |instance| + instance.send(:initialize_metadata) + instance.send(:initialize, attributes, &block) + end + end + # Finds the record from the passed +id+, instantly saves it with the passed +attributes+ (if the validation permits it), # and returns it. If the save fails under validations, the unsaved object is still returned. # @@ -1502,11 +1509,14 @@ # In both instances, valid attribute keys are determined by the column names of the associated table -- # hence you can''t have attributes that aren''t part of the table columns. def initialize(attributes = nil) + self.attributes = attributes unless attributes.nil? + yield self if block_given? + end + + def initialize_metadata #:nodoc: @attributes = attributes_from_column_definition @new_record = true ensure_proper_type - self.attributes = attributes unless attributes.nil? - yield self if block_given? end # A model instance''s primary key is always available as model.id --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
ara.t.howard wrote:> > On Jul 24, 2007, at 11:15 AM, Pratik wrote: > >> Anyways, I''m off this thread unless we see some patches. That makes >> arguing a lot easier :-) > > --- vendor/rails/activerecord/lib/active_record/base.rb.org > 2007-07-24 11:37:07.000000000 -0600 > +++ vendor/rails/activerecord/lib/active_record/base.rb 2007-07-24 > 11:40:32.000000000 -0600 > @@ -1502,10 +1502,14 @@ > @attributes = attributes_from_column_definition > @new_record = true > ensure_proper_type > - self.attributes = attributes unless attributes.nil? > + self.attributes = (attributes || > {}).stringify_keys.reverse_merge!(defaults.stringify_keys) > yield self if block_given? > end > + def defaults > + Hash.new > + end > + > # A model instance''s primary key is always available as model.id > # whether you name it the default ''id'' or set it to something > else. > def id >+1 Ben PS: however, not sure what rev you''re diffing against but in head there''s another line between the self.attributes and the yield: self.class.send(:scope, :create).each { |att,value| self.send("#{att}=", value) } if self.class.send(:scoped?, :create) --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
http://pastie.caboo.se/81829 That''s my proposed solution. Thanks, Pratik On 7/24/07, Ben Munat <bmunat@gmail.com> wrote:> > ara.t.howard wrote: > > > > On Jul 24, 2007, at 11:15 AM, Pratik wrote: > > > >> Anyways, I''m off this thread unless we see some patches. That makes > >> arguing a lot easier :-) > > > > --- vendor/rails/activerecord/lib/active_record/base.rb.org > > 2007-07-24 11:37:07.000000000 -0600 > > +++ vendor/rails/activerecord/lib/active_record/base.rb 2007-07-24 > > 11:40:32.000000000 -0600 > > @@ -1502,10 +1502,14 @@ > > @attributes = attributes_from_column_definition > > @new_record = true > > ensure_proper_type > > - self.attributes = attributes unless attributes.nil? > > + self.attributes = (attributes || > > {}).stringify_keys.reverse_merge!(defaults.stringify_keys) > > yield self if block_given? > > end > > + def defaults > > + Hash.new > > + end > > + > > # A model instance''s primary key is always available as model.id > > # whether you name it the default ''id'' or set it to something > > else. > > def id > > > > +1 > > Ben > > PS: however, not sure what rev you''re diffing against but in head there''s another line between the > self.attributes and the yield: > > self.class.send(:scope, :create).each { |att,value| self.send("#{att}=", value) } if > self.class.send(:scoped?, :create) > > > > >-- Cheers! - Pratik http://m.onkey.org --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On Jul 24, 2007, at 1:33 PM, Pratik wrote:> > http://pastie.caboo.se/81829 > > That''s my proposed solution.my take is that is HAS to be a method so that self.foobar = self.foo + self.bar which is very hard to do with a hash based mechanism (i realize mine is hash based too!) so, to me, it really suggests a callback or post_initialize approach... 2 cts. a @ http://drawohara.com/ -- we can deny everything, except that we have the possibility of being better. simply reflect on that. h.h. the 14th dalai lama --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 7/23/07, Michael Koziarski <michael@koziarski.com> wrote:> Without cases which > can''t be solved by this pattern, I''m not sure that adding a new > callback is justified.Sorry for replying to myself, but the wireless here is a little too spotty to reply to all the individual threads. Providing a method to allow static defaults such as: set_default_values :foo=>''bar'' Is just reimplementing the pre-existing defaults code that we extract from the columns, so I''m not sure that''s the right way to go. For dynamic defaults, such as ''the default value for the categories on a blog, is taken from the account''s default values'' I''ve not seen a case that can''t be catered for by overriding the initialize method. Requiring users to call super first is perfectly acceptable to me. To expect to use methods from a super class without first initializing that super class seems very strange to me. Of course, there may well be particular cases where it''s genuinely confusing that something doesn''t work, or that the error messages are misleading, but we can fix those on a case by case basis. Every feature that we add, becomes something we need to support on an ongoing basis, for the foreseeable future. So we have to have a reject by default, or we''ll end up with another ''components''. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 24/07/07, Michael Koziarski <michael@koziarski.com> wrote:> > On 7/23/07, Michael Koziarski <michael@koziarski.com> wrote: > > > Without cases which > > can''t be solved by this pattern, I''m not sure that adding a new > > callback is justified. > > Sorry for replying to myself, but the wireless here is a little too > spotty to reply to all the individual threads. > > Providing a method to allow static defaults such as: > > set_default_values :foo=>''bar'' > > Is just reimplementing the pre-existing defaults code that we extract > from the columns, so I''m not sure that''s the right way to go. > > For dynamic defaults, such as ''the default value for the categories on > a blog, is taken from the account''s default values'' I''ve not seen a > case that can''t be catered for by overriding the initialize method. > > Requiring users to call super first is perfectly acceptable to me. To > expect to use methods from a super class without first initializing > that super class seems very strange to me.class Foo attr_accessor :bibble end class Bar < Foo def initialize # Look ma, I''m using a superclass method before I initialize it! self.bibble = "Pling!" super end end Happens all the time. The current ActiveRecord behaviour is weird because it breaks that expectation. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On Jul 24, 2007, at 2:50 PM, Michael Koziarski wrote:> > On 7/23/07, Michael Koziarski <michael@koziarski.com> wrote: > >> Without cases which >> can''t be solved by this pattern, I''m not sure that adding a new >> callback is justified. > > Sorry for replying to myself, but the wireless here is a little too > spotty to reply to all the individual threads. > > Providing a method to allow static defaults such as: > > set_default_values :foo=>''bar'' > > Is just reimplementing the pre-existing defaults code that we extract > from the columns, so I''m not sure that''s the right way to go. > > For dynamic defaults, such as ''the default value for the categories on > a blog, is taken from the account''s default values'' I''ve not seen a > case that can''t be catered for by overriding the initialize method. > > Requiring users to call super first is perfectly acceptable to me. To > expect to use methods from a super class without first initializing > that super class seems very strange to me. > > Of course, there may well be particular cases where it''s genuinely > confusing that something doesn''t work, or that the error messages are > misleading, but we can fix those on a case by case basis. > > Every feature that we add, becomes something we need to support on an > ongoing basis, for the foreseeable future. So we have to have a > reject by default, or we''ll end up with another ''components''. >i agree nearly 100%. the simplest solution, by far, seem to simply make after_initialize actually called then. leaving open the door for before_initialize and remaining backward compatible.> -- > Cheers > > Koz > > >a @ http://drawohara.com/ -- we can deny everything, except that we have the possibility of being better. simply reflect on that. h.h. the 14th dalai lama --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On Jul 24, 2007, at 2:50 PM, Michael Koziarski wrote:> > For dynamic defaults, such as ''the default value for the categories on > a blog, is taken from the account''s default values'' I''ve not seen a > case that can''t be catered for by overriding the initialize method. > > Requiring users to call super first is perfectly acceptable to me. To > expect to use methods from a super class without first initializing > that super class seems very strange to me. >so i''m hacking on this right now, and the issue with this is as follows, you basically have to do something like this: def initialize options = nil, &block super if options options.to_options! unless options.has_key? :some_column self.some_column = ''some_default_value'' end end end to be robust because otherwise you will clobber a call like this Module.new ''some_column'' => nil where the user explicitly asks for a column to be nil, or false for that matter, or used a string key, etc. you can''t just check the value from the db either since that may be false, etc. my point is that, to me, ''default'' in this context means the following: set it iff the user did not specify it. clobbering, or setting the value only because it''s not a truth value doesn''t fit the bill here and testing whether it was passed in seems a little verbose for what should be a very easy task. maybe i am making it over complicated? cheers. a @ http://drawohara.com/ --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> Happens all the time. The current ActiveRecord behaviour is weird > because it breaks that expectation.You''re describing the current behaviour of active record and it''s use of method_missing to trigger generate_read_methods, and the lack of generate write methods. It leads to other crazy stuff like>> Post.public_instance_methods.grep(/body/)=> []>> Post.find(1).body=> "Howdy World">> Post.public_instance_methods.grep(/body/)=> ["body?", "body"] I agree that this is counter intuitive, if you have a fix for this, I''d definitely be interested in applying it, the current behaviour causes problems here and in other cases. However this is the problem that should be fixed, not adding a callback to work around existing confusing implementation of our attribute related methods. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 24/07/07, Michael Koziarski <michael@koziarski.com> wrote:> > > Happens all the time. The current ActiveRecord behaviour is weird > > because it breaks that expectation. > > You''re describing the current behaviour of active record and it''s use > of method_missing to trigger generate_read_methods, and the lack of > generate write methods. It leads to other crazy stuff like > > >> Post.public_instance_methods.grep(/body/) > => [] > >> Post.find(1).body > => "Howdy World" > >> Post.public_instance_methods.grep(/body/) > => ["body?", "body"] > > I agree that this is counter intuitive, if you have a fix for this, > I''d definitely be interested in applying it, the current behaviour > causes problems here and in other cases. However this is the problem > that should be fixed, not adding a callback to work around existing > confusing implementation of our attribute related methods.Um... the patch I posted doesn''t actually add a callback, it just rejigs things so that def initialize(attrs = nil) self.some_attrib = ''a default value'' super end works as well as def initialize(attrs = nil) super self.some_attrib ||= ''a default value'' end As for the public_instance_methods issue, when does a class find out about its columns? Assuming columns is known about early enough before any records are fetched from the database, it should be possible to fake out public_instance_methods. It''s just a matter of finding someone who has an appropriately shaped tuit. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On Jul 24, 2007, at 2:50 PM, Michael Koziarski wrote:> > Sorry for replying to myself, but the wireless here is a little too > spotty to reply to all the individual threads. > > Providing a method to allow static defaults such as: > > set_default_values :foo=>''bar'' > > Is just reimplementing the pre-existing defaults code that we extract > from the columns, so I''m not sure that''s the right way to go. > > For dynamic defaults, such as ''the default value for the categories on > a blog, is taken from the account''s default values'' I''ve not seen a > case that can''t be catered for by overriding the initialize method. > > Requiring users to call super first is perfectly acceptable to me. To > expect to use methods from a super class without first initializing > that super class seems very strange to me. > > Of course, there may well be particular cases where it''s genuinely > confusing that something doesn''t work, or that the error messages are > misleading, but we can fix those on a case by case basis. > > Every feature that we add, becomes something we need to support on an > ongoing basis, for the foreseeable future. So we have to have a > reject by default, or we''ll end up with another ''components''.okay, this isn''t a patch but it''s what i''m using internally and a suggestion for an impl: class ActiveRecord::Base alias_method ''__initialize__'', ''initialize'' def initialize options = nil, &block returning( __initialize__(options, &block) ) do options ||= {} options.to_options! defaults = self.class.defaults || self.defaults || Hash.new (defaults.keys - options.keys).each do |key| value = defaults[key] case value when Proc value = instance_eval &value when Symbol value = send value end send "#{ key }=", value end end end def self.defaults *argv @defaults = argv.shift.to_hash if argv.first return @defaults if defined? @defaults end def defaults *argv @defaults = argv.shift.to_hash if argv.first return @defaults if defined? @defaults end end while this might seem overly complicated at first it makes all this behave correctly class C < ActiveRecord::Base defaults :foo => 42 end p C.new.foo #=> 42 p C.new(''foo'' => 42.0) #=> 42.0 class C < ActiveRecord::Base defaults :foo => :bar def bar() 42 end end p C.new.foo #=> 42 class C < ActiveRecord::Base defaults :foo => lambda{ 42 } end p C.new.foo #=> 42 class C < ActiveRecord::Base defaults :foo => lambda{ bar } def bar() 42 end end p C.new.foo #=> 42 also note that the defaults are set *after* the ''normal'' initialization had been executed - allowed things like class C default :foobar => lambda{ foo + bar } end kind regards. a @ http://drawohara.com/ -- we can deny everything, except that we have the possibility of being better. simply reflect on that. h.h. the 14th dalai lama --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
> Um... the patch I posted doesn''t actually add a callback, it just > rejigs things so that > > def initialize(attrs = nil) > self.some_attrib = ''a default value'' > super > end > > works as well as > > def initialize(attrs = nil) > super > self.some_attrib ||= ''a default value'' > endI''m not sold on the need for this in and of itself but if someone else feels it''s justified I wouldn''t object to them applying it. I''d prefer to see work done towards the correct fix than put something in that wouldn''t be needed in the long term, and if we have currently motivated people, all the better :).> As for the public_instance_methods issue, when does a class find out > about its columns? Assuming columns is known about early enough before > any records are fetched from the database, it should be possible to > fake out public_instance_methods.The columns are lazily retrieved, but once they''re retrieved it should be possible to generate all the accessors and mutators. You don''t want to retrieve them too early as the database isn''t always available, however it''s definitely something that a motivated experimenter could investigate in a relatively short period of time. -- Cheers Koz --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On Jul 24, 2007, at 2:50 PM, Michael Koziarski wrote:> Every feature that we add, becomes something we need to support on an > ongoing basis, for the foreseeable future. So we have to have a > reject by default, or we''ll end up with another ''components''.i hear that. i put up a summary of what i''m doing now in my code, for posterity, at: http://drawohara.tumblr.com/post/6677354 maybe it will inspire people or at least serve as an example of what people hate ;-) a @ http://drawohara.com/ -- we can deny everything, except that we have the possibility of being better. simply reflect on that. h.h. the 14th dalai lama --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
Not sure if you checked this or not - http://dev.rubyonrails.org/ticket/9093 :) Any inputs for that ? Thanks. On 7/25/07, ara.t.howard <ara.t.howard@gmail.com> wrote:> > > On Jul 24, 2007, at 2:50 PM, Michael Koziarski wrote: > > > Every feature that we add, becomes something we need to support on an > > ongoing basis, for the foreseeable future. So we have to have a > > reject by default, or we''ll end up with another ''components''. > > i hear that. i put up a summary of what i''m doing now in my code, > for posterity, at: > > http://drawohara.tumblr.com/post/6677354 > > maybe it will inspire people or at least serve as an example of what > people hate ;-) > > a @ http://drawohara.com/ > -- > we can deny everything, except that we have the possibility of being > better. simply reflect on that. > h.h. the 14th dalai lama > > > > > > >-- Cheers! - Pratik http://m.onkey.org --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
On 25/07/07, Michael Koziarski <michael@koziarski.com> wrote:> > > Um... the patch I posted doesn''t actually add a callback, it just > > rejigs things so that > > > > def initialize(attrs = nil) > > self.some_attrib = ''a default value'' > > super > > end > > > > works as well as > > > > def initialize(attrs = nil) > > super > > self.some_attrib ||= ''a default value'' > > end > > I''m not sold on the need for this in and of itself but if someone else > feels it''s justified I wouldn''t object to them applying it. I''d > prefer to see work done towards the correct fix than put something in > that wouldn''t be needed in the long term, and if we have currently > motivated people, all the better :).The thing is, I actually think that the fix that makes initialize overloadable in the same way as pretty much every other ruby class out there _is_ the correct thing, regardless of whether there there are any class methods in place to set defaults in a more declarative fashion. Given a straightforwardly overloadable initialize + a declarative setup, the user has the choice of how they implement defaults, either by overloading or by using ''default :foo => whatever'' type declarations. --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
I recall hearing the Holy Beasts of Doom argument for overriding new -
but not for initialize. I''ve been using something like Piers''
approach, which has always worked for me:
def initialize(attrs = {}, &block)
super attrs.reverse_merge(default_attributes_hash), &block
more_initialization_dependent_on_instance_already_being_built
end
I see the problem with string-versus-symbol keys, but Rails has had
that problem for a long time as well as tolerable solutions
(HashWithIndifferentAccess, or the manual equivalent).
Unless somebody has some good evidence of why overriding initialize is
inherently dangerous (and not just tricky w.r.t. before and after
super), I would vote for leaving the callbacks as they are.
On Jul 24, 6:40 am, "Piers Cawley" <pdcaw...@bofh.org.uk>
wrote:> On 24/07/07, Ben Munat <bmu...@gmail.com> wrote:
>
> > So, Piers, you''re the one that started this whole thing... if
it really is safe to do:
>
> > class MyModel < ActiveRecord::Base
> > def initialize(attrs = {}, &block)
> > super
> > # all my initialization stuff here
> > end
> > end
>
> > and this is documented in the AR::Base docs, then this does kind of
seem like enough. I mean, it''s
> > what I did all the time in my java life.
>
> > I''d just swear that someone told me the holy beasts of doom
would swarm down upon me if I ever tried
> > to override AR::Base#initialize.
>
> Yeah, I think I''ve heard the Holy Beasts of Doom argument as well,
I
> just can''t for the life of me remember where. I have certainly
been
> bitten by trying to set the defaults before calling super though
> (because, dammit, that''s the logical place to set defaults).
Thinking
> about it, the way to do that would be something like:
>
> def initialize(attrs = {}, &block)
> super attrs.reverse_merge(:default => ''this''),
&block
> end
>
> but then you make yourself a hostage to fortune that everyone''s
going
> to use symbols as keys. The post super approach to setting defaults
> has some edge case problems too in cases where nil is a legal value
> for some attribute (yeah, it''s a weird edge case, but an edge case
all
> the same). You end up with ugly code like:
>
> def initialize(attrs = {}, &block)
> super
> unless attrs.has_key?(:content) ||
attrs.has_key?(''content'')
> self.content = "Write something here"
> end
> end
>
> The issue is, I think, that ActiveRecord::Base#initialize is doing two
> different ''sorts'' of things: class invariant metadata
initialization,
> and instance specific initialization. You might compose the method as:
>
> def initialize(attributes = nil, &block)
> initialize_metadata
> initialize_instance(attributes, &block)
> end
>
> def initialize_metadata
> @attributes = attributes_from_column_definition
> @new_record = true
> ensure_proper_type
> end
>
> def initialize_instance(attributes
> self.attributes = attributes unless attributes.nil?
> yield self if block_given?
> end
>
> Maybe the right thing to do is to implement ActiveRecord::Base.new as:
>
> class ActiveRecord::Base
> def self.new(attributes = nil, &block)
> returning(self.allocate) do |instance|
> instance.initialize_metadata
> instance.initialize(attributes, &block)
> end
> end
>
> def initialize_metadata
> @attributes = attributes_from_column_definition
> @new_record = true
> ensure_proper_type
> end
>
> def initialize(attributes)
> self.attributes = attributes unless attributes.nil?
> yield self if block_given?
> end
> end
>
> Then anyone who wants to write code that initializes defaults before
> the actual attributes are set can do:
>
> def initialize(attributes = nil, &block)
> self.content = "Write something here"
> super
> end
>
> and everybody is happy.
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
Piers Cawley wrote:> On 24/07/07, Ben Munat <bmunat@gmail.com> wrote: > >> So, Piers, you''re the one that started this whole thing... if it really is safe to do: >> >> class MyModel < ActiveRecord::Base >> def initialize(attrs = {}, &block) >> super >> # all my initialization stuff here >> end >> end >> >> and this is documented in the AR::Base docs, then this does kind of seem like enough. I mean, it''s >> what I did all the time in my java life. >> >> I''d just swear that someone told me the holy beasts of doom would swarm down upon me if I ever tried >> to override AR::Base#initialize. >> > > Yeah, I think I''ve heard the Holy Beasts of Doom argument as well, I > just can''t for the life of me remember where. I have certainly been > bitten by trying to set the defaults before calling super though > (because, dammit, that''s the logical place to set defaults). Thinking > about it, the way to do that would be something like: > > def initialize(attrs = {}, &block) > super attrs.reverse_merge(:default => ''this''), &block > end > > but then you make yourself a hostage to fortune that everyone''s going > to use symbols as keys. The post super approach to setting defaults > has some edge case problems too in cases where nil is a legal value > for some attribute (yeah, it''s a weird edge case, but an edge case all > the same). You end up with ugly code like: > > def initialize(attrs = {}, &block) > super > unless attrs.has_key?(:content) || attrs.has_key?(''content'') > self.content = "Write something here" > end > end > > The issue is, I think, that ActiveRecord::Base#initialize is doing two > different ''sorts'' of things: class invariant metadata initialization, > and instance specific initialization. You might compose the method as: > > def initialize(attributes = nil, &block) > initialize_metadata > initialize_instance(attributes, &block) > end > > def initialize_metadata > @attributes = attributes_from_column_definition > @new_record = true > ensure_proper_type > end > > def initialize_instance(attributes > self.attributes = attributes unless attributes.nil? > yield self if block_given? > end > > Maybe the right thing to do is to implement ActiveRecord::Base.new as: > > class ActiveRecord::Base > def self.new(attributes = nil, &block) > returning(self.allocate) do |instance| > instance.initialize_metadata > instance.initialize(attributes, &block) > end > end > > def initialize_metadata > @attributes = attributes_from_column_definition > @new_record = true > ensure_proper_type > end > > def initialize(attributes) > self.attributes = attributes unless attributes.nil? > yield self if block_given? > end > end > > Then anyone who wants to write code that initializes defaults before > the actual attributes are set can do: > > def initialize(attributes = nil, &block) > self.content = "Write something here" > super > end > > and everybody is happy. >+1 for everybody. Largely the case is simply setting defaults with "if new_record?". Since it''s 80% of the job that can be handled with (ficitional) 20% code such as: def after_new_record country = ''United States'' is_administrator? = false end I think that''s nice :) (since it doesn''t use a dynamic trigger, it should pose not very big overhead?) -- Hendy Irawan www.hendyirawan.com --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---