byterock

It fix the API (yet again) day here in the Moose-Pen

So as we all know I have been playing with the extended operators over the last few posts and I just notices something in my code;

I can enter a mixed case operator like this 'BeETween' and things will still work. This is good but I think I left a part out on the Database::Deriver side of things and in Driver ::DBI I do this;

elsif (uc($predicate->operator) eq Database::Accessor::Driver::DBI::SQL::BETWEEN) {

I uc or upper case that operator and if I take it out

--elsif (uc($predicate->operator) eq Database::Accessor::Driver::DBI::SQL::BETWEEN) {

++elsif ($predicate->operato) eq Database::Accessor::Driver::DBI::SQL::BETWEEN) {

I will obvioulsy fail my test

not ok 3 - Between Operator Params retrieve SQL correct

not ok 4 - Between Operator Params retrieve params correct

Now I am already doing a type check on that operator;

subtype 'Operator',

  as 'Str',

  where { exists( Database::Accessor::Constants::OPERATORS->{ uc($_) } ) },

…

in the 'Database::Accessor::Types' class as you can see the 'uc' there but I am not converting the incomming '$_' to uppercase I am just checking to see if it is that 'OPERATORS' constans. Thus when I pass things down to my DAD I get this in the class;

  'conditions' => [

                            bless( {

                                        'predicates' => bless( {

                                           'operator' => 'BETWeeN',

…

and I am compensating for it with that second 'uc' in Driver::DBI. Not really what I want as I should keep the API constant and treat this like the 'view' on an element, I inherit that from the DA, and 'uc' the operator before it is sent to the DA.

The question is where to do this??

I originally though it would be as easy as just adding a 'coercion' in the types class;

subtype 'Operator',

  as 'Str',

  where { exists( Database::Accessor::Constants::OPERATORS->{ uc($_) } ) },

  message {

    "The Operator '$_', is not a valid Accessor Operator!"

      . _try_one_of( Database::Accessor::Constants::OPERATORS() );

  };


++coerce 'Operator', from 'Str', via { uc($_) };

and then changing my attribute to have a 'coerce';

        has operator => (

            is      => 'rw',

            isa     => 'Operator',

++          coerce  => 1

        );

that actually proved to be a dead end as the 'subtype' will override the coercion as it already is a coercion. To make it work and I would have to create a sub-sub type which is a little too much code for what I want.

Next what I tried to do was drop the 'subtype' all together and just make a validating 'coercion' like I did in this post for the gather class;

coerce 'Operator', from 'Str', via {

    

    die "The Operator '$_', is not a valid Accessor Operator!"

         . _try_one_of( Database::Accessor::Constants::OPERATORS())

      unless(exists( Database::Accessor::Constants::OPERATORS->{ uc($_) } ));

    uc($_);     

};

however that will not work as I can only use a the above pattern in conjunction with the 'class_type' so I would have to add in this;

class_type 'Operator',   { class => 'Str' };

and as we all know there is no such thing a 'String' in perl as ref($_) when $_ is 'BeTEEN' is just 'undef' so for simple types I have to forget about using 'class_type' coercion as they have no class.

In the end I will have to also try and cover this off when I do the drill down for checking the 'parentheses' and 'view' in the '_check_element' sub, and that I think will just bea a simple one line addition like this;

 elsif (ref($element) eq 'Database::Accessor::Condition'){

           $element->predicates->operator($self->default_operator())

             if ( !$element->predicates->operator() );

           $element->predicates->operator(uc( $element->predicates->operator));

++         $element->predicates->condition($self->default_condition())

and now of course I have to pull those 'uc' out of Driver::DBI in the few places I have added them, no need to show that here and my tests still run at 100%

However just a little more for today. I went back to '30_where_basic.t' and changed one of the conditions cases to have a mixed case 'condition' value;

…

                operator => '=',

            },

            {

--              condition => 'AND',

++              condition => 'AnD',

                left      => {

                    name => 'last_name',

                    view => 'people'

…

sloppy of course but perfectly valid if I allow the same sort of thing on the operator; In the test I now get a fail;

..

ok 10 - Two field conditions create params correct

not ok 11 - Two field conditions retrieve SQL correct

ok 12 - Two field conditions retrieve params correct

…

another simple two line add in will fix that;

           $element->predicates->condition($self->default_condition())

             if ( $self->_add_condition>=2 and !$element->predicates->condition() );

           $element->predicates->condition(undef)

             if ( $self->_add_condition<=1  );

++         $element->predicates->condition(uc( $element->predicates->condition))

++           if ($element->predicates->condition() );

           $self->_check_parentheses($element->predicates);

me thinks;

Now I get

..

ok 10 - Two field conditions create params correct

ok 11 - Two field conditions retrieve SQL correct

ok 12 - Two field conditions retrieve params correct

…

So not much code today but I nipped that API inconstancy in the bud.