Welcome to Moose Re-Factory

Its re-factor day here in the Moose-Pen

The first thing I got to today was complete the test case '40_dynamic_not_present.t' for all four dynamic attributes. No need to dump the many lines of very similar hash-refs here but here is the latest results from the case above;


not ok 1 - add_condition attribute->condition only allowed to add elements that are in the
elements attribute
ok 2 - add_gather attribute->view_elements only allowed to add elements that are in the
elements attribute
not ok 3 - add_gather attribute->elements only allowed to add elements that are in the
elements attribute
not ok 4 - add_gather attribute->conditions only allowed to add elements that are in the
elements attribute
not ok 5 - add_link attribute->link only allowed to add elements that are in the e
lements attribute
not ok 6 - add_sort attribute->sort only allowed to add elements that are in the
elements attribute

You might notice that I have a few in there twice that is because I have to tests more than one attribute on an incoming action.

The fist thing I looked at was the amount of iteration I do when I 'execute' a 'CRUD' method on my DA. It seems that I iterate over both the static and dynamic attributes when doing the '_elements_check' sub. For example;


push( @items,
@{ $self->conditions },
@{ $self->dynamic_conditions },
@{ $self->sorts },
@{ $self->dynamic_sorts },
@{ $self->elements } );

I really only have to check the 'static' attributes when the $da is instantiated not each time I execute a 'CRUD' method so a re-facotring I will go.

The fist thing I will do it change the '_elements_check' sub to this.


private_method _elements_check => sub {
my $self = shift;
my ($items,$action,$type) = @_;

foreach my $item (@{$items}) {
$self->_inc_conditions()
if (ref($item) eq 'Database::Accessor::Condition');
if (ref($item) eq 'ARRAY'){
$self->_reset_conditions();
foreach my $condition (@{$item}){
$self->_inc_conditions()
if (ref($condition) eq 'Database::Accessor::Condition');
$self->_check_element($condition,0);
}
}
else {
$self->_check_element($item,0);
}
}

die " Database::Accessor->"
. lc($action)
. " Unbalanced parentheses in your “.$type.” attributes. Please check them!"
if ( $self->_parens_are_open() );

};


Which allows me to call it from more than one place by assembling the '$items' array-ref before I make the call. I will check the static attributes first and only once by making the above call in the “BEGIN” block like this;


sub BUILD {
…
        my @items;
        push( @items,
            @{ $self->conditions },
            @{ $self->sorts },
             @{ $self->elements } );

foreach my $link (@{ $self->links }){
my $view = $link->to;
$self->_check_element($link->conditions,0,$view->name);
push(@items,$link->conditions);
}
push(@items,(@{ $self->gather->conditions }, @{ $self->gather->elements }))
if ( $self->gather());
push(@items,@{ $self->gather->view_elements })
if ($self->gather() and $self->gather->view_elements);

$self->_elements_check(\@items,"New","Static");
...


The second part of this re-factor is found in the 'execute' sub where I now do;

...
$self->_reset_parens();
$self->_reset_conditions();
my @items;
push( @items,
@{ $self->dynamic_conditions },
@{ $self->dynamic_sorts },
);
foreach my $link ((@{ $self->links },@{ $self->dynamic_links })){
push(@items,$link->conditions);
}

push(@items,(@{ $self->dynamic_gather->conditions }, @{ $self->dynamic_gather->elements }))
if ( $self->dynamic_gather());
push(@items,@{ $self->dynamic_gather->view_elements })
if ($self->dynamic_gather() and $self->dynamic_gather->view_elements);
push(@items,@{ $self->dynamic_gather->conditions })
if ($self->dynamic_gather() and $self->dynamic_gather->conditions);

$self->_elements_check(\@items,$action,"dynamic");


before I do the '_elements_check'.

As it stands I know with this change I have invalidated a number of test cases since I am making that elements check at instantiation. The first test case that I know will not work right is '58_parenthes.t' as I check for unbalanced parentheses like this;


$da = Database::Accessor->new($in_hash2);
like(
exception { $da->retrieve( Data::Test->new() ) },
qr /Unbalanced parentheses in your static or dynamic attributes/,
"Caught unbalanced parentheses"
);

I will fix that to this

like(
exception { $da = Database::Accessor->new($in_hash2); },
qr /Unbalanced parentheses in your static or dynamic attributes/,
"Caught unbalanced parentheses"
);

and a number of other small changes and eventually I get it to fully pass.

As I was getting the above working I had a though on the 'dynamic' attributes. Why wait until one of the the 'CRUD' functions are called to do the 'dynamic' elements check. I could just add in triggers to the four 'add_' functions and do the elements_check then?

Will I sort of like that idea as I will be able to do the '_check_elements_present' at the same time as well.

The only problem I can see if and end user wants to add a dynamic condition with an open parentheses then after some local logic add one or more conditions I could see having it hard fail at the first add could be problematic.

Oh well plenty of things to do for tomorrow.

Blue-Moose-Turd-Factory-sig.jpg

Leave a comment

About byterock

user-pic Long time Perl guy, a few CPAN mods allot of work on DBD::Oracle and a few YAPC presentations