byterock

ts re-factor day again here at the Moose-pen

With all my test passing since yesterday's post I think it is time I spend a little time re factoring before I move onto other things. Now as I said many times before I would like to have whatever is going to the DAD, Driver::DBI in this case, to be as free of logic errors as possible.

In this vain I noticed that when dealing with a where clause and predicates in general there are a two rules that should be enforced before any conditions or filters are passed into a DAD, First any parentheses that is opened must be closed and second two or more predicates must be joined by a condition.

To implement this I am going back into Accessor.pm, now I was thinking I could do this as part of the Coercing using types but I would be limited to only data that is coming in. I would be able to check if an inputting hash-ref or array-ref followed the two rules above but I would not be able have these values interact with values already in the attribute so one would could still get errors.

The only fail-safe way to test for the above tow rules is on the execute when all the values of the attributes both dynamic and static are present and the place to do that is in the private _execute sub.

Fist though I will add in this attribute

has _parens_are_open => (

    traits  => ['Counter'],

    is      => 'rw',

    default => 0,

    isa     => 'Int',

    handles => {

        _inc_parens   => 'inc',

        _dec_parens   => 'dec',

        _reset_parens => 'reset'

    }

);

Now I have a counter that I can use to tests if any parentheses are unmatched which is really all I can test for and next I added these two subs

     private_method _parenthes_check => sub {

        my $self = shift;

        my ($action) = @_;

        $self->_reset_parens();

        $self->_count_parenthes( @{ $self->conditions },

            @{ $self->dynamic_conditions } );

        die " Database::Accessor->"

          . lc($action)

          . " Unbalanced parenthes in your conditions and dynamic_conditions. Please check them!"

          if ( $self->_parens_are_open() );

        if ( $action eq Database::Accessor::Constants::RETRIEVE ) {

            $self->_reset_parens();

            $self->_count_parenthes( @{ $self->filters },

                @{ $self->dynamic_filters } );

            die " Database::Accessor->"

              . lc($action)

              . " Unbalanced parenthes in your filters and dynamic_filters. Please check them!"

              if ( $self->_parens_are_open() );

        }


    };

    private_method _count_parenthes => sub {

        my $self            = shift;

        my (@conditions)    = @_;

        my $predicate_count = 0;

        foreach my $condition (@conditions) {

            foreach my $predicate ( @{ $condition->{predicates} } ) {

                $self->_inc_parens()

                  if ( $predicate->open_parenthes() );

                $self->_dec_parens()

                  if ( $predicate->close_parenthes() );

                $predicate->condition(Database::Accessor::Constants::AND)

                  if ( $predicate_count and !$predicate->condition() );

                $predicate_count=1;

            }

        }

    };

Now I will spare you description of all the iterations I went though to get the above I will just give the 25cent tour.

In _parenthes_check I first reset the parens counter, then send the static and dynamic condtions off to another sub to be processed. In that count_parenthes sub I simply iterate over whatever conditions are sent in and either increment the parens counter if the predicate has an open or decrement if it has a close.

At the same time I check to see if the predicate does not have a condition and there are more than one predicate has been processed, if this is true I set the condition to 'and'.

After I return from this iteration I will die if the _parens_are_open has a value. Next I do the same for static and dynamic filters if this is a retrieve action.

Now I changed things a little here as set that second rule 'two or more predicates must be joined by a condition' by satisfying by giving it a default of 'and' which was what I had originally.

Now none of this would be much good without a test and you can have a look at the shinny new '58_parenthes.t' test case if you want to see all the details. So another promise kept by Database::Accessor.