On Sun Nov 16 08:51:03 2008, JSWARTZ wrote:
Show quoted text> > Passing inline_constructor => 0 to make_immutable is probably good
> > enough (it was for me! :)).
>
> Cool. Is this documented somewhere? The only documentation I could find
> for make_immutable was Recipe7, which doesn't mention this.
It should be mentioned in the Moose::Cookbook::WTF, there is a whole section on the evils of
overriding new() in there (
http://search.cpan.org/~drolsky/Moose-
0.61/lib/Moose/Cookbook/WTF.pod#Constructors_&_Immutability)
Show quoted text> >
> > >
> > > Fey::ORM has an example of this.
> > >
> > > But this is definitely not a bug.
> >
> > Agreed. It might be useful to warn in this instance though. I've been
> > bitten by the same thing. If the "new" is not provided by
> > Moose::Object, then we could warn if there's an *implicit*
> > inline_constructor => 1. Explicitly saying inline_constructor => 1
> > would silence the warning, because presumably you know what you're
> > doing.
> >
> > I figure many users of Moose are just cargo culting the make_immutable
> > as an easy way to speed up the class, without knowing what it's
> > actually doing.
>
> Guilty as charged. But I shouldn't HAVE to understand how make_immutable
> is implemented to use it.
Agreed, however I think all you need to understand here (and unfortunately it doesnt seem to be
documented clearly anywhere) that immutable creates an inlined constructor for you, which means it
will create a local ->new() which obviously will override.
Show quoted text> make_immutable is essentially documented as
> "this will speed up your class, but in return you can't make changes to
> the metaclass afterwards". I followed those rules, and still got bitten,
> because I assumed that basic OO rules (like constructor inheritance)
> would not be affected. Sounds like Shawn got bitten too. I expect then
> that many others will also get bitten.
I won't disagree that it can be surprising, however it is not breaking basic OO rules because it is
creating a local ->new(), this it the trade off.
Show quoted text> If this behavior remains, as it sounds like it will, then there should
> definitely at least be a warning, and something in the docs about "this
> will ignore inherited constructors."
We can detect an inherited constructor so we can basically just warn in the specific circumstance,
Shawn mentioned that above.
Show quoted text> What makes this worse is that you *can* override new() in the class
> itself, just not in its parents. If make_immutable were really "all
> about" inlining the constructor (it isn't - it's also about speeding up
> attributes), then make_immutable ought to fail if there is a new()
> already defined.
Actually, that line in the docs is kind of misleading, because Moose already inlines the accessors for
you by default. So immutability has little affect on the accessors and almost all of the benefit is in the
constructors.
Show quoted text> > This issue is one of the main reasons we tell people to not to
> > override new().
>
> I didn't know that. I did know that overriding new() is strongly
> discouraged, but as I said, I had a legitimate reason to override new()
> in this case.
Can you explain that reason? Perhaps there is a (poorly documented) feature that exists that we can
point you too. Between BUILD, BUILDARGS, attribute defaults, attribute builders and attribute laziness
there are very few good reasons left actually.
- Stevan
