byterock

It condiment day again here in the Moose-Pen

Time to do some more catch-up coding as my most recent round of changes to fix all my 'element->view' woes I ended up removing any checks for 'parentheses' I had in place. To recap on the original idea of the parentheses check was to make sure that there was at least one closing bracket for each opening bracket in the 'conditions' being send down to the DAD.

As it stands now I am using the '_parentheses_check' sub to check all the attributes that that may contains not just an 'element' but any class that can use a 'predicate' so 'Elelemets','Functions','Expressions' and 'Conditions' quite an expended set.

Unfortunately I can not really test to see that the positioning of the brackets are syntactically correct as this;

  ORDER BY (left(test,12)+12)*12

may work in one SQL but not in another. All I can do is give that basic open and close promise as that will catch most problems expect when I the error spans different attributes like an open in the elements attributes and matching close for in say the link.

The first thing I am going to change is the name of the '_parentheses_check' sub to '_elements_check' as I am really doing two checks now, parentheses and view inheitance.

Next I will have to decide where and when I am going to reapply this old code;

  $self->_inc_parens()

                  if ( $predicate->open_parentheses() );

                $self->_dec_parens()

                  if ( $predicate->close_parentheses() ); 

As I was looking at the above in git-history I also noticed these lines I left out

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

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

what that does in insert the default 'AND' if the current predicate has more than one nested predicate and there is no condition.

I guess that sub name change is a better idea that I though as I am doing three checks not just two. I did wonder if I could do more but taking one more look at the code I did not find any more.

The next sub I looked at was of course '_check_view' but now that I am doing more than one check I will rename that one as well to '_check_element' and the first thing I did after that was re-run '57_dad_elements.t' to make sure I did not bugger anything up and it ran clean.

Looking at the new '_check_element' sub I know that my parentheses will only occur in an class that expresses the 'Database::Accessor::Roles::Comparators' role and I already check for that here;

...

 else {

           return 

              unless(does_role($element,"Database::Accessor::Roles::Comparators"));

…

thus I can put that check right after that (after I change it to work in the new sub);

            return 

              unless(does_role($element,"Database::Accessor::Roles::Comparators"));

++           $self->_inc_parens()

++                  if ( $element->open_parentheses() );

++                $self->_dec_parens()

++                  if ( $element->close_parentheses() ); 

I also noticed that right after this I had this;

 map( $self->_check_element($_),@{$element->left})

              if (ref($element->left) eq "ARRAY");

            map( $self->_check_element($_),@{$element->right})

               if (ref($element->right) eq "ARRAY");

which is now obsolete code as I check for that much earlier in my code so out that goes. Finally I will have to change this block of code a little;

       die " Database::Accessor->"

          . lc($action)

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

          if ( $self->_parens_are_open() );

to account for the fact that I am checking many more things;

       die " Database::Accessor->"

          . lc($action)

          . " Unbalanced parentheses in your static or dynamic attributes. Please check them!"

          if ( $self->_parens_are_open() );

For the 'AND' condition I may have just wait on that one for now and see how my testing goes as I do not think I can just use the simple solution from the first iteration of this code.

My test case for this is '58_parenthes.t' and on my first run with the above changes I get a few fails and a few passes but most importantly I got a 255 exit on the fifth test;

can't call method "conditions" on an undefined value at D:\GitHub\database-accessor\lib/Database/Accessor.pm line 697.

Which is this block of code

       if ( $self->gather() || $self->dynamic_gather()  ) {

            push(

                @items,

                (

                    @{ $self->gather->conditions }, @{ $self->gather->elements }, @{ $self->dynamic_gather->conditions }, @{ $self->dynamic_gather->elements }

                )

            );

        }

and checking the test I see that there is no 'dynamic_gather' hence the error so a quick re-write to;

       push(@items,(@{ $self->gather->conditions }, @{ $self->gather->elements }))

          if ( $self->gather());

          

        push(@items,(@{ $self->dynamic_gather->conditions }, @{ $self->dynamic_gather->elements }))

          if ( $self->dynamic_gather());

on the next run that was fixed but I got

Can't locate object method "add_filter" via package "Database::Accessor" at 58_parenthes.t line 288.

and the culprit was my test as I still had some old API code in there that I just removed

--$da->add_filter({left => {

--                name => 'last_name',

--                view => 'People'

--            },

--            right           => { value => 'test-8' },

--            operator        => '=',

--            open_parentheses  => 0,

--            close_parentheses => 1,

--            condition       => 'AND',

--        }); 

and I had a few updates to the test as my 'fail message' has changed and now I no longer get any of those nasty 255 fails just

ok 1 - Balanced parentheses

not ok 2 - Caught unbalanced parentheses

ok 3 - Balanced parentheses

not ok 4 - Caught parentheses

not ok 5 - And added to last condition predicate

ok 6 - Balanced parentheses# Looks like you planned 9 tests but ran 7.

not ok 7 - Caught unbalanced parentheses

Looks like I have a little more work to do but that is another post.