Re: [TEST CASE] Constructor provided arguments and strict-default fire ordering is confusing or buggy.
On Mon, Oct 25, 2010 at 10:48 PM, Hans Dieter Pearcey wrote: > On Mon, 25 Oct 2010 16:23:45 -0500, Evan Carroll wrote: >> I'll go ahead and >> show you what I wanted to do with this. I have a Class, it should >> accept either a "company_id", or a hash of "attributes" if you provide >> a company_id the attribute hash is lazy when it is needed. If you >> provide a hash of company attributes, then the value is immediately >> entered into the DB, and the company_id gets set appropriately: > > Using new() for this seems wrong to me. Your object constructor should > construct objects, not construct objects and sometimes insert data but other > times retrieve it. For example, in DBIC this is find_or_create, which is > itself built on top of other predicates that are built on top of object > construction; it's a complex operation, and trying to make it all an > inextricable part of calling new is a code smell. I found this: http://misko.hevery.com/code-reviewers-guide/flaw-constructor-does-real-work/ to be a very clear explanation of why this is a code smell :) -- Zbigniew Lukasiak http://brudnopis.blogspot.com/ http://perlalchemy.blogspot.com/
Re: [TEST CASE] Constructor provided arguments and strict-default fire ordering is confusing or buggy.
On Mon, 25 Oct 2010 16:23:45 -0500, Evan Carroll wrote: > I'll go ahead and > show you what I wanted to do with this. I have a Class, it should > accept either a "company_id", or a hash of "attributes" if you provide > a company_id the attribute hash is lazy when it is needed. If you > provide a hash of company attributes, then the value is immediately > entered into the DB, and the company_id gets set appropriately: Using new() for this seems wrong to me. Your object constructor should construct objects, not construct objects and sometimes insert data but other times retrieve it. For example, in DBIC this is find_or_create, which is itself built on top of other predicates that are built on top of object construction; it's a complex operation, and trying to make it all an inextricable part of calling new is a code smell. If you insist on doing it anyway, I don't think you'll find any support for the approach you've chosen, because we've repeatedly said that using 'lazy' on everything is the right way to deal with this kind of attribute interdependency. Using lazy already gives you the initialization order that you want. In this particular case, you could mark everything as lazy and resolve it in BUILD. If you find that unappealing, this would also be a good use of the hypothetical "lazy default/builder, but call it automatically after initialization" attribute trait/option that's come up with some frequency in the past, if you felt like writing that. hdp.
Re: [TEST CASE] Constructor provided arguments and strict-default fire ordering is confusing or buggy.
> Also, I have no idea what your parenthetical means.
Parenthetical indicate that I know default fires before the
initializer for the attribute, but that not all defaults for all
attributes will fire before any of the initializers. Currently, an
Attribute process not a class process.
> "Awkward and counter-intuitive" is very subjective and therefore not a
> compelling reason to change things; for example, people regularly seem to
> think
> that it is awkward and counter-intuitive not to override new() when coming
> from
> vanilla Perl 5 OO.
>
> In this particular case, the way Moose does things means that code related to
> a
> given attribute (default, etc.) only sees that particular attribute in a
> half-built state -- that is, each other attribute is either completely
> initialized or not at all. This means there's much less opportunity for your
> object to be exposed in an inconsistent state.
I don't see the logic in the term "inconsistent." I've showed the flow
defined in a sequential list in my original post, maybe you can
clarify why you feel such a flow is inconsistent. I'll go ahead and
show you what I wanted to do with this. I have a Class, it should
accept either a "company_id", or a hash of "attributes" if you provide
a company_id the attribute hash is lazy when it is needed. If you
provide a hash of company attributes, then the value is immediately
entered into the DB, and the company_id gets set appropriately:
has 'attr' => (
isa => 'HashRef'
, is => 'ro'
, lazy=> 1
, predicate => '_has_attr'
, default => sub {
my $self = shift;
die "No company id" unless $self->_has_company;
# Get from db.
}
);
has 'company' => (
isa => 'Int'
, is => 'ro'
, predicate => '_has_company'
, default => sub {
my $self = shift;
die 'No attributes or company id' unless $self->_has_attr;
# Insert into DB
}
)
Anyway, it just seems very naturally that this should work. You're
explicitly providing non-defaults for some attributes, but they're
order is undefined until other attribute's get their defaults -- it
seems like the current method provides no gain whatsoever.
--
Evan Carroll - m...@evancarroll.com
System Lord of the Internets
web: http://www.evancarroll.com
ph: 281.901.0011
Re: [TEST CASE] Constructor provided arguments and strict-default fire ordering is confusing or buggy.
On Mon, 25 Oct 2010 15:28:26 -0500, Evan Carroll wrote: > I'd have assumed that the process went like this: > * BUILDARGS if any yes > * Moose provided-new which sets the attributes to the values in the hashref > * The default/initializer stage (undefined for the class, defined for > the attribute) * Initialize each attribute in turn, from constructor argument or default/builder Also, I have no idea what your parenthetical means. > * BUILD which gets the complete object yes > However, I'm now seeing this. The non-lazy defaults fire before `foo` > get's set even though foo is explicitly provided. This seems awkward > and counter-intuitive. You can find a test case here > http://gist.github.com/645659 . Could anyone provide commentary on > this behavior? Is this a bug in Moose. I'm on latest stable: 1.17. "Awkward and counter-intuitive" is very subjective and therefore not a compelling reason to change things; for example, people regularly seem to think that it is awkward and counter-intuitive not to override new() when coming from vanilla Perl 5 OO. In this particular case, the way Moose does things means that code related to a given attribute (default, etc.) only sees that particular attribute in a half-built state -- that is, each other attribute is either completely initialized or not at all. This means there's much less opportunity for your object to be exposed in an inconsistent state. hdp.
Re: [TEST CASE] Constructor provided arguments and strict-default fire ordering is confusing or buggy.
> This is not a bug. Attribute initialization order is explicitly
> undefined (the documentation mentions this in several places). If you
> need a defined initialization order, you should make the appropriate
> attributes lazy.
That's not how I read that statement. When you say attribute
initialization order is undefined I read that literally. Example,
has "foo" => ( isa => "Int", is => "ro", default => 1 );
has "bar" => ( isa => "Int", is => "ro", default => 2 );
Is undefined such that "foo", and "bar" will be set in an undefined
order. Not, that the constructor provided arguments and
strict-defaults are undefined. I don't even read "initialization" as
being an apt choice of words for setting "foo", via
Class->new({ foo => bar })
I'm not sure if this misunderstanding is unique to myself or not, but
I didn't take your statement (or the docs) to be at odds with my own
mental model of Moose behavior.
--
Evan Carroll - m...@evancarroll.com
System Lord of the Internets
web: http://www.evancarroll.com
ph: 281.901.0011
Re: [TEST CASE] Constructor provided arguments and strict-default fire ordering is confusing or buggy.
On Mon, Oct 25, 2010 at 03:28:26PM -0500, Evan Carroll wrote:
> I've recently discovered that rather against my own intuition Moose
> does not set constructor provided arguments first. I've always thought
> that Moose did the logical thing here.. If I was to read the code
> here:
>
> Class->new({ foo => bar })->baz;
>
> I'd have assumed that the process went like this:
> * BUILDARGS if any
> * Moose provided-new which sets the attributes to the values in the hashref
> * The default/initializer stage (undefined for the class, defined for
> the attribute)
> * BUILD which gets the complete object
>
> However, I'm now seeing this. The non-lazy defaults fire before `foo`
> get's set even though foo is explicitly provided. This seems awkward
> and counter-intuitive. You can find a test case here
> http://gist.github.com/645659 . Could anyone provide commentary on
> this behavior? Is this a bug in Moose. I'm on latest stable: 1.17.
This is not a bug. Attribute initialization order is explicitly
undefined (the documentation mentions this in several places). If you
need a defined initialization order, you should make the appropriate
attributes lazy.
-doy
