stevan / p5-mop-redux

A(nother) MOP for Perl 5
139 stars 36 forks source link

Attribute values should be set from constructor arguments prior to default values being applied #151

Open Ovid opened 10 years ago

Ovid commented 10 years ago

I've wound up with a non-deterministic bug because I've set the value from an attribute in the constructor, but the default value of a different attribute relies on the first attribute. The following test demonstrates the error. Roughly half of these tests fail. If we're agree that this is an error, I'm happy to add a new test to the test suite for this.

use Test::More;
{
    use 5.18.0;
    use mop;
    class Attributes {
        has $!first is ro = do {
            ::ok($_->second, '$!second should be set') 
        };                                                                                                                                                                                                    
        has $!second is ro;
    }
}

for ( 1..100 ) {
    Attributes->new( second => 1 )->first;
}

done_testing;

And the summary:

Test Summary Report
-------------------
attribute_order.t (Wstat: 10752 Tests: 100 Failed: 42)
  Failed tests:  3-4, 6, 12-13, 16, 18-19, 22, 24-26, 28
                30, 32-36, 39, 47-48, 53, 59, 61, 63, 65
                67, 69-72, 76-77, 81-83, 92, 94, 96-97
                99
  Non-zero exit status: 42
Files=1, Tests=100,  0 wallclock secs ( 0.03 usr  0.01 sys +  0.04 cusr  0.00 csys =  0.08 CPU)
Result: FAIL

Cheers, Ovid

doy commented 10 years ago

I agree that this is a problem, but I don't think that making constructor parameters different from defaults is the right answer. I think that if class Foo { has $!foo; has $!bar = $!foo }; Foo->new(foo => 1) works, then class Foo { has $!foo = 1; has $!bar = $!foo; }; Foo->new should also work. I kinda wonder if we do want to make attribute declarations respect ordering - we are making them resemble variable declarations as much as possible, and my $foo = 1; my $bar = $foo does work, so it seems like has $!foo = 1; has $!bar = $!foo should work for the same reason.

doy commented 10 years ago

(If all you care about is initializing via accessors, then adding is lazy to the attribute whose default is calling the accessor already works.)

Ovid commented 10 years ago

I would suggest that the less dependencies we have on order, the more robust p5-mop is likely to be. We don't care what order subs/methods are declared, why should we for attributes? One of the lovely features of functional languages is that order generally does not matter (that said, I agree it's not a trivial problem).

doy commented 10 years ago

It is possible that we could provide a different magic implementation for attribute access in defaults vs attribute access in methods, but it's not entirely clear that that wouldn't cause other problems, especially when dealing with classes built through the mop rather than through an actual class declaration. It is something to consider, though.