Account Options

  1. Sign in
The old Google Groups will be going away soon, but your browser is incompatible with the new version.
Google Groups Home
« Groups Home
after_initialize/after_find misfeature
There are currently too many topics in this group that display first. To make this topic appear first, remove this option from another topic.
There was an error processing your request. Please try again.
flag
  Messages 1 - 25 of 57 - Collapse all  -  Translate all to Translated (View all originals)   Newer >
The group you are posting to is a Usenet group. Messages posted to this group will make your email address visible to anyone on the Internet.
Your reply message has not been sent.
Your post was successful
 
From:
To:
Cc:
Followup To:
Add Cc | Add Followup-to | Edit Subject
Subject:
Validation:
For verification purposes please type the characters you see in the picture below or the numbers you hear by clicking the accessibility icon. Listen and type the numbers you hear
 
Piers Cawley  
View profile  
 More options Jul 22 2007, 5:54 am
From: "Piers Cawley" <pdcaw...@bofh.org.uk>
Date: Sun, 22 Jul 2007 10:54:42 +0100
Local: Sun, Jul 22 2007 5:54 am
Subject: after_initialize/after_find misfeature
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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Michael Koziarski  
View profile  
 More options Jul 22 2007, 2:02 pm
From: "Michael Koziarski" <mich...@koziarski.com>
Date: Mon, 23 Jul 2007 06:02:40 +1200
Local: Sun, Jul 22 2007 2:02 pm
Subject: Re: [Rails-core] after_initialize/after_find misfeature

> 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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Piers Cawley  
View profile  
 More options Jul 22 2007, 2:50 pm
From: "Piers Cawley" <pdcaw...@bofh.org.uk>
Date: Sun, 22 Jul 2007 19:50:03 +0100
Local: Sun, Jul 22 2007 2:50 pm
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature
On 22/07/07, Michael Koziarski <mich...@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.

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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Damian Janowski  
View profile  
 More options Jul 22 2007, 4:14 pm
From: "Damian Janowski" <damian.janow...@gmail.com>
Date: Sun, 22 Jul 2007 22:14:14 +0200
Local: Sun, Jul 22 2007 4:14 pm
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature
On 7/22/07, Piers Cawley <pdcaw...@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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
court3nay  
View profile  
 More options Jul 22 2007, 4:53 pm
From: court3nay <court3...@gmail.com>
Date: Sun, 22 Jul 2007 13:53:04 -0700
Local: Sun, Jul 22 2007 4:53 pm
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature

On Jul 22, 2007, at 1:14 PM, "Damian Janowski" <damian.janow...@gmail.com

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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Piers Cawley  
View profile  
 More options Jul 22 2007, 5:03 pm
From: "Piers Cawley" <pdcaw...@bofh.org.uk>
Date: Sun, 22 Jul 2007 22:03:35 +0100
Local: Sun, Jul 22 2007 5:03 pm
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature
On 22/07/07, court3nay <court3...@gmail.com> wrote:

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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Michael Koziarski  
View profile  
 More options Jul 22 2007, 5:25 pm
From: "Michael Koziarski" <mich...@koziarski.com>
Date: Mon, 23 Jul 2007 09:25:35 +1200
Local: Sun, Jul 22 2007 5:25 pm
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature

> 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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
court3nay  
View profile  
 More options Jul 22 2007, 5:25 pm
From: court3nay <court3...@gmail.com>
Date: Sun, 22 Jul 2007 14:25:04 -0700
Local: Sun, Jul 22 2007 5:25 pm
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature
On Jul 22, 2007, at 2:03 PM, "Piers Cawley" <pdcaw...@bofh.org.uk>  
wrote:

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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Michael Koziarski  
View profile  
 More options Jul 22 2007, 5:27 pm
From: "Michael Koziarski" <mich...@koziarski.com>
Date: Mon, 23 Jul 2007 09:27:10 +1200
Local: Sun, Jul 22 2007 5:27 pm
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature

> 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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Piers Cawley  
View profile  
 More options Jul 22 2007, 5:53 pm
From: "Piers Cawley" <pdcaw...@bofh.org.uk>
Date: Sun, 22 Jul 2007 22:53:46 +0100
Local: Sun, Jul 22 2007 5:53 pm
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature

> > 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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Michael Koziarski  
View profile  
 More options Jul 22 2007, 6:39 pm
From: "Michael Koziarski" <mich...@koziarski.com>
Date: Sun, 22 Jul 2007 15:39:38 -0700
Local: Sun, Jul 22 2007 6:39 pm
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature

> 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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Piers Cawley  
View profile  
 More options Jul 22 2007, 6:56 pm
From: "Piers Cawley" <pdcaw...@bofh.org.uk>
Date: Sun, 22 Jul 2007 23:56:39 +0100
Local: Sun, Jul 22 2007 6:56 pm
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature
On 22/07/07, Michael Koziarski <mich...@koziarski.com> wrote:

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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Jonathan Viney  
View profile  
 More options Jul 23 2007, 6:09 am
From: "Jonathan Viney" <jonathan.vi...@gmail.com>
Date: Mon, 23 Jul 2007 22:09:09 +1200
Local: Mon, Jul 23 2007 6:09 am
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature

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.janow...@gmail.com> wrote:


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Tom Ward  
View profile  
 More options Jul 23 2007, 10:30 am
From: "Tom Ward" <t...@popdog.net>
Date: Mon, 23 Jul 2007 15:30:18 +0100
Local: Mon, Jul 23 2007 10:30 am
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature
On 22/07/07, Piers Cawley <pdcaw...@bofh.org.uk> wrote:

> On 22/07/07, Michael Koziarski <mich...@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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Michael Koziarski  
View profile  
 More options Jul 23 2007, 11:42 am
From: "Michael Koziarski" <mich...@koziarski.com>
Date: Mon, 23 Jul 2007 08:42:59 -0700
Local: Mon, Jul 23 2007 11:42 am
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature

> 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.

--
Cheers

Koz


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Ben Munat  
View profile  
 More options Jul 23 2007, 8:58 pm
From: Ben Munat <bmu...@gmail.com>
Date: Mon, 23 Jul 2007 14:58:46 -1000
Local: Mon, Jul 23 2007 8:58 pm
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature

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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Pratik  
View profile  
 More options Jul 23 2007, 9:14 pm
From: Pratik <pratikn...@gmail.com>
Date: Tue, 24 Jul 2007 02:14:17 +0100
Local: Mon, Jul 23 2007 9:14 pm
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature

>  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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Ben Munat  
View profile  
 More options Jul 23 2007, 9:42 pm
From: Ben Munat <bmu...@gmail.com>
Date: Mon, 23 Jul 2007 15:42:19 -1000
Local: Mon, Jul 23 2007 9:42 pm
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature
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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Piers Cawley  
View profile  
 More options Jul 24 2007, 1:43 am
From: "Piers Cawley" <pdcaw...@bofh.org.uk>
Date: Tue, 24 Jul 2007 06:43:05 +0100
Local: Tues, Jul 24 2007 1:43 am
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature
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...


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Ben Munat  
View profile  
 More options Jul 24 2007, 3:28 am
From: Ben Munat <bmu...@gmail.com>
Date: Mon, 23 Jul 2007 21:28:19 -1000
Local: Tues, Jul 24 2007 3:28 am
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature

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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Pratik  
View profile  
 More options Jul 24 2007, 6:33 am
From: Pratik <pratikn...@gmail.com>
Date: Tue, 24 Jul 2007 10:33:59 -0000
Local: Tues, Jul 24 2007 6:33 am
Subject: Re: after_initialize/after_find misfeature
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:


 
You must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Piers Cawley  
View profile  
 More options Jul 24 2007, 6:40 am
From: "Piers Cawley" <pdcaw...@bofh.org.uk>
Date: Tue, 24 Jul 2007 11:40:57 +0100
Local: Tues, Jul 24 2007 6:40 am
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature
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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Piers Cawley  
View profile  
 More options Jul 24 2007, 7:49 am
From: "Piers Cawley" <pdcaw...@bofh.org.uk>
Date: Tue, 24 Jul 2007 12:49:26 +0100
Local: Tues, Jul 24 2007 7:49 am
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature
On 24/07/07, Pratik <pratikn...@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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Pratik  
View profile  
 More options Jul 24 2007, 8:03 am
From: Pratik <pratikn...@gmail.com>
Date: Tue, 24 Jul 2007 13:03:50 +0100
Local: Tues, Jul 24 2007 8:03 am
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature
On 7/24/07, Piers Cawley <pdcaw...@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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Piers Cawley  
View profile  
 More options Jul 24 2007, 8:14 am
From: "Piers Cawley" <pdcaw...@bofh.org.uk>
Date: Tue, 24 Jul 2007 13:14:00 +0100
Subject: Re: [Rails-core] Re: after_initialize/after_find misfeature
On 24/07/07, Pratik <pratikn...@gmail.com> wrote:

> On 7/24/07, Piers Cawley <pdcaw...@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 must Sign in before you can post messages.
To post a message you must first join this group.
Please update your nickname on the subscription settings page before posting.
You do not have the permission required to post.
Messages 1 - 25 of 57   Newer >
« Back to Discussions « Newer topic     Older topic »