byterock

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.